fix: refactor formatExceptionMessage to accept generic request context by AbanoubGhadban · Pull Request #2877 · shakacode/react_on_rails

AbanoubGhadban · 2026-03-28T20:44:19Z

Summary

Renamed renderingRequest parameter to requestDescription in formatExceptionMessage
Changed the hardcoded label from "JS code for rendering request was:" to the neutral "Request:"
Updated JSDoc to reflect the broader usage

Context

formatExceptionMessage was originally written for the render endpoint and hardcoded the label "JS code for rendering request was:". Since the /upload-assets endpoint now also calls it (via lock-failure handling) with a task description string like "Uploading bundles [server.js] with assets [stats.json]", this produced misleading error logs. The fix uses a generic label that works for both use cases.

All 11 call sites already pass a string, so no caller changes were needed.

Test plan

TypeScript type-check passes (pnpm run --filter react-on-rails-pro-node-renderer type-check)
ESLint passes on the changed file
Package builds successfully (pnpm run --filter react-on-rails-pro-node-renderer build)
Verified all 11 call sites — no caller changes required

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Improved rendering error messages to clearly show the request context, choosing a descriptive label when a JS rendering snippet is present or using a provided custom label/content.
- Simplified the default label to "Request:" where applicable.
- Trimmed and cleaned surrounding whitespace/newlines so error snippets and overall output are easier to read.

coderabbitai · 2026-03-28T20:44:34Z

Caution

Review failed

An error occurred during the review process. Please try again later.

Walkthrough

Updated exception formatting by introducing a RequestInfo type and changing formatExceptionMessage to accept a RequestInfo object; call sites were updated to pass { renderingRequest } or { label: 'Request:', content: ... } as appropriate. No other control flow changes.

Changes

