-
-
Notifications
You must be signed in to change notification settings - Fork 988
fix(core): delegate to original console in ConsoleInterceptor to preserve log chain (#2900) #2991
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 our terms of service and privacy 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(core): delegate to original console in ConsoleInterceptor to preserve log chain (#2900) #2991
Conversation
…t build server failures (triggerdotdev#2913)
🦋 Changeset detectedLatest commit: 93aa053 The changes in this PR will be included in the next version bump. 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 |
WalkthroughThe PR introduces two patch fixes and supporting improvements. First, the ConsoleInterceptor in Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
🧰 Additional context used📓 Path-based instructions (9)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.ts📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}📄 CodeRabbit inference engine (AGENTS.md)
Files:
{packages,integrations}/**/*📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{test,spec}.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.test.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.test.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (13)📓 Common learnings📚 Learning: 2026-01-15T11:50:06.067ZApplied to files:
📚 Learning: 2025-11-27T16:26:44.496ZApplied to files:
📚 Learning: 2025-11-27T16:26:37.432ZApplied to files:
📚 Learning: 2025-11-27T16:27:35.304ZApplied to files:
📚 Learning: 2026-01-15T11:50:06.067ZApplied to files:
📚 Learning: 2025-11-27T16:26:37.432ZApplied to files:
📚 Learning: 2025-11-27T16:26:58.661ZApplied to files:
📚 Learning: 2025-11-27T16:27:35.304ZApplied to files:
📚 Learning: 2025-11-27T16:27:35.304ZApplied to files:
📚 Learning: 2026-01-15T11:50:06.067ZApplied to files:
📚 Learning: 2025-11-14T19:24:39.536ZApplied to files:
📚 Learning: 2025-11-27T16:27:35.304ZApplied to files:
🧬 Code graph analysis (2)packages/cli-v3/src/commands/update.test.ts (1)
packages/cli-v3/src/commands/deploy.ts (1)
🔇 Additional comments (8)
✏️ Tip: You can disable this entire section by setting 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 |
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.
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/v3/consoleInterceptor.ts (1)
67-113:⚠️ Potential issue | 🟠 MajorPass method name to
#handleLogto preserve method-specific console patches.Both
log()andinfo()useSeverityNumber.INFO, but the switch in#handleLogalways delegates tooriginalConsole.log(...)at line 99. This bypasses any patches onconsole.infothat may have been registered before interception. Pass the method name ("log", "info", "warn", "error", "debug") to#handleLogand delegate tooriginalConsole[method](...args)to preserve method-specific behavior.Suggested method-aware delegation
- debug(...args: unknown[]): void { - this.#handleLog(SeverityNumber.DEBUG, this.#getTimestampInHrTime(), "Debug", ...args); - } + debug(...args: unknown[]): void { + this.#handleLog("debug", SeverityNumber.DEBUG, this.#getTimestampInHrTime(), "Debug", ...args); + } - log(...args: unknown[]): void { - this.#handleLog(SeverityNumber.INFO, this.#getTimestampInHrTime(), "Log", ...args); - } + log(...args: unknown[]): void { + this.#handleLog("log", SeverityNumber.INFO, this.#getTimestampInHrTime(), "Log", ...args); + } - info(...args: unknown[]): void { - this.#handleLog(SeverityNumber.INFO, this.#getTimestampInHrTime(), "Info", ...args); - } + info(...args: unknown[]): void { + this.#handleLog("info", SeverityNumber.INFO, this.#getTimestampInHrTime(), "Info", ...args); + } - warn(...args: unknown[]): void { - this.#handleLog(SeverityNumber.WARN, this.#getTimestampInHrTime(), "Warn", ...args); - } + warn(...args: unknown[]): void { + this.#handleLog("warn", SeverityNumber.WARN, this.#getTimestampInHrTime(), "Warn", ...args); + } - error(...args: unknown[]): void { - this.#handleLog(SeverityNumber.ERROR, this.#getTimestampInHrTime(), "Error", ...args); - } + error(...args: unknown[]): void { + this.#handleLog("error", SeverityNumber.ERROR, this.#getTimestampInHrTime(), "Error", ...args); + } `#handleLog`( + method: "log" | "info" | "warn" | "error" | "debug", severityNumber: SeverityNumber, timestamp: ClockTime, severityText: string, ...args: unknown[] ): void { const body = util.format(...args); if (this.sendToStdIO) { if (this.originalConsole) { - switch (severityNumber) { - case SeverityNumber.INFO: - this.originalConsole.log(...args); - break; - case SeverityNumber.WARN: - this.originalConsole.warn(...args); - break; - case SeverityNumber.ERROR: - this.originalConsole.error(...args); - break; - case SeverityNumber.DEBUG: - this.originalConsole.debug(...args); - break; - default: - this.originalConsole.log(...args); - break; - } + this.originalConsole[method](...args); } else { if (severityNumber === SeverityNumber.ERROR) { process.stderr.write(body + "\n"); } else { process.stdout.write(body + "\n"); } } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/fix-console-interceptor-2900.md.changeset/fix-github-install-node-version-2913.mdpackages/cli-v3/src/commands/deploy.tspackages/cli-v3/src/commands/update.test.tspackages/cli-v3/src/commands/update.tspackages/core/src/v3/consoleInterceptor.ts
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Always import tasks from@trigger.dev/sdk, never use@trigger.dev/sdk/v3or deprecatedclient.defineJobpattern
Every Trigger.dev task must be exported and have a uniqueidproperty with no timeouts in the run function
Files:
packages/core/src/v3/consoleInterceptor.tspackages/cli-v3/src/commands/update.test.tspackages/cli-v3/src/commands/deploy.tspackages/cli-v3/src/commands/update.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
packages/core/src/v3/consoleInterceptor.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Import from
@trigger.dev/coreusing subpaths only, never import from root
Files:
packages/core/src/v3/consoleInterceptor.tspackages/cli-v3/src/commands/update.test.tspackages/cli-v3/src/commands/deploy.tspackages/cli-v3/src/commands/update.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
packages/core/src/v3/consoleInterceptor.tspackages/cli-v3/src/commands/update.test.tspackages/cli-v3/src/commands/deploy.tspackages/cli-v3/src/commands/update.ts
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier before committing
Files:
packages/core/src/v3/consoleInterceptor.tspackages/cli-v3/src/commands/update.test.tspackages/cli-v3/src/commands/deploy.tspackages/cli-v3/src/commands/update.ts
{packages,integrations}/**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Add a changeset when modifying any public package in
packages/*orintegrations/*usingpnpm run changeset:add
Files:
packages/core/src/v3/consoleInterceptor.tspackages/cli-v3/src/commands/update.test.tspackages/cli-v3/src/commands/deploy.tspackages/cli-v3/src/commands/update.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use vitest for all tests in the Trigger.dev repository
Files:
packages/cli-v3/src/commands/update.test.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.test.{ts,tsx,js,jsx}: Test files should live beside the files under test and use descriptivedescribeanditblocks
Tests should avoid mocks or stubs and use the helpers from@internal/testcontainerswhen Redis or Postgres are needed
Use vitest for running unit tests
**/*.test.{ts,tsx,js,jsx}: Use vitest exclusively for testing and never mock anything - use testcontainers instead
Place test files next to source files with naming pattern: source file (e.g.,MyService.ts) →MyService.test.ts
Files:
packages/cli-v3/src/commands/update.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use testcontainers helpers (
redisTest,postgresTest,containerTest) from@internal/testcontainersfor Redis/PostgreSQL testing instead of mocks
Files:
packages/cli-v3/src/commands/update.test.ts
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use logger methods (debug, log, info, warn, error) from `trigger.dev/sdk/v3` for structured logging in tasks
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger.config.ts : Use build extensions in trigger.config.ts (additionalFiles, additionalPackages, aptGet, prismaExtension, etc.) to customize the build
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2593
File: packages/core/src/v3/workers/warmStartClient.ts:168-170
Timestamp: 2025-10-08T11:48:12.327Z
Learning: The trigger.dev runners execute only in Node 21 and 22 environments, so modern Node.js APIs like AbortSignal.any (introduced in v20.3.0) are supported.
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
📚 Learning: 2026-01-15T11:50:06.067Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.067Z
Learning: Applies to {packages,integrations}/**/* : Add a changeset when modifying any public package in `packages/*` or `integrations/*` using `pnpm run changeset:add`
Applied to files:
.changeset/fix-console-interceptor-2900.mdpackages/cli-v3/src/commands/update.test.ts.changeset/fix-github-install-node-version-2913.md
📚 Learning: 2025-11-27T16:26:44.496Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/executing-commands.mdc:0-0
Timestamp: 2025-11-27T16:26:44.496Z
Learning: For running tests, navigate into the package directory and run `pnpm run test --run` to enable single-file test execution (e.g., `pnpm run test ./src/engine/tests/ttl.test.ts --run`)
Applied to files:
packages/cli-v3/src/commands/update.test.ts
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Use vitest for all tests in the Trigger.dev repository
Applied to files:
packages/cli-v3/src/commands/update.test.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger.config.ts : Use build extensions in trigger.config.ts (additionalFiles, additionalPackages, aptGet, prismaExtension, etc.) to customize the build
Applied to files:
packages/cli-v3/src/commands/update.test.tspackages/cli-v3/src/commands/deploy.ts.changeset/fix-github-install-node-version-2913.md
📚 Learning: 2026-01-15T11:50:06.067Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.067Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Use vitest exclusively for testing and never mock anything - use testcontainers instead
Applied to files:
packages/cli-v3/src/commands/update.test.ts
📚 Learning: 2025-11-27T16:26:37.432Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T16:26:37.432Z
Learning: Applies to packages/trigger-sdk/**/*.{ts,tsx} : In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code
Applied to files:
packages/cli-v3/src/commands/update.test.ts
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/**/*.{ts,tsx} : Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Applied to files:
packages/cli-v3/src/commands/deploy.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Export tasks with unique IDs within the project to enable proper task discovery and execution
Applied to files:
packages/cli-v3/src/commands/deploy.ts
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger.config.ts : Configure Trigger.dev project in `trigger.config.ts` using `defineConfig()` with project ref and task directories
Applied to files:
packages/cli-v3/src/commands/deploy.ts
📚 Learning: 2026-01-15T11:50:06.067Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-15T11:50:06.067Z
Learning: Default to patch version for changesets for bug fixes and minor changes; confirm with maintainers before selecting minor or major
Applied to files:
.changeset/fix-github-install-node-version-2913.md
📚 Learning: 2025-11-14T19:24:39.536Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 2685
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.settings/route.tsx:1234-1257
Timestamp: 2025-11-14T19:24:39.536Z
Learning: In the trigger.dev project, version validation for the `useNativeBuildServer` setting cannot be performed at the settings form level because the SDK version is only known at build/deployment time, not when saving project settings.
Applied to files:
.changeset/fix-github-install-node-version-2913.md
📚 Learning: 2025-11-27T16:27:35.304Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-11-27T16:27:35.304Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use `trigger.dev/sdk/v3` for all imports in Trigger.dev tasks
Applied to files:
.changeset/fix-github-install-node-version-2913.md
🧬 Code graph analysis (2)
packages/cli-v3/src/commands/update.test.ts (1)
packages/cli-v3/src/commands/update.ts (1)
updateTriggerPackages(51-304)
packages/cli-v3/src/commands/deploy.ts (1)
packages/cli-v3/src/commands/update.ts (1)
updateTriggerPackages(51-304)
🔇 Additional comments (8)
.changeset/fix-github-install-node-version-2913.md (1)
1-5: Changeset reads correct for a patch bugfix..changeset/fix-console-interceptor-2900.md (1)
1-5: Changeset looks good for the fix scope.packages/cli-v3/src/commands/update.ts (2)
18-22: Nice: option is plumbed into the schema.
261-281: The--ignore-enginesflag for Yarn is incompatible with Yarn v4+ (modern/Berry).The flag is only supported in Yarn Classic (v1); Yarn v4+ does not recognize it. The code needs to detect the Yarn version and handle v4+ differently—either use a supported option or skip the flag entirely. The repo's test fixtures use Yarn 4.2.2, so this will fail in practice.
Update the
yarncase to check the detected version and adjust behavior accordingly, or consider whether--ignore-enginesis the right approach for modern Yarn.packages/cli-v3/src/commands/deploy.ts (2)
255-255: Good: deploy now skips engine checks during update flow.
492-493: Consistent URL/link and message formatting looks clean.Also applies to: 709-712, 734-735, 742-743, 786-788, 1086-1087, 1153-1155, 1200-1203, 1236-1238, 1251-1253, 1277-1284, 1296-1297, 1302-1304, 1315-1316, 1321-1323, 1334-1335, 1340-1342, 1359-1366
packages/core/src/v3/consoleInterceptor.ts (1)
18-63: The re-entrancy concern is not applicable to this codebase. Task execution is sequential with explicit guards:
- The worker processes one task at a time (checked with
if (_isRunning))intercept()is called exactly once per task execution (line 112 in taskExecutor.ts)- The callback wraps the entire task lifecycle and does not recursively trigger execution
- The execution environment is reset between tasks
No nested or concurrent
intercept()calls occur in practice. The current implementation is safe without additional depth tracking.Likely an incorrect or invalid review comment.
packages/cli-v3/src/commands/update.test.ts (1)
1-113: The test file correctly uses mocks for external dependencies and CLI utilities. Testcontainers is specifically for stateful services like Redis and PostgreSQL, not for mocking utility libraries likenypm,pkg-types, or file system operations. This pattern is already established throughout the codebase (e.g.,apps/webapp/test/extensively mocks internal services anddb.server). No changes needed.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Summary
Fixes #2900
Problem
When using Sentry (or other libraries that patch
consolemethods) indevmode, logs can be swallowed. This happens becauseConsoleInterceptoroverridesconsolemethods with its own implementation that writes directly toprocess.stdout/stderr, bypassing any previous patches (like Sentry's breadcrumb capture or upstream transport). Additionally, if Sentry patchesconsoleafterConsoleInterceptorstarts interception, the restoration logic inConsoleInterceptormight conflict or break compatibility.Fix
ConsoleInterceptor.tsto store the original console methods (those present before interception starts) in the class instance.#handleLogto delegate to theseoriginalConsolemethods (e.g.this.originalConsole.log(...)) instead of writing directly toprocess.stdout.process.std[out|err]is retained iforiginalConsoleis somehow missing (though unlikely during interception).Verification