Fix excessive framework noise in error stack traces #2442
Conversation
🦋 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 |
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 with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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, | ||
| }), | ||
| }; |
There was a problem hiding this comment.
💡 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 directly
rg -nP --type=ts --type=tsx -C2 '\bcreateJsonErrorObject\s*\('
rg -nP --type=ts --type=tsx -C2 '\bstackTrace\b' apps packagesLength of output: 185
🏁 Script executed:
#!/bin/bash
# Rerun search for createJsonErrorObject across TS and TSX files in apps and packages
rg -n 'createJsonErrorObject\(' -C2 -g '*.{ts,tsx}' apps packages
# Rerun search for direct stackTrace property access in apps and packages
rg -n '\.stackTrace\b' -C2 -g '*.{ts,tsx}' apps packagesLength of output: 12404
Adjust downstream consumers for new stackTrace format
We’ve confirmed that several formatting routines rely on the first line of stackTrace being the "Error: …" header. Since we now strip that line by default, these consumers need to be updated to use the separate name/message fields rather than parsing stackTrace 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 wraps error.stackTrace ?? error.message in a code block. To preserve context, consider rendering Error: ${error.name}: ${error.message} separately and then the trace.
• apps/webapp/app/routes/…/route.tsx (around line 911)
UI renders enhancedError.stackTrace without the header—verify that the header is shown elsewhere (e.g. via error.name + error.message).
Please update these formatters/UI components to reference the dedicated error fields instead of assuming the header lives in stackTrace.
There was a problem hiding this comment.
@omkardongre Can you update this change to set removeFirstLine: false that way we don't break these expectations?
There was a problem hiding this comment.
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.
@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 createJsonErrorObject Add test for framework stack trace filtering Add changeset This change improves readability of error traces in user-facing contexts (Slack alerts, email notifications, API responses, run streams) by filtering out internal framework noise that clutters error messages.
omkardongre
force-pushed
the
fix/slack-alert-stack-trace-noise
branch
from
cf9be9f to
e16f893
Compare
There was a problem hiding this comment.
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.
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 using createJsonErrorObject:
Closes #2097
✅ Checklist
Testing
Changelog
Improve user-facing error readability by filtering framework noise from stack traces
Screenshots
N/A