fix: dedupe Next captureOutput logs and map dev stacks to source#381
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Thank you for following the naming conventions! 🙏 |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR fixes duplicate terminal output in Next.js ChangesOutput handling and error stack improvements
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
commit: |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/fix-capture-output-duplicates.md:
- Line 5: Update the changelog wording to state that "node: frames are filtered
during stack enrichment, not during snippet preview rendering" and clarify that
the "preview skip logic only excludes paths matching .next, .nuxt, and .output";
reference the existing phrases "stack enrichment", "snippet preview rendering",
and "preview skip logic" when making the change so the sentence replaces the
current ambiguous wording accordingly.
In `@packages/evlog/src/next/enrich-error-stack.ts`:
- Around line 1-13: Add a JSDoc comment for the exported function
enrichNextErrorStackForDev describing its purpose, parameters, and return type;
place it immediately above the function declaration and document that it
enriches stack traces for Next.js dev only (no-op in production), annotate the
options param (pretty?: boolean) and that the function returns a Promise<void>,
and mention any side effects (dynamic import of
../shared/enrich-error-stack-next.node and calling enrichErrorStackFromNextDev).
In `@packages/evlog/src/shared/enrich-error-stack-next.node.ts`:
- Around line 55-61: There are duplicate isFrameworkRuntimePath implementations;
extract the logic into a single shared utility module (e.g.,
shared/framework-paths.ts) that exports isFrameworkRuntimePath and reuse it from
both enrich-error-stack-next.node.ts and pretty-error.ts by replacing their
local implementations with an import; ensure the exported function preserves the
same normalization and checks for 'route-modules/', 'webpack://next/',
'/next/dist/', and '/compiled/next-server/' so both files call the shared
isFrameworkRuntimePath symbol instead of having duplicated code.
- Around line 63-69: In shouldSkipMappedSource remove the redundant node_modules
check: the current normalized.includes('node_modules') already covers
normalized.includes('/node_modules/'), so delete the latter condition and keep
the checks for '/packages/evlog/' and isFrameworkRuntimePath(normalized) to
preserve intent; update the return expression in shouldSkipMappedSource
accordingly.
In `@packages/evlog/src/shared/pretty-error.ts`:
- Around line 255-266: The isFrameworkRuntimePath function is duplicated;
extract it into a shared utility (e.g., create a helper module exporting
isFrameworkRuntimePath) and update both isFrameworkRuntimePath and
isFrameworkRuntimeFrame in this file to import and call the shared function
instead of keeping the local copy; remove the local duplicate and ensure
enrich-error-stack-next.node.ts also imports the same helper to avoid divergence
while keeping the isFrameworkRuntimeFrame (which calls decodeFileUrl and the
shared isFrameworkRuntimePath) behavior unchanged.
In `@packages/evlog/test/core/enrich-error-stack-next.test.ts`:
- Around line 28-30: The test currently silently returns when the source map
fixture is missing (the if (!existsSync(`${chunk}.map`)) { return } branch),
which masks failures; update this to fail loudly by replacing the early return
with an explicit test failure (e.g., throw new Error or use an assertion like
expect(false).toBeTruthy()) that includes the missing fixture name
(`${chunk}.map`) so CI fails and developers see which artifact is absent; keep
the existence check using existsSync and the same `chunk` variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e323bcbf-19ba-4bf5-8305-1cba504fc6dc
📒 Files selected for processing (12)
.changeset/fix-capture-output-duplicates.mdapps/docs/content/3.integrate/frameworks/02.nextjs.mdpackages/evlog/src/logger.tspackages/evlog/src/next/enrich-error-stack.tspackages/evlog/src/next/handler.tspackages/evlog/src/next/instrumentation-create.tspackages/evlog/src/shared/enrich-error-stack-next.node.tspackages/evlog/src/shared/enrich-error-stack.node.tspackages/evlog/src/shared/pretty-error.tspackages/evlog/test/core/enrich-error-stack-next.test.tspackages/evlog/test/core/pretty-error.test.tspackages/evlog/test/next/instrumentation.test.ts
| "evlog": patch | ||
| --- | ||
|
|
||
| Fix duplicate terminal output when Next.js `captureOutput` is enabled: pretty-print writes use the native stdout handle registered at patch time, passthrough is skipped unless `silent: true`, and evlog-formatted terminal lines are ignored by capture. Next.js dev stacks are source-mapped to original TypeScript (like Nitro) via a Next-only enricher that does not bundle nitropack/youch; stored stacks are compacted and useless `.next`/`node:` snippet previews are skipped. The primary `at` line now points at your route/handler file instead of Next `route-modules` internals. |
There was a problem hiding this comment.
Tighten the stack/snippet wording.
node: frames are filtered during stack enrichment, not snippet preview rendering; the preview skip logic only excludes .next, .nuxt, and .output paths.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.changeset/fix-capture-output-duplicates.md at line 5, Update the changelog
wording to state that "node: frames are filtered during stack enrichment, not
during snippet preview rendering" and clarify that the "preview skip logic only
excludes paths matching .next, .nuxt, and .output"; reference the existing
phrases "stack enrichment", "snippet preview rendering", and "preview skip
logic" when making the change so the sentence replaces the current ambiguous
wording accordingly.
| /** | ||
| * Source-map stack enrichment for Next.js dev — isolated from Nitro to avoid bundling nitropack/youch. | ||
| */ | ||
| export async function enrichNextErrorStackForDev( | ||
| error: Error, | ||
| options: { pretty?: boolean } = {}, | ||
| ): Promise<void> { | ||
| if (process.env.NODE_ENV === 'production') return | ||
| if (options.pretty === false) return | ||
|
|
||
| const { enrichErrorStackFromNextDev } = await import('../shared/enrich-error-stack-next.node') | ||
| enrichErrorStackFromNextDev(error) | ||
| } |
There was a problem hiding this comment.
Missing JSDoc on public API.
Per coding guidelines, public APIs in packages/evlog/src/**/*.{ts,tsx} require JSDoc comments. The exported enrichNextErrorStackForDev function lacks documentation.
📝 Suggested JSDoc
-/**
- * Source-map stack enrichment for Next.js dev — isolated from Nitro to avoid bundling nitropack/youch.
- */
+/**
+ * Rewrite `error.stack` with source-mapped frames in Next.js dev.
+ * Isolated from Nitro to avoid bundling nitropack/youch.
+ *
+ * `@param` error - The error whose stack should be enriched.
+ * `@param` options - Configuration options.
+ * `@param` options.pretty - When false, skip enrichment. Defaults to true in dev.
+ */
export async function enrichNextErrorStackForDev(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/evlog/src/next/enrich-error-stack.ts` around lines 1 - 13, Add a
JSDoc comment for the exported function enrichNextErrorStackForDev describing
its purpose, parameters, and return type; place it immediately above the
function declaration and document that it enriches stack traces for Next.js dev
only (no-op in production), annotate the options param (pretty?: boolean) and
that the function returns a Promise<void>, and mention any side effects (dynamic
import of ../shared/enrich-error-stack-next.node and calling
enrichErrorStackFromNextDev).
Source: Coding guidelines
- Revert shared/enrich-error-stack.node.ts to main: the Next branch and createRequire refactor were dead code (file is only imported by the Nitro plugins, where NEXT_RUNTIME is never set) - Only compact stored stacks in dev: compactStackForStorage filtered out .output/.next/node_modules frames, which would have reduced production stacks to the message line in drains; it now also returns the stack unchanged when no app frame survives - Drop the evlog-format regex from DEFAULT_CAPTURE_OUTPUT_IGNORE: the native stdout bypass already prevents recapture, and the regex silently swallowed app lines starting with INFO [ etc. - Deduplicate isFrameworkRuntimePath and EvlogProcessOutputGlobal, inline computeErrorName - Replace the enricher test that depended on a local playground build artifact with a synthetic chunk + sourcemap fixture that runs in CI https://claude.ai/code/session_01M4hK5tKJCvXKZDBpa93G15
…into fix/next-capture-output-stack-enrichment
…ut-stack-enrichment
Summary by CodeRabbit
Release Notes
Bug Fixes
captureOutputis enabled in Next.js.New Features
silentoption to control whether stdout is passed through during output capture.