Skip to content

Commit d2f31cc

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 ae83ac4 commit d2f31cc

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
@@ -180,7 +180,9 @@ const invalidRenderingRequestMessage = (body: Record<string, unknown>) => {
180180
} else if (renderingRequest === '') {
181181
renderingRequestType = 'empty string';
182182
}
183-
const bodyKeys = Object.keys(body).filter((key) => !SENSITIVE_REQUEST_BODY_KEYS.has(key.toLowerCase()));
183+
const bodyKeys = Object.keys(body).filter(
184+
(key) => key !== 'renderingRequest' && !SENSITIVE_REQUEST_BODY_KEYS.has(key.toLowerCase()),
185+
);
184186

185187
return [
186188
'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
@@ -210,6 +210,7 @@ describe('worker', () => {
210210
expect(res.statusCode).toBe(400);
211211
expect(res.payload).toContain('Invalid "renderingRequest" field in render request.');
212212
expect(res.payload).toContain('Received type: array.');
213+
expect(res.payload).not.toMatch(/Received body keys:.*renderingRequest/);
213214
expect(res.payload).toContain('Likely causes: request body truncation');
214215
});
215216

@@ -231,6 +232,7 @@ describe('worker', () => {
231232
AUTH_TOKEN: 'auth',
232233
accessToken: 'access',
233234
authToken: 'auth-camel',
235+
Credentials: 'creds-secret',
234236
safeField: 'safe',
235237
})
236238
.end();
@@ -243,6 +245,7 @@ describe('worker', () => {
243245
expect(res.payload).not.toContain('AUTH_TOKEN');
244246
expect(res.payload).not.toContain('accessToken');
245247
expect(res.payload).not.toContain('authToken');
248+
expect(res.payload).not.toContain('Credentials');
246249
expect(res.payload).toContain('safeField');
247250
});
248251

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)