Cohort / File(s)	Summary
Exception formatting utility `packages/react-on-rails-pro-node-renderer/src/shared/utils.ts`	Added exported `RequestInfo` type (`{ renderingRequest: string }
Worker entrypoints and handlers `packages/react-on-rails-pro-node-renderer/src/worker.ts`, `packages/react-on-rails-pro-node-renderer/src/worker/handleRenderRequest.ts`	Updated all `formatExceptionMessage` calls to pass a `RequestInfo` object: `{ renderingRequest }` for render requests, and `{ label: 'Request:', content: taskDescription }` for upload-assets/lock-failure paths. No other logic changes.
VM & stream error paths `packages/react-on-rails-pro-node-renderer/src/worker/vm.ts`	Adjusted error reporting in `runInVM` and stream error handler to pass `{ renderingRequest }` to `formatExceptionMessage` instead of a raw string. No control flow changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I hopped through code with gentle paws,
Wrapped requests in tags without a cause.
Now labels sing true, no puzzling test —
Snippets snugged in a tidy nest. ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR implements discriminated union approach for request context, but issue `#2858` specifies a simpler solution with parameter rename and neutral 'Request:' label without caller changes.	The implementation diverges from `#2858` requirements. Use parameter name 'requestDescription' and single neutral 'Request:' label, avoiding caller modifications needed for the union type approach.
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: refactoring formatExceptionMessage to accept generic request context instead of render-specific parameters.
Out of Scope Changes check	✅ Passed	All changes directly address the requirement to make formatExceptionMessage accept generic request context; no extraneous modifications detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/2858-refactor-format-exception-message

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

greptile-apps · 2026-03-28T20:45:49Z

Greptile Summary

This PR is a focused, low-risk refactor of formatExceptionMessage in utils.ts to make the function generic enough for use beyond rendering requests. The change renames the first parameter from renderingRequest to requestDescription and replaces the hardcoded label "JS code for rendering request was:" with the neutral "Request:", eliminating misleading log output when the upload-assets endpoint calls the function with a task description string.

Purely cosmetic/label change — no logic, return shape, or control-flow is modified.
All 11 existing call sites already pass a string, so no callers required updates.
JSDoc updated to accurately describe the broader contract.
No new tests are needed for a label-only string change; existing error-message checks would still pass if they only assert on EXCEPTION MESSAGE: / STACK: sections.

Confidence Score: 5/5

Safe to merge — pure cosmetic/label refactor with no logic changes and no call-site modifications required.

The only changed lines are a parameter rename and a template-literal label string. There are no logic changes, no type signature breaking changes, and all 11 call sites are unaffected. No P0 or P1 issues were found.

No files require special attention.

Important Files Changed

Filename	Overview
packages/react-on-rails-pro-node-renderer/src/shared/utils.ts	Renames `renderingRequest` → `requestDescription` and changes the label string from "JS code for rendering request was:" to "Request:" to support generic call sites (render + upload-assets); JSDoc updated accordingly. No logic changes.

Sequence Diagram

sequenceDiagram
    participant Render as Render Endpoint
    participant Upload as Upload-Assets Endpoint
    participant FEM as formatExceptionMessage(requestDescription, error, context?)
    participant Log as Error Log

    Render->>FEM: JS code string (e.g. bundle contents)
    Upload->>FEM: Task description string (e.g. "Uploading bundles [server.js]…")
    FEM->>Log: "Request:\n{smartTrim(requestDescription)}\nEXCEPTION MESSAGE: …"

_{Reviews (1): Last reviewed commit: "fix: rename formatExceptionMessage param..." | Re-trigger Greptile}

claude · 2026-03-28T20:47:11Z

Overall: clean, focused fix. The rename and label change correctly address the misleading output in upload-assets error logs. No blocking issues - all 11 call sites were verified. Minor notes: (1) The label 'Request:' is quite terse; for render paths the content is JS code, for upload paths it is a task description - something like 'Request context:' would be marginally clearer in logs. (2) worker.ts lines 326/334 still pass a variable named renderingRequest while upload-assets callers use requestContext - no change required, just a mild naming inconsistency. (3) Pre-existing trailing whitespace on line 70 of utils.ts in the template literal blank line.

claude · 2026-03-28T20:47:19Z

  return `${context ? `\nContext:\n${context}\n` : ''}
-JS code for rendering request was:
-${smartTrim(renderingRequest)}
+Request:


The label Request: is correct in that it applies to both JS-code render requests and task-description upload requests, but it is quite terse for a debugging log. When scanning error output it's not immediately clear what the content beneath it represents (raw JS? a URL? a task name?).

Suggested change

Request:

Request context:

A slightly more descriptive label would still be generic enough to cover both callers while giving more signal to someone triaging a production error. Either way this is not a blocker.

Addressed — updated to Request context: in 63e2105. Also removed the trailing whitespace on the blank line.

github-actions · 2026-03-28T20:48:54Z

size-limit report 📦

Path	Size
react-on-rails/client bundled (gzip)	62.63 KB (0%)
react-on-rails/client bundled (gzip) (time)	62.63 KB (0%)
react-on-rails/client bundled (brotli)	53.7 KB (0%)
react-on-rails/client bundled (brotli) (time)	53.7 KB (0%)
react-on-rails-pro/client bundled (gzip)	63.56 KB (0%)
react-on-rails-pro/client bundled (gzip) (time)	63.56 KB (0%)
react-on-rails-pro/client bundled (brotli)	54.6 KB (0%)
react-on-rails-pro/client bundled (brotli) (time)	54.6 KB (0%)
registerServerComponent/client bundled (gzip)	127.39 KB (0%)
registerServerComponent/client bundled (gzip) (time)	127.39 KB (0%)
registerServerComponent/client bundled (brotli)	61.52 KB (0%)
registerServerComponent/client bundled (brotli) (time)	61.52 KB (0%)
wrapServerComponentRenderer/client bundled (gzip)	122.02 KB (0%)
wrapServerComponentRenderer/client bundled (gzip) (time)	122.02 KB (0%)
wrapServerComponentRenderer/client bundled (brotli)	56.77 KB (0%)
wrapServerComponentRenderer/client bundled (brotli) (time)	56.77 KB (0%)

claude · 2026-03-28T20:59:39Z

Code Review: The change is well-motivated and the implementation is correct. A few items worth noting: (1) CHANGELOG entry missing: per AGENTS.md, user-visible bug fixes need a CHANGELOG.md entry with a [Pro] prefix. Changing the error label is observable in production logs. (2) PR description says the new label is 'Request:' but the code uses 'Request context:' — worth keeping accurate. (3) Label confusion: when context is provided the output contains both 'Context:' and 'Request context:', using the word context with different meanings. A cleaner label would be 'Request:' or 'Request details:'. (4) Good catch removing the trailing whitespace after the request snippet.

claude · 2026-03-28T20:59:46Z

-JS code for rendering request was:
-${smartTrim(renderingRequest)}
-    
+Request context:


The label "Request context:" clashes with the "Context:" label that appears just above it in the same output when context is provided:

Context: Error in a rendering stream ← the "context" param Request context: ← confusingly similar label <JS code or task description>

The PR description says the intended label is "Request:" — that shorter form avoids the ambiguity. Consider:

Suggested change

Request context:

Request:

Good catch — reverted to plain Request: in 16d6458. The clash with the Context: section above was a clear issue.

claude · 2026-03-28T21:11:51Z

Review

Clean, well-scoped fix. The rename and label change are correct, and the PR description accurately reflects the final code.

Must fix: CHANGELOG entry missing. Per AGENTS.md, user-visible bug fixes require a CHANGELOG entry. Changing the error log label is observable in production logs (operators watching for patterns in error output would see a different string). Add an entry under [Unreleased] > Fixed with a **[Pro]** prefix.

Minor notes (non-blocking):

Leading blank line in output: the template literal always produces a leading newline (the literal newline after the context ? expression on line 67). This was present before the PR and is likely intentional for log readability, but worth verifying.
Label when context is present: when context is supplied the message has both Context: and Request: sections, using the word context with two different meanings. Pre-existing issue, not a blocker.
Good catch removing the four trailing spaces on the old blank line in the template literal.

claude · 2026-03-28T21:12:00Z

-JS code for rendering request was:
-${smartTrim(renderingRequest)}
-    
+Request:


The label Request: is a clear improvement over the render-specific old label and works correctly for both call sites. One thing worth considering: when context is also provided, the output reads:

Context: <context> Request: <requestDescription>

The word "context" is now used for two different things — the outer context parameter and the surrounding phrase "request context." Not blocking, but Request details: or Request input: would make the two sections visually distinct.

Also note: this file needs a CHANGELOG entry — the label change is user-visible in production logs and qualifies as a **[Pro]** bug fix per AGENTS.md.

Per the changelog guidelines: "Do NOT add entries for: linting, formatting, refactoring, tests, or documentation fixes." This is a parameter rename + label change (refactoring), so no CHANGELOG entry is needed.

Regarding the label name — Request: is what the issue specifies, and both Request details: and Request input: add words without meaningfully disambiguating from Context:. Keeping Request: as-is.

…ntext (#2858) The function hardcoded "JS code for rendering request was:" which was misleading when called from the /upload-assets endpoint with a task description string. Rename the parameter from `renderingRequest` to `requestDescription` and change the label to the neutral "Request:". No caller changes needed - all 11 call sites already pass a string. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

Address review feedback: use a more descriptive label than bare "Request:" for better log clarity, and remove pre-existing trailing whitespace in the template literal. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

'Request context:' is ambiguous when the optional 'Context:' section is also present in the same error output. Plain 'Request:' is unambiguous and matches the original PR intent. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

…r-specific label Rendering callers now pass { renderingRequest } and get the original "JS code for rendering request was:" label, while non-rendering callers (upload-assets) pass { label, content } with an explicit label. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

claude · 2026-04-08T10:20:08Z

  return badRequestResponseResult(msg);
 }

+export type RequestInfo = { renderingRequest: string } | { label: string; content: string };


Missing blank line between the type export and the JSDoc comment — the JSDoc currently reads as if it documents the type, not formatExceptionMessage.

Suggested change

export type RequestInfo = { renderingRequest: string } | { label: string; content: string };

export type RequestInfo = { renderingRequest: string } | { label: string; content: string };

claude · 2026-04-08T10:20:10Z

      // See https://github.com/shakacode/react_on_rails/issues/2463
-      const result = await handleNewBundlesProvided(taskDescription, providedNewBundles, assetsToCopy);
+      const result = await handleNewBundlesProvided(
+        { label: 'Request:', content: taskDescription },


'Request:' is quite generic in an error log — consider something like 'Upload task:' to make it immediately clear in a log tail that this error came from the upload-assets path rather than a render request.

claude · 2026-04-08T10:20:27Z

 */
 async function handleNewBundleProvided(
-  requestContext: string,
+  requestContext: RequestInfo,


The JSDoc just above (line 87) still has @param renderingRequest — should be updated to @param requestContext to match the renamed parameter.

claude · 2026-04-08T10:20:40Z

Review: fix: refactor formatExceptionMessage to accept generic request context

The change is well-motivated and correctly solves the misleading label problem for the /upload-assets path. The discriminated-union design for RequestInfo is idiomatic TypeScript and the backward-compatible label ('JS code for rendering request was:') is preserved for all existing render-request call sites.

Found issues (all minor):

Stale @param in handleNewBundleProvided JSDoc — @param renderingRequest was not updated to @param requestContext. Flagged inline.
Missing blank line between type export and JSDoc — RequestInfo is exported on the line immediately before the JSDoc for formatExceptionMessage, making it look like the JSDoc describes the type. Flagged inline.
Generic label 'Request:' for the upload-assets path — In the error log this won't immediately identify which endpoint the failure came from. Something like 'Upload task:' would make log triage faster. Flagged inline.

No test gaps that need addressing — the { label, content } variant is covered indirectly via the uploadRaceCondition.test.ts integration tests, and the happy path for the { renderingRequest } variant has extensive coverage. A targeted unit test for formatExceptionMessage itself would be nice-to-have, but is not blocking.

Overall: LGTM with the three minor nits above addressed.

#2877) ## Summary - Renamed `renderingRequest` parameter to `requestDescription` in `formatExceptionMessage` - Changed the hardcoded label from `"JS code for rendering request was:"` to the neutral `"Request:"` - Updated JSDoc to reflect the broader usage Closes #2858 ## Context `formatExceptionMessage` was originally written for the render endpoint and hardcoded the label `"JS code for rendering request was:"`. Since the `/upload-assets` endpoint now also calls it (via lock-failure handling) with a task description string like `"Uploading bundles [server.js] with assets [stats.json]"`, this produced misleading error logs. The fix uses a generic label that works for both use cases. All 11 call sites already pass a `string`, so no caller changes were needed. ## Test plan - [x] TypeScript type-check passes (`pnpm run --filter react-on-rails-pro-node-renderer type-check`) - [x] ESLint passes on the changed file - [x] Package builds successfully (`pnpm run --filter react-on-rails-pro-node-renderer build`) - [x] Verified all 11 call sites — no caller changes required 🤖 Generated with [Claude Code](https://claude.com/claude-code)  ## Summary by CodeRabbit * **Bug Fixes** * Improved rendering error messages to clearly show the request context, choosing a descriptive label when a JS rendering snippet is present or using a provided custom label/content. * Simplified the default label to "Request:" where applicable. * Trimmed and cleaned surrounding whitespace/newlines so error snippets and overall output are easier to read.  --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>

#2877) - Renamed `renderingRequest` parameter to `requestDescription` in `formatExceptionMessage` - Changed the hardcoded label from `"JS code for rendering request was:"` to the neutral `"Request:"` - Updated JSDoc to reflect the broader usage Closes #2858 `formatExceptionMessage` was originally written for the render endpoint and hardcoded the label `"JS code for rendering request was:"`. Since the `/upload-assets` endpoint now also calls it (via lock-failure handling) with a task description string like `"Uploading bundles [server.js] with assets [stats.json]"`, this produced misleading error logs. The fix uses a generic label that works for both use cases. All 11 call sites already pass a `string`, so no caller changes were needed. - [x] TypeScript type-check passes (`pnpm run --filter react-on-rails-pro-node-renderer type-check`) - [x] ESLint passes on the changed file - [x] Package builds successfully (`pnpm run --filter react-on-rails-pro-node-renderer build`) - [x] Verified all 11 call sites — no caller changes required 🤖 Generated with [Claude Code](https://claude.com/claude-code)  * **Bug Fixes** * Improved rendering error messages to clearly show the request context, choosing a descriptive label when a JS rendering snippet is present or using a provided custom label/content. * Simplified the default label to "Request:" where applicable. * Trimmed and cleaned surrounding whitespace/newlines so error snippets and overall output are easier to read.  --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>

…ages * origin/main: (44 commits) Consolidate CSP nonce sanitization into shared module (#2828) Add comprehensive --rsc-pro generator tests (#3098) fix: cross-env validation and docs for renderer password (#3090) Improve package metadata and Pro upgrade CTAs (#3112) docs: standardize warning syntax to GFM alert format (#3115) docs: improve react-intl documentation for React Server Components (#3085) Fix generator CI SSR regression on main (#3110) Refocus GitHub README on docs navigation (#3113) Add manual dev environment testing checklist for coding agents (#3074) Bump version to 16.6.0 Update CHANGELOG.md for 16.6.0 (#3078) fix: node-renderer diagnostic improvements (#3086) fix: pin third-party npm deps in generator to prevent peer dep conflicts (#3083) chore(deps): bump lodash from 4.17.23 to 4.18.1 in the npm-security group across 1 directory (#2920) fix: refactor formatExceptionMessage to accept generic request context (#2877) Bump version to 16.6.0.rc.1 Update CHANGELOG.md for 16.6.0.rc.1 (#3079) Update CHANGELOG.md unreleased section (#3077) Fix Content-Length mismatch and null renderingRequest errors in node renderer (#3069) Improve memory debugging docs with simpler heap snapshot approach (#3072) ... # Conflicts: # docs/pro/home-pro.md # docs/pro/react-on-rails-pro.md # docs/sidebars.ts

claude Bot reviewed Mar 28, 2026

View reviewed changes

coderabbitai Bot approved these changes Mar 28, 2026

View reviewed changes

claude Bot reviewed Mar 28, 2026

View reviewed changes

AbanoubGhadban and others added 4 commits April 8, 2026 12:09

AbanoubGhadban force-pushed the fix/2858-refactor-format-exception-message branch from 630c5e8 to b05027b Compare April 8, 2026 10:17

claude Bot reviewed Apr 8, 2026

View reviewed changes

AbanoubGhadban merged commit 8fd1cbc into main Apr 8, 2026
39 checks passed

AbanoubGhadban deleted the fix/2858-refactor-format-exception-message branch April 8, 2026 10:48

This was referenced Apr 18, 2026

docs: address RSC pitfalls review follow-ups (#3155) #3156

Merged

refactor: unify renderer cache staging with mode: :copy / :symlink #3167

Merged

Fix Bencher reporting permanently broken on pushes to main #3148

Merged

This was referenced May 4, 2026

feat: add REACT_ON_RAILS_BASE_PORT for concurrent worktree port management #3142

Open

feat: rolling_deploy_adapter for seeding previous bundle hashes #3173

Open

	export type RequestInfo = { renderingRequest: string } \| { label: string; content: string };
	export type RequestInfo = { renderingRequest: string } \| { label: string; content: string };

Uh oh!

Conversation

AbanoubGhadban commented Mar 28, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Context

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (2 warnings)

Uh oh!

greptile-apps Bot commented Mar 28, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

claude Bot commented Mar 28, 2026

Uh oh!

claude Bot Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

AbanoubGhadban Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

claude Bot commented Mar 28, 2026

Uh oh!

claude Bot Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

AbanoubGhadban Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot commented Mar 28, 2026

Review

Uh oh!

claude Bot Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

AbanoubGhadban Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot commented Apr 8, 2026

Review: fix: refactor formatExceptionMessage to accept generic request context

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

AbanoubGhadban commented Mar 28, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Mar 28, 2026 •

edited

Loading

github-actions Bot commented Mar 28, 2026 •

edited

Loading