Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThe pull request enhances error serialization and logging capabilities. A new comprehensive test suite validates the logger utility's error serialization across diverse error shapes. The serializeError function is refactored to use object spreading with explicit field normalization. Webhook error handling is updated to log raw error objects instead of stringified messages. The version is bumped to v2.20.22. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/utils/logger.ts (1)
139-150: LGTM! Clean error serialization approach.The spread operator combined with explicit field assignment correctly captures both custom enumerable properties (like
statusCode,code) and standard Error properties (name,message,stack). This ensures rich error context is preserved in production logs.One minor edge case: properties added with
Object.defineProperty(..., {enumerable: false})won't be captured by the spread. If you want to verify the spread behavior handles your common error shapes correctly, you could run:#!/bin/bash # Find examples of Error instances with custom properties to verify spread behavior rg -n -C3 --type=ts 'new Error\(' | rg -A5 'statusCode|code|response|config'
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/utils/logger.test.ts(1 hunks)apps/web/utils/logger.ts(1 hunks)apps/web/utils/webhook/error-handler.ts(1 hunks)version.txt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (3)
version.txt (1)
1-1: LGTM!Standard version bump to v2.20.22.
apps/web/utils/webhook/error-handler.ts (1)
37-40: LGTM!Logging the raw error object instead of extracting the message aligns well with the improved error serialization in
logger.ts. The logger will now capture richer error context including custom properties like status codes and error codes.apps/web/utils/logger.test.ts (1)
1-182: LGTM! Comprehensive error serialization test coverage.The test suite thoroughly validates that various error shapes are serialized without
[object Object]artifacts while preserving important fields. The coverage includes simple errors, nested structures, arrays, and axios-like formats.The comment on lines 56-57 correctly notes that Error instances show only their message in console logs. This is expected behavior—the console logger (used in development) converts Error instances to messages for readability via
processErrorsInObject, while the Axiom logger (used in production) preserves custom properties in theerrorFullfield viaserializeError.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="apps/web/utils/logger.test.ts">
<violation number="1" location="apps/web/utils/logger.test.ts:28">
console.error is spied on but never restored, so later tests run with a mocked console.error and lose real error output. Add an afterEach (or enable vi.restoreAllMocks) to clean up the spy after each test.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); |
There was a problem hiding this comment.
console.error is spied on but never restored, so later tests run with a mocked console.error and lose real error output. Add an afterEach (or enable vi.restoreAllMocks) to clean up the spy after each test.
Prompt for AI agents
Address the following comment on apps/web/utils/logger.test.ts at line 28:
<comment>console.error is spied on but never restored, so later tests run with a mocked console.error and lose real error output. Add an afterEach (or enable vi.restoreAllMocks) to clean up the spy after each test.</comment>
<file context>
@@ -0,0 +1,182 @@
+
+ beforeEach(() => {
+ vi.clearAllMocks();
+ consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
+ });
+
</file context>
1 participant
Adjust logger error serialization in
apps/web/utils/logger.serializeErrorand log full webhook errors inapps/web/utils/webhook/error-handler.handleWebhookErrorReplace manual property copying with object spread plus explicit
name,message, andstackinserializeError, removeisRecord, update webhook error logging to pass the raw error, and add a Vitest suite validating error output across varied shapes.📍Where to Start
Start with
serializeErrorin logger.ts, then review the logging change in webhook/error-handler.ts, and validate behavior via logger.test.ts.Macroscope summarized 7c92712.
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.