fix(dev): cache sourcemap consumer per bundle in error handler - #4454
fix(dev): cache sourcemap consumer per bundle in error handler#4454danielroe wants to merge 2 commits into
Conversation
|
@danielroe is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughDevelopment stack-trace source-map loading now reuses lazily created consumers per map file, passes the cache through parsing, cleans up consumers afterward, and falls back to original frames when decoding fails. ChangesDevelopment source-map loading
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/runtime/internal/error/dev.ts (1)
141-141: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSilent failures with no warning on sourcemap decode/lookup errors.
Both the
readFile(...).catch(() => {})fallback inloadConsumerand the outercatch {}insourceLoader(lines 176-179) swallow genuine decode/lookup failures (as opposed to the expected "no.mapfile" case) without any log, leaving no trail when sourcemap enhancement silently stops working.As per coding guidelines,
src/**/*.{ts,js}: "Prefer explicit errors over silent failures; use warnings for recoverable situations; throw for invalid states."🩹 Proposed fix: warn on unexpected decode/lookup failures
} catch { - // Some bundler-generated maps trip the source-map wasm decoder; degrade to - // the un-enhanced frame instead of throwing for every frame. + // Some bundler-generated maps trip the source-map wasm decoder; degrade to + // the un-enhanced frame instead of throwing for every frame. + console.warn(`[nitro] Failed to resolve source map for ${bundleFileName}`); }Also applies to: 176-179
🤖 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 `@src/runtime/internal/error/dev.ts` at line 141, Update loadConsumer and sourceLoader to distinguish an absent sourcemap file from decode or lookup failures: retain the fallback for expected missing .map files, but emit a warning through the existing logging mechanism when read, decode, or lookup operations fail unexpectedly. Replace the empty catch blocks around rawSourceMap loading and the outer sourceLoader handling without altering successful sourcemap enhancement behavior.Source: Coding guidelines
🤖 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 `@src/runtime/internal/error/dev.ts`:
- Line 171: Update the path resolution in the frame-processing logic around
frame.fileName to use the corresponding pathe path utilities instead of
node:path, while preserving the existing dirname and resolve behavior.
- Around line 132-150: Update loadConsumer so SourceMapConsumer is initialized
with the appropriate wasm URL before constructing it from rawSourceMap. Keep the
initialization within the existing async loading flow and ensure initialization
failures remain handled consistently without silently skipping valid source-map
processing.
---
Nitpick comments:
In `@src/runtime/internal/error/dev.ts`:
- Line 141: Update loadConsumer and sourceLoader to distinguish an absent
sourcemap file from decode or lookup failures: retain the fallback for expected
missing .map files, but emit a warning through the existing logging mechanism
when read, decode, or lookup operations fail unexpectedly. Replace the empty
catch blocks around rawSourceMap loading and the outer sourceLoader handling
without altering successful sourcemap enhancement behavior.
🪄 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: CHILL
Plan: Pro
Run ID: 796a8533-d5e0-4ab2-a7b9-fccd474db78c
📒 Files selected for processing (1)
src/runtime/internal/error/dev.ts
1 participant
🔗 Linked issue
❓ Type of change
📚 Description
spotted this while investigating a Nuxt rspack dev CI failure in nuxt/nuxt#35751 - rendering any SSR error page either hung for seconds or flooded the log with:
in nitro, the dev error handler's
sourceLoaderbuilds a freshSourceMapConsumerfor every stack frame: youch-core caches byframe.fileName, but we mutate it to the original source before returning, so every app frame comes back in as the same bundle path and misses the cache.this is particularly painful for rspaack because it's one big file and one multi-MB
.mjs.mapre-parsed dozens of times per error additionally,source-map@0.7's wasm consumer throwsunreachableon some rspack maps, re-thrown per frame with no try/catch.vite is unaffected since it serves native ESM.
this PR caches the consumer per
.mapfile keyed on the bundle path so it's parsed once per request, wraps the parse and lookup so a map that trips the wasm decoder degrades to the un-enhanced frame instead of throwing, and destroys the consumers once the pass is done.📝 Checklist