Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork928
Fix excessive framework noise in error stack traces#2442
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Fix excessive framework noise in error stack traces#2442
Uh oh!
There was an error while loading.Please reload this page.
Conversation
changeset-botbot commentedAug 27, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
🦋 Changeset detectedLatest commit:e16f893 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 packages
Not sure what this means?Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
coderabbitaibot commentedAug 27, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
WalkthroughReworks stack-trace handling in packages/core/src/v3/errors.ts: BUILT_IN_ERROR now generates its stackTrace via correctErrorStackTrace(enhancedError.stackTrace, undefined, { removeFirstLine: false }) so the first line is preserved while per-line normalization and frame filtering run; AsyncLocalStorage frames are added to the ignore list. Adds a unit test at packages/core/test/errors.test.ts that asserts user frames are preserved and internal/framework frames (including AsyncLocalStorage and internal modules) are filtered from createJsonErrorObject output. Adds a changeset bumping @trigger.dev/core with a note about improved stack-trace readability. No public API changes. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat withCodeRabbit:
SupportNeed help? Create a ticket on oursupport page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File ( |
There was a problem hiding this 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
🧹 Nitpick comments (2)
packages/core/src/v3/errors.ts (1)
378-379:Broaden (or tighten) AsyncLocalStorage filtering to reduce false positives and cover more runtimes/AsyncLocalStorage/ may hide legitimate user frames and might miss other node internals. Consider adding targeted patterns for node built-ins and OTEL context manager while keeping current behavior.
Apply:
const LINES_TO_IGNORE = [ /ConsoleInterceptor/, /TriggerTracer/, /TaskExecutor/, /EXECUTE_TASK_RUN/, /@trigger.dev\/core/, /packages\/core\/src\/v3/, /safeJsonProcess/, /__entryPoint.ts/, /ZodIpc/, /startActiveSpan/, /processTicksAndRejections/,- /AsyncLocalStorage/,+ /AsyncLocalStorage/, // keep for backward compatibility+ /AsyncLocalStorageContextManager/, // otel context mgr+ /node:async_hooks/, // node built-in scheme+ /node:internal\//, // other node internals+ /@opentelemetry\//, // otel frames ];packages/core/test/errors.test.ts (1)
25-47:Also assert header removal and preserved name/messageSince removeFirstLine is now applied, assert the stack no longer contains the header, and that name/message are still surfaced.
Apply:
const jsonError = createJsonErrorObject(taskRunError);+ // Header should be removed from stackTrace but preserved in fields+ expect(jsonError.stackTrace).not.toContain("Error: Network error occurred");+ expect(jsonError.name).toBe("Error");+ expect(jsonError.message).toBe("Network error occurred");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.changeset/dry-taxis-wash.md(1 hunks)packages/core/src/v3/errors.ts(2 hunks)packages/core/test/errors.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
packages/core/test/errors.test.tspackages/core/src/v3/errors.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Our tests are all vitest
Files:
packages/core/test/errors.test.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
packages/core/test/errors.test.tspackages/core/src/v3/errors.ts
🧬 Code graph analysis (1)
packages/core/test/errors.test.ts (1)
packages/core/src/v3/errors.ts (1)
createJsonErrorObject(222-251)
🔇 Additional comments (3)
.changeset/dry-taxis-wash.md (1)
1-6:Patch bump and message look goodClear, scoped changeset; patch level is appropriate for behavior-only change.
packages/core/src/v3/errors.ts (1)
230-233:Apply stack filtering for BUILT_IN_ERROR — good fixRouting BUILT_IN_ERROR through correctErrorStackTrace with removeFirstLine improves user-facing stacks and aligns with other paths.
packages/core/test/errors.test.ts (1)
1-48:Strong, targeted test — preserves user frames and filters framework noiseCovers the key regression. Nice.
| stackTrace:correctErrorStackTrace(enhancedError.stackTrace,undefined,{ | ||
| removeFirstLine:true, | ||
| }), | ||
| }; |
coderabbitaibotAug 27, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
💡 Verification agent
🧩 Analysis chain
Verify no consumers rely on the “Error: …” header being present in stackTrace
Since removeFirstLine drops the header line from stackTrace, double-check downstream formatters (Slack/email/API clients) don’t expect it there.
Run:
🏁 Script executed:
#!/bin/bash# Find usages that might format jsonError.stackTrace directlyrg -nP --type=ts --type=tsx -C2'\bcreateJsonErrorObject\s*\('rg -nP --type=ts --type=tsx -C2'\bstackTrace\b' apps packages
Length of output: 185
🏁 Script executed:
#!/bin/bash# Rerun search for createJsonErrorObject across TS and TSX files in apps and packagesrg -n'createJsonErrorObject\(' -C2 -g'*.{ts,tsx}' apps packages# Rerun search for direct stackTrace property access in apps and packagesrg -n'\.stackTrace\b' -C2 -g'*.{ts,tsx}' apps packages
Length of output: 12404
Adjust downstream consumers for new stackTrace format
We’ve confirmed that several formatting routines rely on the first line ofstackTrace being the"Error: …" header. Since we now strip that line by default, these consumers need to be updated to use the separatename/message fields rather than parsingstackTrace directly:
• packages/cli-v3/src/mcp/formatters.ts (around line 51)
Current:
if(run.error.stackTrace){lines.push(`Stack:${run.error.stackTrace.split("\n")[0]}`);}
Suggested:
- lines.push(`Stack: ${run.error.stackTrace.split("\n")[0]}`);+ // Use the explicit name/message fields now that header’s removed+ lines.push(`Stack: ${run.error.name || "Error"}: ${run.error.message}`);
• packages/cli-v3/src/dev/devOutput.ts (around line 260)
Current:
return`\n\n${error.stackTrace.replace(/^Error:/,chalkError("X Error: "))}\n`;
Suggested:
- ${error.stackTrace.replace(/^Error: /, chalkError("X Error: "))}+ ${chalkError("X Error: ")}${error.message}\n${error.stackTrace}
• apps/webapp/app/v3/services/alerts/deliverAlert.server.ts (line 705)
Current wrapserror.stackTrace ?? error.message in a code block. To preserve context, consider renderingError: ${error.name}: ${error.message} separately and then the trace.
• apps/webapp/app/routes/…/route.tsx (around line 911)
UI rendersenhancedError.stackTrace without the header—verify that the header is shown elsewhere (e.g. viaerror.name +error.message).
Please update these formatters/UI components to reference the dedicated error fields instead of assuming the header lives instackTrace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@omkardongre Can you update this change to setremoveFirstLine: false that way we don't break these expectations?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag@coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@ericallam Thanks for the feedback. Updated removeFirstLine to false as it is better to keep the error header for compatibility and readability.
Add AsyncLocalStorage to LINES_TO_IGNORE (removes OpenTelemetry noise)Apply error stack trace filtering for createJsonErrorObjectAdd test for framework stack trace filteringAdd changesetThis change improves readability of error traces in user-facing contexts(Slack alerts, email notifications, API responses, run streams) byfiltering out internal framework noise that clutters error messages.
cf9be9f toe16f893CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/core/src/v3/errors.ts (1)
405-406:Narrow AsyncLocalStorage filter to avoid hiding legitimate user framesThe regex /AsyncLocalStorage/ is broad and may drop user stacks that use Node’s AsyncLocalStorage directly. Target OTEL/core frames instead.
Apply:
const LINES_TO_IGNORE = [ /ConsoleInterceptor/, /TriggerTracer/, /TaskExecutor/, /EXECUTE_TASK_RUN/, /@trigger.dev\/core/, /packages\/core\/src\/v3/, /safeJsonProcess/, /__entryPoint.ts/, /ZodIpc/, /startActiveSpan/, /processTicksAndRejections/,- /AsyncLocalStorage/,+ /\bAsyncLocalStorageContextManager\b/,+ /node:async_hooks/,+ /\bAsyncLocalStorage\.run\b/, ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.changeset/dry-taxis-wash.md(1 hunks)packages/core/src/v3/errors.ts(2 hunks)packages/core/test/errors.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/dry-taxis-wash.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/test/errors.test.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
packages/core/src/v3/errors.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
packages/core/src/v3/errors.ts
🔇 Additional comments (2)
packages/core/src/v3/errors.ts (2)
230-233:BUILT_IN_ERROR stack filtering with header preserved — LGTMUsing correctErrorStackTrace with removeFirstLine: false keeps consumer expectations intact while filtering framework noise.
308-312:Confirm TASK_HAS_N0_EXECUTION_SNAPSHOT vs TASK_HAS_NO_EXECUTION_SNAPSHOT
Both the Zod schema inpackages/core/src/v3/schemas/common.ts(L181) and the branch inpackages/core/src/v3/errors.ts(L310) reference"TASK_HAS_N0_EXECUTION_SNAPSHOT"(zero). No occurrences of"TASK_HAS_NO_EXECUTION_SNAPSHOT"found; verify the intended code value against the spec and correct both locations if it should use the letter O.
| expect(jsonError.stackTrace).toContain( | ||
| "at fetchData (file:///src/trigger/utils/helper.ts:4:9)" | ||
| ); | ||
| expect(jsonError.stackTrace).toContain( | ||
| "at processResponse (file:///src/trigger/utils/helper.ts:9:10)" | ||
| ); | ||
| expect(jsonError.stackTrace).toContain( | ||
| "at parseResult (file:///src/trigger/utils/helper.ts:14:10)" | ||
| ); | ||
| expect(jsonError.stackTrace).toContain("at callAPI (file:///src/trigger/services/api.ts:6:10)"); | ||
| expect(jsonError.stackTrace).toContain("at localHelper (file:///src/trigger/example.ts:7:10)"); | ||
| expect(jsonError.stackTrace).toContain("at run (file:///src/trigger/example.ts:17:12)"); | ||
| // Should filter framework noise | ||
| expect(jsonError.stackTrace).not.toContain("_tracer.startActiveSpan.attributes"); | ||
| expect(jsonError.stackTrace).not.toContain("taskExecutor.ts"); | ||
| expect(jsonError.stackTrace).not.toContain("tracer.ts"); | ||
| expect(jsonError.stackTrace).not.toContain("AsyncLocalStorage.run"); | ||
| expect(jsonError.stackTrace).not.toContain("AsyncLocalStorageContextManager"); | ||
| expect(jsonError.stackTrace).not.toContain("node_modules/@trigger.dev/core"); | ||
| expect(jsonError.stackTrace).not.toContain(".npm/_npx"); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I would prefer an inline snapshot here to be able just plainly see how the cleaned up stack trace looks like
Problem
Users reported excessive framework noise in Slack error alerts, making debugging difficult. Issue#2097 shows stack traces cluttered with trigger.dev internals like:
_RunTimelineMetricsAPI.measureMetricConsoleInterceptor.intercepttaskExecutor.tsinternal callsSolution
Applied existing correctErrorStackTrace filtering to
BUILT_IN_ERRORcase in createJsonErrorObject - it was the only error type returning raw stack traces without filtering.Changes:
enhancedError.stackTracewith correctErrorStackTrace() for filtering stack tracesAsyncLocalStoragepattern toLINES_TO_IGNOREto remove OpenTelemetry noiseImpact
Cleaner error experiences across all user-facing error contexts usingcreateJsonErrorObject:
Closes#2097
✅ Checklist
Testing
Changelog
Improve user-facing error readability by filtering framework noise from stack traces
Screenshots
N/A