Skip to content

Conversation

@truehazker
Copy link
Owner

@truehazker truehazker commented Dec 3, 2025

  • Bump @biomejs/biome to version 2.3.8.
  • Update elysia package to version 1.4.17.
  • Enhance error handling in main.ts to include request body in logs.

Summary by CodeRabbit

  • Bug Fixes

    • Improved error handling to avoid exposing internal errors to clients; unhandled issues now return a generic 500 response and logging message updated for clarity.
  • Chores

    • Bumped configuration/schema references and runtime dependency versions for maintenance.
    • Removed an explicit environment binding from the CI test job.

✏️ Tip: You can customize this high-level summary in your review settings.

- Bump @biomejs/biome to version 2.3.8.
- Update elysia package to version 1.4.17.
- Enhance error handling in main.ts to include request body in logs.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 3, 2025

Walkthrough

Bumps schema and dependencies; modifies server error handling to log unhandled exceptions and return a generic 500 instead of exposing errors; removes the test environment binding from the CI workflow.

Changes

Cohort / File(s) Summary
Configuration / Dependencies
biome.json, package.json
biome.json $schema bumped 2.3.6 → 2.3.8. package.json dependency bumps: elysia ^1.4.16 → ^1.4.17, @biomejs/biome 2.3.6 → 2.3.8.
Error handling
src/main.ts
Added imports ElysiaCustomStatusResponse, status; onError now leaves custom status responses intact, logs other unhandled errors as "Unhandled error", and responds with a generic 500 instead of returning the original error.
CI workflow
.github/workflows/tests.yml
Removed the environment: test setting from the test job.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Client
participant Server
participant Logger
Note over Server: Request handling with onError hook
Client->>Server: HTTP request
alt handler throws ElysiaCustomStatusResponse or non-UNKNOWN
Server-->>Client: return original/custom status response
else handler throws UNKNOWN error
Server->>Logger: log "Unhandled error" + details
Logger-->>Server: logged
Server-->>Client: HTTP 500 Internal Server Error (generic)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Review src/main.ts onError branches and type guards around ElysiaCustomStatusResponse and code === 'UNKNOWN'.
  • Confirm package.json/biome.json bumps are compatible with CI and build tooling.
  • Check workflow secret/environment implications after removing environment: test.

Possibly related PRs

Poem

🐰 A rabbit pads through logs at night,
Swaps exposed errors for a quiet 500 light,
Versions hopped up, CI lost its vest,
The warren hums — the server rests. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: updating biome schema version and bumping elysia/biome dependencies. However, it omits the significant error handling enhancement in main.ts, which is a substantial behavioral change beyond just dependency updates.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/on-error-fix

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 63b4a37 and ede39b7.

📒 Files selected for processing (1)
  • .github/workflows/tests.yml (0 hunks)
💤 Files with no reviewable changes (1)
  • .github/workflows/tests.yml

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main.ts (1)

23-38: Critical: Logging request body exposes PII and sensitive data.

Logging the raw request body without sanitization can expose sensitive information including passwords, tokens, credit card numbers, emails, and other PII. This violates privacy regulations (GDPR, CCPA) and the coding guideline against logging PII.

Based on coding guidelines, logging sensitive data like emails and other user identifiers is prohibited. Apply this diff to sanitize the body before logging:

+// Helper to sanitize sensitive fields from request body
+const sanitizeBody = (body: unknown): unknown => {
+  if (!body || typeof body !== 'object') return body;
+  
+  const sensitiveFields = ['password', 'token', 'secret', 'apiKey', 'creditCard', 'ssn'];
+  const sanitized = { ...body };
+  
+  for (const key of Object.keys(sanitized)) {
+    if (sensitiveFields.some(field => key.toLowerCase().includes(field))) {
+      sanitized[key] = '[REDACTED]';
+    }
+  }
+  
+  return sanitized;
+};
+
 .onError(({ code, error, request, body }) => {
   // Return Elysia's handled errors as-is
   if (error instanceof ElysiaCustomStatusResponse || code !== 'UNKNOWN') {
     return error;
   }

   // Log unhandled errors
   log.error(
     {
       code,
       err: error,
       http: request
         ? {
             method: request.method,
             url: request.url,
             referrer: request.headers.get('referer') ?? undefined,
-            body: body,
+            body: sanitizeBody(body),
           }
         : undefined,
     },
     'Unhandled error',
   );

Alternatively, consider removing the body from logs entirely if comprehensive sanitization is difficult to maintain.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 22cb880 and c68c87c.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • biome.json (1 hunks)
  • package.json (1 hunks)
  • src/main.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,tsx,js,jsx}: Strictly follow Biome linter rules and run bun run lint:fix to auto-fix issues
Use explicit imports instead of wildcard imports

Files:

  • src/main.ts
src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.{ts,tsx}: TypeScript strict mode is enabled. Do not use any types unless absolutely necessary
Use src/common/config.ts for environment variables with Envalid. Do not use process.env directly
All new features must include unit/integration tests

Files:

  • src/main.ts
🔇 Additional comments (4)
biome.json (1)

2-2: LGTM! Schema version aligned with dependency.

The schema URL update correctly reflects the Biome version bump to 2.3.8 in package.json.

src/main.ts (3)

3-3: Verify new imports are available in elysia 1.4.17.

The new imports ElysiaCustomStatusResponse and status are used in the error handler. Please ensure these are exported by elysia 1.4.17 (see verification in package.json review).


17-21: Good defensive error handling logic.

Returning Elysia's handled errors as-is while intercepting unknown errors is the correct approach. This preserves framework-level error handling while adding custom logic for unhandled cases.


40-41: Excellent security practice!

Returning a generic 500 error instead of exposing internal error details is the correct approach for unhandled errors. This prevents information leakage to potential attackers.

@truehazker truehazker merged commit 249a09d into develop Dec 3, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants