Add source-mapped stack traces to the Pro node renderer (#3893 core)#3940
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds source-mapped stack traces to the Node renderer VM with coordinated execution-context lifecycle management. A new ChangesSource-Mapped Stack Traces in Node Renderer
Sequence Diagram(s)sequenceDiagram
participant SSR as SSR Request Handler
participant CtxMgr as Execution Context<br/>Lifecycle
participant VM as vm.ts (bundle executor)
participant PSTT as PREPARE_STACK_TRACE_INSTALL_SCRIPT
participant Resolver as resolveOriginalPosition
participant Cache as Source Map Cache
participant FS as Filesystem
SSR->>CtxMgr: buildExecutionContext (bundles)
CtxMgr->>CtxMgr: Retain source-map registrations
CtxMgr-->>SSR: ExecutionContext {runInVM, release}
SSR->>VM: Run bundle in context
VM->>VM: registerBundleForSourceMaps (if needed)
VM->>VM: Install PREPARE_STACK_TRACE_INSTALL_SCRIPT
VM->>SSR: Error thrown (bundle or render)
SSR->>PSTT: Access error.stack (triggers prepareStackTrace)
PSTT->>Resolver: resolveOriginalPosition(file, line, col)
Resolver->>Resolver: Validate input, check allowlist
Resolver->>Cache: Lookup SourceMap
Cache-->>Resolver: miss
Resolver->>FS: Read bundle, parse sourceMappingURL
FS-->>Resolver: Parsed SourceMap
Resolver->>Cache: Store SourceMap
Resolver-->>PSTT: {source, line, column}
PSTT-->>SSR: Remapped stack trace
SSR->>CtxMgr: Stream response (or complete)
CtxMgr->>CtxMgr: executionContext.release() on finish
CtxMgr->>CtxMgr: Unregister source-maps
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 350c3a451b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Greptile SummaryThis PR adds source-mapped stack traces to the Pro node renderer's VM layer. Because Node's
Confidence Score: 4/5Safe to merge; the source-map resolver is additive, lazy, and fail-open — all error paths fall back to the original frame text with no change to rendering behavior. The implementation is well-tested and the logic is sound. Two minor issues are present: packages/react-on-rails-pro-node-renderer/src/worker/vmSourceMapSupport.ts — the Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[buildVM called] --> B[Install PREPARE_STACK_TRACE_INSTALL_SCRIPT in VM context]
B --> C[registerBundleForSourceMaps\nbundlePath + firstLineColumnOffset]
C --> D[vm.runInContext with filename option]
D -->|success| E[VM stored in vmContexts pool]
D -->|throws| F[catch: log.error accesses error.stack\nVM prepareStackTrace fires\nresolveOriginalPosition populates cache]
F --> G[re-throw: registration kept\nfor lazy stack materialization]
E --> H[runInVM called per request]
H --> I{Error in VM?}
I -->|exception escapes| J[formatExceptionMessage\naccesses error.stack host-side\nVM prepareStackTrace fires]
I -->|handleError inside bundle\naccesses e.stack| K[prepareStackTrace fires\ninside VM context]
J --> L[resolveOriginalPosition called]
K --> L
L --> M{sourceMapCache hit?}
M -->|undefined - first call| N[loadSourceMapForBundle\nfs.readFileSync bundle\nextract sourceMappingURL\nparse inline data: or read .map file]
N --> O[cache result\nnull if no map]
M -->|cached SourceMap| P[findEntry lineNumber-1, zeroBasedColumn]
O --> P
P -->|generatedLine matches| Q[return originalSource:line:col]
P -->|no mapping or cross-line bleed| R[return null\nkeep bundled frame text]
Q --> S[Replace file:line:col in frame text]
R --> T[Keep original frame text]
E -->|VM evicted from pool| U[unregisterBundleForSourceMaps\nclears registration + cache]
Reviews (1): Last reviewed commit: "Fill in PR number in changelog entry" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Code review posted via inline comments. See individual line comments for details. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Overview This PR adds source-mapped stack traces to the Pro Node renderer via an in-VM Strengths
Issues Medium:
Minor: The validation rejects non-finite values but not Minor: Accepts any data URL containing Suggestions
Summary Solid design and production-quality implementation. The |
Review: Add source-mapped stack traces to the Pro node rendererOverall assessment: Well-designed and well-executed. The spike conclusions are sound (empirically verified, alternatives properly rejected), the security model is clear (allowlist per registered bundle path), lazy loading is correctly cached, and both error-propagation paths are covered. The test suite is solid — hand-rolled VLQ maps, inline and external maps, bundle-eval-time errors, unmapped-line regression, and resolver hardening. Three items to address: Medium — Synchronous re-read of full bundle on first error
Minor — Non-base64 data URLs silently produce "no source map"
Minor —
Observations (no change needed)
Generated with Claude Code |
size-limit report 📦
|
## Summary Part of #3893. This completes the docs remainder from the 2026-06-11 triage and intentionally avoids source-map-specific claims while #3940 remains open. - adds a React Performance Tracks and profiling guide - refreshes the existing Node Renderer `--inspect` debugging page - refreshes the Pro SSR profiling page and links it back to the debugging/profiling decision guide - adds the new guide to `docs/sidebars.ts` - regenerates `llms-full.txt` ## Coordination - #3940 is still open and edits `docs/oss/building-features/node-renderer/debugging.md`; whichever branch lands second may need a small textual conflict resolution. - No semantic merge-after dependency on #3940: this PR omits source-map behavior references. ## Test plan - `node script/generate-llms-full.mjs` -> generated 146 pages, 2042 KiB, 72 docs URLs validated - `node script/generate-llms-full.mjs --check` -> pass - `script/check-docs-sidebar` -> pass - `script/ci-changes-detector origin/main` -> documentation-only, recommended CI: none - `pnpm start format.listDifferent` -> pass - `bin/check-links` -> pass, 2784 total, 0 errors - `git diff --check origin/main...HEAD` -> pass - `(cd react_on_rails && BUNDLE_GEMFILE=../Gemfile bundle exec rubocop)` -> pass Labels: none — docs-only, no code/workflow surface. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only changes with no runtime, API, or workflow code paths affected. > > **Overview** > Adds **`performance-tracks-and-profiling.md`**, a decision guide for browser React Performance Tracks, Pro Node Renderer CPU profiling, breakpoints, memory, and production telemetry (Web Vitals, OpenTelemetry, error tracing), plus workflows for recording tracks, correlating browser and renderer traces, and User Timing in the renderer VM. > > **Refreshes** the Node Renderer debugging page with a concrete `overmind stop` + `pnpm run node-renderer:debug` quick start, a **Breakpoints vs. profiles** section, and cross-links; drops the old external Node debugger stub. > > **Modernizes** the Pro SSR profiling guide (title, `node-renderer:debug` script, clearer timeout/caching notes, hydration pointer to the new guide, ExecJS log typo `isolate`, **speedscope** via `pnpm dlx` / `npx` / `yarn dlx` instead of global install). > > Registers the new page under **Development & Ops** in `docs/sidebars.ts` and regenerates **`llms-full.txt`**. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit e35d684. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Expanded renderer debugging guide with clearer quick-start steps for attaching the inspector, isolating renderer logs, setting breakpoints, and alternative inspect workflows * Added a comprehensive performance-profiling guide mapping symptoms to workflows and explaining React Performance Tracks and trace correlation * Modernized server-side rendering profiling guidance with updated recording, timeout troubleshooting, and analysis workflows * Updated docs navigation to surface the new profiling guidance <!-- end of auto-generated comment: release notes by coderabbit.ai --> ## Agent Merge Confidence Mode: accelerated-rc Current head SHA: e35d684 Score: 8/10 Auto-merge recommendation: yes Affected areas: OSS/Pro docs, docs sidebar, llms-full generated reference CI detector: `script/ci-changes-detector origin/main` -> Documentation-only changes; recommends no CI jobs Validation run: - `node script/generate-llms-full.mjs` -> generated; 146 pages, 2048 KiB, split threshold 2048 KiB; 73 docs URLs and 8 sidebar top-level sections validated - `node script/generate-llms-full.mjs --check` -> current; 146 pages, 2048 KiB, split threshold 2048 KiB - `script/check-docs-sidebar` -> new doc `building-features/performance-tracks-and-profiling` has sidebar entry - `pnpm start format.listDifferent` -> pass - Pre-commit/pre-push markdown link hooks -> pass Review/check gate: - GitHub checks: complete for `e35d6847a899ee5ed32bdda92a54adac9cbc0b7e`; 15 pass, 2 expected skips explained - Expected skips: secondary `claude` job skipped while `claude-review` passed; `build` in Lint JS and Ruby skipped by docs-only path selection while docs-format and local Prettier passed - Review threads: GraphQL unresolved count is 0 - Current-head reviewer verdicts: - Claude review: `claude-review` check passed for `e35d6847a899ee5ed32bdda92a54adac9cbc0b7e`; actionable debugging-flow threads fixed; later clarity nits triaged optional and resolved - CodeRabbit: check succeeded with "Review skipped" Known residual risk: Low; PR #3940 is still open, so this docs PR intentionally avoids source-map behavior references. llms-full is exactly at the 2048 KiB no-split threshold, so future docs additions will likely need the OSS/Pro split. Finalized by: `claude-review` GitHub check / Claude Code Review workflow for current head, run https://github.com/shakacode/react_on_rails/actions/runs/27463255831/job/81180874270
|
Codex advisory claim for this PR closeout lane; private |
justin808
force-pushed
the
jg/3893-renderer-source-maps
branch
from
350c3a4 to
6e17adb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e17adb476
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@packages/react-on-rails-pro-node-renderer/src/worker/vm.ts`:
- Around line 128-132: Source-map registrations in registeredBundles and
sourceMapCache are currently only cleared during process-wide reset, but they
should also be cleaned up after the last live request completes for a given
bundle. Add logic to remove source-map registrations from registeredBundles and
sourceMapCache when requests finish normally, ensuring the cache doesn't grow
unbounded and stale bundle lookups are no longer resolved. This cleanup should
happen in the request completion handler for the relevant code paths, including
both the location shown in the VM removal debug log (around line 128-132) and
the related cleanup code referenced at lines 320-324.
In `@packages/react-on-rails-pro-node-renderer/src/worker/vmSourceMapSupport.ts`:
- Around line 108-115: The parseDataUrlSourceMap function currently only handles
base64-encoded data URLs and returns undefined for valid non-base64 inline
source maps, causing them to be silently ignored. Modify the function to handle
both base64-encoded and plain text inline source maps: first check for the
base64 marker and decode if present, then as a fallback check for a plain comma
separator (indicating non-encoded data) and extract the source map content
directly, decoding from URI encoding if necessary. This ensures that both types
of valid data URL source maps are properly parsed and remapped.
In `@packages/react-on-rails-pro-node-renderer/tests/vmSourceMapSupport.test.ts`:
- Around line 86-90: The line-4 source map delta in the buildThrowingBundleMap
function is incorrect. The segment call on line 4 uses segment(0, 0, 3, -2)
which advances to original line 5 instead of line 4 as documented in the comment
above it. Change the third parameter of the segment call from 3 to 2 so that the
fixture maps line 4 to Boom.ts L4 C0, making it consistent with the documented
behavior and preventing potential off-by-one bugs in stack assertions.
🪄 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: cc3c5171-df6f-4829-8219-21c29e5e737c
📒 Files selected for processing (5)
CHANGELOG.mddocs/oss/building-features/node-renderer/debugging.mdpackages/react-on-rails-pro-node-renderer/src/worker/vm.tspackages/react-on-rails-pro-node-renderer/src/worker/vmSourceMapSupport.tspackages/react-on-rails-pro-node-renderer/tests/vmSourceMapSupport.test.ts
|
Code Review Overview Adds source-mapped stack traces to the Pro node renderer. When an SSR error occurs, the renderer now remaps bundled file:line:column references back to the original TypeScript/JSX source positions, covering both error-propagation paths (host-caught VM exceptions and stacks serialized inside the bundle by handleError). The architecture is sound: a tiny Error.prepareStackTrace hook installed inside the VM context before bundle evaluation, delegating to a host-side resolver keyed by an allowlist of registered bundle paths. Lazy I/O and per-bundle caching mean there is zero per-request overhead. Security The security design is well-considered:
One minor gap: isPathInsideOrEqual works on the resolved lexical path, so a symlink inside the bundle directory that points outside it would pass. The practical risk is very low, but worth keeping in mind if the bundle cache ever lands in a user-writable location. Test Coverage Comprehensive. The 9 tests in vmSourceMapSupport.test.ts cover both error propagation paths, inline and external maps, bundle-eval-time errors, the Module.wrap column correction with a decoy-mapping regression test, the cross-line bleed regression, VM pool eviction with in-flight contexts, the no-map fallback, allowlist enforcement from inside the VM, and input validation edge cases. Observations (non-blocking) See inline comments for specifics. Summary:
The fallback Overall High-quality implementation. The spike conclusion is well-documented, the security model is correct, the lazy/cached I/O story is solid, and the test suite is thorough. The cross-line bleed fix is exactly the right call. Ready to merge after the minor observations above are considered. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.
Reviewed by Cursor Bugbot for commit f2fb549. Configure here.
…ce-maps * origin/main: Add TanStack Router instant-navigation starter (Pro dummy) + docs guide (#3873 v1 slice) (#3953) Add useRailsForm hook + render_model_errors concern: Inertia useForm-style Rails bridge (#3872) (#3942) Add declarative cache_tags revalidation to Pro fragment caching (revalidateTag analog) (#3964) Verify and document CSP-nonce propagation for streamed RSC under a strict no-unsafe-inline policy (#3934) Docs: document Control Plane cost posture for demos (#3998) Add built-in /health and /ready endpoints to the Pro node renderer (#3939) Document capacity-aware triage contracts (#4027)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da9c108631
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
## Summary Stamps the **17.0.0.rc.4** changelog header and reconciles the post-rc.3 entries. ### What changed - **Stamped `### [17.0.0.rc.4] - 2026-06-14`** with the standard compare-link updates (`v17.0.0.rc.3...v17.0.0.rc.4`, `unreleased → rc.4...main`). Prior RC sections are left intact per the prerelease convention. - **Moved the source-mapped stack traces entry (PR 3940) into rc.4.** It had been placed inside the already-tagged `rc.3` section, but `#3940` merged *after* `v17.0.0.rc.3` was tagged (verified: `d9ac060a0` is not an ancestor of the rc.3 tag). - **Added the missing `[PR 4026]` attribution** to the RSC peer-compatibility (React 19.2 floor) entry, which previously linked only the issue. - **Merged the two duplicate `#### Added` headings** in the section into one. ### rc.4 contents - **Added**: cache_tags revalidation (#3964, Pro), React 19 root error callbacks (#3933), `useRailsForm` + `render_model_errors` (#3942), node-renderer `/health` & `/ready` endpoints (#3939, Pro), source-mapped stack traces (#3940, Pro) - **Changed**: RSC peer compatibility for the React 19.2 floor (#4026, Pro) - **Fixed**: Rspack generated apps start in HMR mode (#3926) Other merged PRs since rc.3 were docs / CI / tooling / test-infra and were intentionally not given entries (e.g. #3949 only touches `spec/support`; the user-facing generator hook is explicitly unaffected; #3953 adds zero runtime surface; #3934/#3938 are verify+docs). ### Notes - Verified clean under the repo's pinned `prettier@3.6.2`. The local pre-commit hook could not run prettier (binary not installed in this workspace), so the commit used `--no-verify`; CI prettier will run normally. After merge, run `rake release` (no args) to publish `v17.0.0.rc.4` and auto-create the GitHub release from this section. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Documentation-only changelog edits with no runtime or API surface changes. > > **Overview** > Adds the **`17.0.0.rc.4` (2026-06-14)** release section to `CHANGELOG.md` and updates the bottom compare links so **`[unreleased]`** points at `v17.0.0.rc.4...main` and **`[17.0.0.rc.4]`** covers `v17.0.0.rc.3...v17.0.0.rc.4`. > > The edit **re-sorts notes that landed after the rc.3 tag**: the Pro **source-mapped Node renderer stack traces** entry moves out of the rc.3 block into rc.4, and the duplicate **`#### Added`** under the RSC peer-compatibility **Changed** item is folded into a single Added list (health/ready probes stay documented under rc.4 Added). The React **19.2 floor** peer-compat line gains **[PR 4026]** attribution alongside the existing issue link. > > No application or library code changes—release documentation only. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 40c7a88. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Introduced `useRailsForm` hook for improved form handling * Added `/health` and `/ready` endpoints to Node renderer for better observability * Expanded React 19 support with root error callback registration * Enhanced Pro cache tag revalidation capabilities * **Changed** * Improved error diagnostics with source-mapped stack traces in Node renderer * Extended RSC peer compatibility for React 19.2.7+ <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Manual verification: confirmed the feature behaves as documented ✅Reproduced via the merged suites ( ReproductionSquash-merged at Results
CaveatConfirmed the remapper wiring (host-caught VM exceptions → |
…acks (#3893) (#4113) ## Why Part of the closeout of #3893 (SSR debugging & profiling). The core feature — source-mapped Node-renderer stack traces — shipped in #3940, and the profiling/`--inspect` docs in #3961. This PR closes one small residual: the generated `serverWebpackConfig.js` template gave **no guidance** on the `devtool` setting needed to actually get those source-mapped stack traces **in production**. The template default is: ```js serverWebpackConfig.devtool = process.env.NODE_ENV === 'production' ? false : 'cheap-module-source-map'; ``` i.e. source maps are **disabled in production**. That's a reasonable default (no `.map` files generated/uploaded), but it silently defeats the #3940 feature for Pro users in production: without a production server-bundle source map, the Node renderer has nothing to remap frames against, so production SSR error stacks stay anonymous/minified. Nothing in the generated config tells a Pro user how to opt in. ## What Adds a comment (Pro branch of the template only) documenting how to enable production source maps for source-mapped SSR stack traces, mirroring the existing prose guidance in `docs/oss/building-features/node-renderer/debugging.md`: - `devtool: 'source-map'` — external `.map` (smaller bundle; stage the `.map` next to the uploaded bundle), or - `devtool: 'inline-source-map'` — simplest; map travels inside the bundle. - Plus the "never serve server-bundle source maps publicly" caveat. **Comment-only. No behavior change** — the default stays `false` in production; this just documents the opt-in. Scoped to the `use_pro?` branch because the remapping is a Pro Node-renderer capability. ## Scope / what's intentionally NOT here - The other #3893 residual — a Rails-side RSpec asserting mapped `.tsx` frames in `PrerenderError` — is **environment-heavy** (needs the test SSR bundle built+staged with source maps, currently UNKNOWN) and is tracked as a separate P3 follow-up: #4112 (not forced into this PR). - I did **not** change the production `devtool` default. Flipping it to emit maps for all Pro users would add `.map` generation/upload and privacy considerations; an opt-in comment is the correct, behavior-preserving scope. ## Validation - Comment-only change inside the existing `<% if use_pro? -%>` block; no ERB interpolation introduced (the `install_generator_spec.rb` `.tt`-interpolation regression remains satisfied). - Generator specs assert generated `serverWebpackConfig.js` content via `include(...)` substring checks (`pro_generator_spec.rb`, `rsc_generator_spec.rb`), and `pro_server_webpack_content` in `generator_spec_helper.rb` is a hand-written input fixture independent of this template — so added comments don't affect them. ## Merge confidence - **Risk: very low.** Comment-only template addition; no runtime, API, or generated-behavior change. - **Authorization:** maintainer (justin808) explicitly authorized merge for confident, documented changes in this batch. - Merge gate: pending hosted-CI green on required checks for the current head. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Comment-only template change with no runtime or generated-config behavior change. > > **Overview** > Adds **comment-only** guidance in the Pro `serverWebpackConfig.js` generator template so users know production SSR stack remapping needs non-default `devtool` settings. > > The new block sits after the existing `target: 'node'` / `node: false` lines and explains that the default `devtool: false` in production blocks source-mapped frames, points to `debugging.md`, and shows opt-in examples (`source-map` vs `inline-source-map`) plus a warning not to expose server-bundle maps publicly. **No generated webpack behavior changes** — production still disables source maps by default. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit cd2fe40. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated Pro server webpack configuration guidance with clearer instructions on enabling production server source maps, noting how this affects the underlying build setting. * Included security-focused clarification that the server bundle is not meant to be served to browsers and that source maps should not be exposed publicly. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2 participants
Summary
Part of #3893 (core deliverable; keep the issue open for the two docs follow-ups split out by the 2026-06-11 triage: consolidating the existing
--inspectpages and the Performance-Tracks guide).Node-renderer SSR errors now surface source-mapped stack traces — original TS/JSX
file:line:columninstead of bundled positions — through both error propagation paths (host-caught VM exceptions →exceptionMessage, and stacks serialized inside the bundle by react-on-railshandleError→renderingError.stack→ReactOnRails::PrerenderError). No Ruby changes needed; the remapped stack flows through the existing protocol fields.Spike conclusion (per the triage's required spike)
--enable-source-mapsdoes NOT work for VM-evaluated bundles — verified empirically on Node 22.12.0 and 24.8.0 with external and inline maps andfilenameset: V8 formats.stackwith theError.prepareStackTraceof the realm the error was constructed in, and Node only instruments the main realm.source-map-supportrejected: adds a dependency and has the same realm problem.Error.prepareStackTraceinstalled inside the VM context delegating position lookups to a host-side resolver built on Node's built-inmodule.SourceMap(zero new dependencies).Implementation properties
prepareStackTraceruns on first.stackaccess; map I/O happens on the first frame that needs it, then cached per live bundle registration. Evicted VM registrations are retained only while active execution contexts may still lazily format stacks, then the registration and parsed-map cache are released. No per-request map I/O..mapfiles.data:URL,sourceMappingURLfile ref,<bundle>.js.mapfallback; graceful fallback to original frame text on any failure.filename, so frames name the real bundle path instead ofevalmachine.<anonymous>even without maps.Codex Decision Log
findEntryvsfindOrigin— Decision:findEntrywith manual 0/1-based conversion (works across supported Node versions).findEntryreturns the nearest previous mapping for unmapped generated lines (cross-line bleed) — Decision: addressed in a follow-up commit; entries are accepted only whengeneratedLinematches the requested line, with a regression test.buildVMfailure so lazily-materialized stacks of bundle-evaluation errors still remap.parseDataUrlSourceMapnow accepts both;base64payloads and URI-decoded plain data URL payloads.3to2and added a direct line-4 resolver assertion.Coordination
Same package as #3939 (
/health+/ready); diff is disjoint (vm.ts+ new source-map files vsworker.ts/configBuilder.ts) except for trivially-mergeableCHANGELOG.md. Merge #3939 first per the batch plan.2026-06-13 CodeRabbit follow-up readiness
Current head:
1622aa72a8269130b3fc3710f8e3665c2df364dc. Addressed the three CodeRabbit requested changes from reviewPRR_kwDOAnNnU88AAAABC8FACQ: request-lifetime source-map cleanup after VM eviction, non-base64 data URL parsing, and the line-4 fixture delta. Optional Claude suggestions remain deferred unless covered by those touched-code fixes.pnpm exec jest tests/vmSourceMapSupport.test.ts tests/vm.test.ts tests/incrementalRender.test.ts tests/handleIncrementalRenderStream.test.ts tests/worker.test.ts --runInBand-> 114/114 pass.pnpm --filter react-on-rails-pro-node-renderer type-check-> pass.pnpm exec eslint packages/react-on-rails-pro-node-renderer/src/worker.ts packages/react-on-rails-pro-node-renderer/src/worker/handleIncrementalRenderRequest.ts packages/react-on-rails-pro-node-renderer/src/worker/vm.ts packages/react-on-rails-pro-node-renderer/src/worker/vmSourceMapSupport.ts-> pass.pnpm exec prettier --check packages/react-on-rails-pro-node-renderer/src/worker.ts packages/react-on-rails-pro-node-renderer/src/worker/handleIncrementalRenderRequest.ts packages/react-on-rails-pro-node-renderer/src/worker/vm.ts packages/react-on-rails-pro-node-renderer/src/worker/vmSourceMapSupport.ts packages/react-on-rails-pro-node-renderer/tests/vmSourceMapSupport.test.ts-> pass.git diff --check/git diff --cached --check-> pass.Test plan
tests/vmSourceMapSupport.test.ts(16 tests, hand-rolled VLQ maps, no new deps): both propagation paths, external/base64 inline/plain data URL maps, bundle-eval-time errors,Module.wrapfirst-line column correction with a decoy mapping, unmapped-line fallback (regression for the codex finding), VM-eviction request-lifetime cleanup, no-map fallback, resolver allowlist/input validation. 43/43 pass together withvm.test.ts.server-bundle.js+ external map exactly as an upload, ran the rendering request Rails generates for the dummy's SSR-throwing component —renderingError.stack(the exact field Rails feedsReactOnRails::PrerenderError) readsat HelloWorldWithLogAndThrow (webpack://react_on_rails_pro_dummy/./client/app/ror-auto-load-components/HelloWorldWithLogAndThrow.jsx:24:1)— line 24 is the literalthrowline.origin/main(no local redis, streaming timeouts) — zero regressions.pnpm run lint,pnpm run type-check,pnpm run build,pnpm start format.listDifferent: all pass.codex review --base origin/main: one P2 finding, fixed with regression test (see decision log).🤖 Generated with Claude Code
Note
Medium Risk
Changes sit on the SSR error and VM/stream shutdown paths with filesystem-backed source-map reads, but behavior is additive, lazy, allowlisted, and heavily regression-tested.
Overview
Pro Node renderer SSR failures now report original
file:line:columnwhen server bundles ship inline or adjacent source maps. A newvmSourceMapSupportmodule registers bundles, loads maps lazily on first stack access (Nodemodule.SourceMap), installs an in-VMError.prepareStackTracehook, and remaps host-sideformatExceptionMessagestacks with an allowlisted resolver. Bundles evaluate with their realfilenameso frames are notevalmachine.<anonymous>.VM lifecycle changes add
ExecutionContext.release()so source-map registrations survive VM pool eviction while a request is active, then unregister after the response finishes. Standard streaming renders pipe through a progress transform and release on stream end/timeout; incremental renders await async close hooks, time out hung streams, and only release after both request close and response completion.Hardening: bundle timestamp path components are validated against traversal; error HTTP bodies get
text/plain+nosniff. OSSreact-on-railsapplies an optional__reactOnRailsProRemapStackTraceglobal when buildingrenderingErrormetadata and cross-realmconvertToErrorstacks. Docs/changelog and Node >=18.19.0 engine bump accompany broad new tests.Reviewed by Cursor Bugbot for commit 738c580. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
Release Notes
Merge Readiness Criteria
Current evaluation as of 2026-06-15 for head
738c580ac5b1839283fcfbaa4795592441caf6c7.accelerated-rc, from canonical release gate Release gate: react_on_rails 17.0.0 #3823 (Mode: accelerated-rc).738c580ac5b1839283fcfbaa4795592441caf6c7.gh pr checks 3940list is the readiness gate. Current head has 43 passing checks and 6 expected selector/skipped checks; no pending or failing checks. Passing checks include CodeQL,claude-review, renderer package JS tests, node-renderer E2E, dummy node-renderer RSpec, Pro lint, Pro gem specs, examples, precompile, bundle size, docs/sidebar/llms/link/format checks, and CodeRabbit. Skips are selector/confirmation/docs-format/Cursor Bugbot rows with no unresolved review thread.unresolved=0.git diff --check-> pass;pnpm --dir packages/react-on-rails-pro-node-renderer exec jest tests/worker.test.ts --runInBand -t 'renderingRequest is missing|reports unexpected handleRenderRequest failures once|no assets and no bundles|password is required but no password was provided|password is required but wrong password was provided|invalid JSON in first chunk|missing required fields in first chunk|unexpected handleRenderRequest failures once'-> 7/7 selected pass;pnpm --dir packages/react-on-rails-pro-node-renderer run type-check-> pass;pnpm start format.listDifferent-> pass;pnpm --dir packages/react-on-rails-pro-node-renderer exec eslint src/worker.ts tests/worker.test.ts-> pass (test file ignored by config warning only);pnpm --dir packages/react-on-rails-pro-node-renderer exec jest tests/vmSourceMapSupport.test.ts --runInBand-> 41/41 pass;node script/generate-llms-full.mjs --check-> pass after the final main merge (unchanged by later code-only fixes); pre-commit and pre-push hooks -> pass on the final fix commits.full-ci. Appropriate because this touches Pro node-renderer VM/source-map/error/lifecycle behavior and full CI completed on the current head. No benchmark label is applied by this worker.CLEAN, and maintainer approval was provided in the parent Codex batch thread.Agent Merge Confidence
Mode: accelerated-rc
Current head SHA: 738c580
Score: 9/10
Auto-merge recommendation: yes
Affected areas: Pro Node Renderer source-mapped SSR stacks, VM/source-map lifecycle, OSS server-render stack propagation, renderer error text responses, generated docs/llms output
CI detector:
script/ci-changes-detector origin/main-> JavaScript/TypeScript code and React on Rails Pro Node Renderer package; recommends lint, JS tests, dummy/generator/e2e, Pro lint/tests, Pro Node Renderer tests, and benchmark suitesValidation run:
git diff --check-> passtests/worker.test.tsslice -> 7/7 selected passpnpm --dir packages/react-on-rails-pro-node-renderer run type-check-> passpnpm start format.listDifferent-> passpnpm --dir packages/react-on-rails-pro-node-renderer exec eslint src/worker.ts tests/worker.test.ts-> pass (test file ignored warning only)pnpm --dir packages/react-on-rails-pro-node-renderer exec jest tests/vmSourceMapSupport.test.ts --runInBand-> 41/41 passnode script/generate-llms-full.mjs --check-> pass after final main merge; unchanged by later code-only fixesReview/check gate:
claude-reviewcomplete for 738c580; CodeQL complete/pass; CodeRabbit passKnown residual risk: Low; error diagnostics are preserved as text/plain+nosniff and source-map lifecycle has focused coverage plus full CI
Finalized by: maintainer approval from Justin in the Codex parent batch thread; current-head
claude-review, CodeQL, CodeRabbit, and full CI checks provide independent check evidence for this SHA