Skip to content

Commit cbb2ccc

Browse files
justin808claude
andcommitted
fix: node-renderer diagnostic improvements (#3086)
## Summary - Add `credentials` to the sensitive-key filtering test (every other key in `SENSITIVE_REQUEST_BODY_KEYS` already had coverage) - Exclude `renderingRequest` from diagnostic `bodyKeys` output — it's already reported via the `Received type:` line, so showing it again is redundant - Document that `renderer_http_keep_alive_timeout` should be set shorter than the node renderer's server-side idle timeout to prevent stale-connection errors Fixes #3075 ## Test plan - [x] Node renderer `worker.test.ts` passes (verified locally) - [x] RuboCop passes on `configuration.rb` - [ ] CI green 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Low risk: changes are limited to error-message diagnostics/test coverage in the Node renderer and comment-only documentation for a Rails config option. > > **Overview** > **Node renderer diagnostics are tightened for malformed `renderingRequest` requests.** The invalid-request message now omits the `renderingRequest` key from the reported `bodyKeys` list (it’s already described via `Received type:`), and tests assert this behavior. > > **Sensitive key filtering coverage is expanded.** Tests add `Credentials` to ensure case-insensitive filtering matches `SENSITIVE_REQUEST_BODY_KEYS`. > > **Rails config docs are clarified.** `renderer_http_keep_alive_timeout` is now documented to be set slightly shorter than the node renderer’s server-side idle timeout to avoid stale-connection reuse errors (and to use HTTPX defaults when `nil`). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit f01cd0f. 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 * **Bug Fixes** * Improved error message clarity for invalid render requests by excluding internal keys from diagnostic output. * **Documentation** * Enhanced configuration documentation for HTTP keep-alive timeout settings with recommended values and risk mitigation guidance. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 8106127 commit cbb2ccc

3 files changed

Lines changed: 11 additions & 1 deletion

File tree

packages/react-on-rails-pro-node-renderer/src/worker.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,9 @@ const invalidRenderingRequestMessage = (body: Record<string, unknown>) => {
187187
} else if (renderingRequest === '') {
188188
renderingRequestType = 'empty string';
189189
}
190-
const bodyKeys = Object.keys(body).filter((key) => !SENSITIVE_REQUEST_BODY_KEYS.has(key.toLowerCase()));
190+
const bodyKeys = Object.keys(body).filter(
191+
(key) => key !== 'renderingRequest' && !SENSITIVE_REQUEST_BODY_KEYS.has(key.toLowerCase()),
192+
);
191193

192194
return [
193195
'Invalid "renderingRequest" field in render request.',

packages/react-on-rails-pro-node-renderer/tests/worker.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@ describe('worker', () => {
216216
expect(res.statusCode).toBe(400);
217217
expect(res.payload).toContain('Invalid "renderingRequest" field in render request.');
218218
expect(res.payload).toContain('Received type: array.');
219+
expect(res.payload).not.toMatch(/Received body keys:.*renderingRequest/);
219220
expect(res.payload).toContain('Likely causes: request body truncation');
220221
});
221222

@@ -237,6 +238,7 @@ describe('worker', () => {
237238
AUTH_TOKEN: 'auth',
238239
accessToken: 'access',
239240
authToken: 'auth-camel',
241+
Credentials: 'creds-secret',
240242
safeField: 'safe',
241243
})
242244
.end();
@@ -249,6 +251,7 @@ describe('worker', () => {
249251
expect(res.payload).not.toContain('AUTH_TOKEN');
250252
expect(res.payload).not.toContain('accessToken');
251253
expect(res.payload).not.toContain('authToken');
254+
expect(res.payload).not.toContain('Credentials');
252255
expect(res.payload).toContain('safeField');
253256
});
254257

react_on_rails_pro/lib/react_on_rails_pro/configuration.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,11 @@ def concurrent_component_streaming_buffer_size=(value)
9595

9696
# Sets the keep-alive timeout (in seconds) for persistent HTTP connections to the node renderer.
9797
#
98+
# For best results, set this value to slightly less than the node renderer's own
99+
# keep-alive / idle-connection timeout. If the client-side timeout is longer than
100+
# the server's, connections may be reused after the server has already closed them,
101+
# resulting in stale-connection errors. If set to nil, the HTTPX default is used.
102+
#
98103
# @param value [Numeric, nil] A positive number or nil (to use the HTTPX default)
99104
# @raise [ReactOnRailsPro::Error] if value is not a positive number or nil
100105
def renderer_http_keep_alive_timeout=(value)

0 commit comments

Comments
 (0)