Redact RSC rendering error details from client DOM in production

CHANGELOG.md

@@ -105,6 +105,16 @@ After a release, run `/update-changelog` in Claude Code to analyze commits, writ
   [PR 4624](https://github.com/shakacode/react_on_rails/pull/4624) by
   [justin808](https://github.com/justin808).
 
+- **[Pro]** **RSC render errors no longer leak server details into client DOM**: In production, the
+  `REACT_ON_RAILS_RSC_ERRORS` inline script now emits only `{ hasErrors: true }` — the full error
+  message, stack trace, and file paths are redacted. Uses an allowlist (development/test show full
+  diagnostics) so custom envs like staging default to redacted. RSC rendering errors are now also
+  reported to error reporters (Sentry/Honeybadger) even with the default `throwJsErrors: false`
+  config, via a custom `'renderingError'` stream event. Fixes
+  [Issue 4629](https://github.com/shakacode/react_on_rails/issues/4629).
+  [PR 4631](https://github.com/shakacode/react_on_rails/pull/4631) by
+  [AbanoubGhadban](https://github.com/AbanoubGhadban).
+
 - **[Pro]** **Hardened Node renderer authentication and multipart uploads**: Authenticated multipart
   clients must now send the password field before file parts; the renderer rejects and discards leading file
   parts before creating upload storage. Uploads now enforce a 100 MB total request limit across files, fields,

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

@@ -25,7 +25,7 @@ import m from 'module';
 import cluster from 'cluster';
 import type { Readable } from 'stream';
 import { ReadableStream } from 'stream/web';
-import { promisify, TextEncoder } from 'util';
+import { promisify, TextEncoder, types as utilTypes } from 'util';
 import type { ReactOnRails as ROR } from 'react-on-rails' with { 'resolution-mode': 'import' };
 import type { Context } from 'vm';
 
@@ -612,14 +612,41 @@ export async function buildExecutionContext(
       });
 
       if (isReadableStream(result)) {
-        const newStreamAfterHandlingError = handleStreamError(result, (error) => {
+        const reportedErrors = new WeakSet<object>();
+        // A stream error thrown inside the sandboxed VM realm is a genuine Error, but it
+        // fails the worker-realm `instanceof Error` check because it comes from a different
+        // realm's `Error.prototype`. Wrapping it in `new Error(String(error))` would discard
+        // its original message and stack (the whole point of reporting to Sentry/Honeybadger).
+        // Use a realm-agnostic check — `util.types.isNativeError` inspects the internal
+        // [[ErrorData]] slot, so it recognizes VM-realm Errors — plus a duck-type for
+        // error-like objects. Only truly non-Error throwables (strings, numbers, …) are
+        // wrapped, since those have no usable stack to preserve.
+        const isErrorLike = (value: unknown): value is { message?: string; stack?: string } => {
+          // eslint-disable-next-line @typescript-eslint/no-deprecated -- Error.isError (the suggested replacement) is not available until Node 23+; this package still supports Node 22, where util.types.isNativeError is the realm-agnostic native-Error check.
+          if (utilTypes.isNativeError(value)) return true;
+          if (typeof value !== 'object' || value === null) return false;
+          const { message, stack } = value as { message?: unknown; stack?: unknown };
+          return typeof stack === 'string' || typeof message === 'string';
+        };
+        const reportStreamError = (error: unknown, label: string) => {
+          const reportable: object = isErrorLike(error) ? error : new Error(String(error));
+          if (reportedErrors.has(reportable)) return;
+          reportedErrors.add(reportable);
           const msg = formatExceptionMessage(
             { renderingRequest },
-            error,
-            'Error in a rendering stream',
+            reportable,
+            label,
             remapStackTraceForRequest,
           );
           errorReporter.message(msg);
+        };
+
+        result.on('renderingError', (error: unknown) => {
+          reportStreamError(error, 'Rendering error in stream');
+        });
+
+        const newStreamAfterHandlingError = handleStreamError(result, (error) => {
+          reportStreamError(error, 'Error in a rendering stream');
         });
         return newStreamAfterHandlingError;
       }

packages/react-on-rails-pro-node-renderer/tests/htmlStreaming.test.js

@@ -229,12 +229,20 @@ describe('html streaming', () => {
     10000,
   );
 
-  it("shouldn't notify error reporter when throwJsErrors is false and shell error happens", async () => {
+  it('notifies the error reporter with the genuine render error when throwJsErrors is false and a shell error happens', async () => {
+    // Behavior change (#4629 / PR #4631 observability fix): RSC rendering errors now reach the
+    // error reporter (Sentry/Honeybadger) even with the default throwJsErrors:false, via the
+    // custom 'renderingError' stream event. Previously the reporter was never called on this path
+    // (this test asserted `.not.toHaveBeenCalled()`), so genuine production render failures were
+    // invisible to monitoring. The reported error is the real render failure, not a benign path.
     await makeRequest({
       props: { throwSyncError: true },
       // throwJsErrors is false by default
     });
-    expect(errorReporter.message).not.toHaveBeenCalled();
+    expect(errorReporter.message).toHaveBeenCalled();
+    expect(errorReporter.message).toHaveBeenCalledWith(
+      expect.stringMatching(/Rendering error in stream[\s\S.]*Sync error from AsyncComponentsTreeForTesting/),
+    );
   }, 10000);
 
   it('should notify error reporter when throwJsErrors is true and shell error happens', async () => {
@@ -288,12 +296,19 @@ describe('html streaming', () => {
     10000,
   );
 
-  it('should not notify error reporter when throwJsErrors is false and async error happens', async () => {
+  it('notifies the error reporter with the genuine render error when throwJsErrors is false and an async error happens', async () => {
+    // Behavior change (#4629 / PR #4631 observability fix): async RSC rendering errors now reach
+    // the error reporter even with throwJsErrors:false, via the custom 'renderingError' stream
+    // event. Previously this asserted `.not.toHaveBeenCalled()`. The reported error is the real
+    // async render failure, not a benign path.
     await makeRequest({
       props: { throwAsyncError: true },
       throwJsErrors: false,
     });
-    expect(errorReporter.message).not.toHaveBeenCalled();
+    expect(errorReporter.message).toHaveBeenCalled();
+    expect(errorReporter.message).toHaveBeenCalledWith(
+      expect.stringMatching(/Rendering error in stream[\s\S.]*Async error from AsyncHelloWorldHooks/),
+    );
   }, 10000);
 
   it('should notify error reporter when throwJsErrors is true and async error happens', async () => {

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

@@ -16,6 +16,8 @@
 import path from 'path';
 import fs from 'fs';
 import vm from 'vm';
+import { Readable } from 'stream';
+import { types as utilTypes } from 'util';
 import {
   uploadedBundlePath,
   createUploadedBundle,
@@ -30,6 +32,7 @@ import {
 import { buildExecutionContext, hasVMContextForBundle, resetVM } from '../src/worker/vm';
 import { getConfig } from '../src/shared/configBuilder';
 import { isErrorRenderResult } from '../src/shared/utils';
+import * as errorReporter from '../src/shared/errorReporter';
 
 const testName = 'vm';
 const uploadedBundlePathForTest = () => uploadedBundlePath(testName);
@@ -830,3 +833,114 @@ describe('buildVM and runInVM', () => {
     });
   });
 });
+
+// Reception side of the RSC observability fix (#4629 PR #4631): when runInVM returns a
+// readable render stream, it attaches a custom 'renderingError' listener plus a WeakSet
+// dedup so rendering errors reach the error reporter (Sentry/Honeybadger) even with the
+// default throwJsErrors:false, and the same Error object is never reported twice across
+// the 'renderingError' and standard 'error' events. These tests drive that path through a
+// real VM whose rendering request returns a stream, then emit events on the original
+// stream (the one runInVM attaches its listeners to, before handleStreamError wraps it).
+describe('runInVM stream error reporting (renderingError listener + WeakSet dedup)', () => {
+  const streamTestName = 'vmStreamErrors';
+  const streamBundlePath = () => uploadedBundlePath(streamTestName);
+
+  let messageSpy: jest.SpyInstance;
+  const activeStreams: Readable[] = [];
+
+  beforeEach(async () => {
+    await resetForTest(streamTestName);
+    messageSpy = jest.spyOn(errorReporter, 'message').mockImplementation(() => {});
+  });
+
+  afterEach(() => {
+    activeStreams.forEach((stream) => {
+      if (!stream.destroyed) stream.destroy();
+    });
+    activeStreams.length = 0;
+    messageSpy.mockRestore();
+  });
+
+  // Build a VM whose rendering request returns a Readable stream and hands the test both
+  // the ORIGINAL stream (the one runInVM attaches its listeners to) and a genuine VM-realm
+  // Error. A VM-realm Error is a real Error (`util.types.isNativeError` is true) but fails
+  // the worker-realm `instanceof Error` check because it comes from the VM realm's own
+  // `Error.prototype` — the exact case codex flagged where `new Error(String(error))` used
+  // to discard the original message and stack.
+  async function buildStreamReturningVM() {
+    const captured: { stream?: Readable; vmError?: unknown } = {};
+    const config = getConfig();
+    config.additionalContext = {
+      __TestReadable: Readable,
+      __captureStreamHandles: (stream: Readable, vmError: unknown) => {
+        captured.stream = stream;
+        captured.vmError = vmError;
+      },
+    };
+    await createUploadedBundle(streamTestName);
+    const { runInVM } = await buildExecutionContext([streamBundlePath()], /* buildVmsIfNeeded */ true);
+    const renderingRequest = [
+      '(function () {',
+      '  const stream = new __TestReadable({ read() {} });',
+      "  __captureStreamHandles(stream, new Error('VM realm stream failure'));",
+      '  return stream;',
+      '})()',
+    ].join('\n');
+    const wrapper = (await runInVM(renderingRequest, streamBundlePath())) as unknown as Readable;
+    activeStreams.push(wrapper);
+    if (captured.stream) activeStreams.push(captured.stream);
+    return { wrapper, stream: captured.stream as Readable, vmError: captured.vmError };
+  }
+
+  it('reports a renderingError stream event to the error reporter', async () => {
+    const { stream, vmError } = await buildStreamReturningVM();
+
+    stream.emit('renderingError', vmError);
+
+    expect(messageSpy).toHaveBeenCalledTimes(1);
+    expect(messageSpy.mock.calls[0][0]).toContain('VM realm stream failure');
+  });
+
+  it('preserves a genuine VM-realm Error message and stack instead of wrapping it', async () => {
+    const { stream, vmError } = await buildStreamReturningVM();
+
+    // Confirms the realm boundary: this is a real Error but not a worker-realm instance.
+    expect(utilTypes.isNativeError(vmError)).toBe(true);
+    expect(vmError instanceof Error).toBe(false);
+    const originalStack = (vmError as { stack: string }).stack;
+
+    stream.emit('renderingError', vmError);
+
+    const reported = messageSpy.mock.calls[0][0] as string;
+    // Message preserved verbatim (not a `String(error)` "Error: Error: ..." double-wrap).
+    expect(reported).toContain('VM realm stream failure');
+    // The reporter received the error's own stack frames, not a vm.ts wrapper frame.
+    const originalFrame = originalStack
+      .split('\n')
+      .map((line) => line.trim())
+      .find((line) => line.startsWith('at '));
+    expect(originalFrame).toBeDefined();
+    expect(reported).toContain(originalFrame as string);
+  });
+
+  it('dedups the SAME Error instance across renderingError and error events (reported once)', async () => {
+    const { stream, vmError } = await buildStreamReturningVM();
+
+    stream.emit('renderingError', vmError);
+    stream.emit('error', vmError);
+
+    expect(messageSpy).toHaveBeenCalledTimes(1);
+  });
+
+  it('reports two DISTINCT Error instances separately', async () => {
+    const { stream } = await buildStreamReturningVM();
+
+    stream.emit('renderingError', new Error('first stream failure'));
+    stream.emit('renderingError', new Error('second stream failure'));
+
+    expect(messageSpy).toHaveBeenCalledTimes(2);
+    const messages = messageSpy.mock.calls.map((call) => call[0] as string).join('\n');
+    expect(messages).toContain('first stream failure');
+    expect(messages).toContain('second stream failure');
+  });
+});

packages/react-on-rails-pro/src/capabilities/proRSC.ts

@@ -113,8 +113,14 @@ const streamRenderRSCComponent = (
     isShellReady: true,
   };
 
-  const { pipeToTransform, readableStream, emitError, isConsumerAborted, onConsumerAbort } =
-    transformRenderStreamChunksToResultObject(renderState);
+  const {
+    pipeToTransform,
+    readableStream,
+    emitError,
+    notifyRenderingError,
+    isConsumerAborted,
+    onConsumerAbort,
+  } = transformRenderStreamChunksToResultObject(renderState);
 
   // On client disconnect the RSC render stream is aborted by cancelUpstream; also release any RSC
   // payload streams this render fetched so their upstream Rails/API work stops, and run post-SSR
@@ -129,9 +135,10 @@ const streamRenderRSCComponent = (
 
   const reportError = (error: Error): Error => {
     const diagnosticError = addRSCClientHookDiagnostic(error, componentName);
-    console.error('Error in RSC stream', diagnosticError);
     if (throwJsErrors) {
       emitError(diagnosticError);
+    } else {
+      notifyRenderingError(diagnosticError);
     }
     renderState.hasErrors = true;
     renderState.error = diagnosticError;

packages/react-on-rails-pro/src/injectRSCPayload.ts

@@ -100,13 +100,24 @@ function createRSCDiagnosticScript(
   metadata: Record<string, unknown>,
   cacheKey: string,
   sanitizedNonce?: string,
+  railsEnv?: string,
 ) {
   const { hasErrors, renderingError } = metadata;
   // `hasErrors` is a boolean per the server wire contract; renderingError only carries
   // a useful diagnostic when the server provided a non-blank message or stack.
   if (hasErrors !== true && !hasRenderingErrorSignal(renderingError)) return undefined;
+  // Outside development/test, emit only the error signal without the server error message or stack.
+  // Full diagnostics are reported server-side via the streaming error reporter (Sentry/Honeybadger).
+  // Fail-closed: unknown or missing railsEnv defaults to redacted (safe for a security gate).
+  const showFullDiagnostics = railsEnv === 'development' || railsEnv === 'test';
+  // The two branches normalize `hasErrors` differently on purpose. The redacted (production)
+  // branch forces `hasErrors: true` so a chunk that only tripped `hasRenderingErrorSignal`
+  // (server sent `hasErrors: false` but a non-blank message/stack) still emits a positive
+  // generic signal for error boundaries. The development/test branch instead passes the raw
+  // metadata through unchanged to preserve the exact historical dev payload shape for debugging.
+  const clientPayload = showFullDiagnostics ? { hasErrors, renderingError } : { hasErrors: true };
   return createScriptTag(
-    `${cacheKeyDiagnosticObject(cacheKey)}||=${JSON.stringify({ hasErrors, renderingError })}`,
+    `${cacheKeyDiagnosticObject(cacheKey)}||=${JSON.stringify(clientPayload)}`,
     sanitizedNonce,
     true,
   );
@@ -1596,6 +1607,7 @@ type InjectRSCPayloadOptions = {
   rscClientManifestStylesheetHrefs?: ReadonlySet<string>;
   rscClientChunkStylesheetHrefsByChunkName?: RSCClientChunkStylesheetHrefsByChunkName;
   rscStreamObservability?: boolean;
+  railsEnv?: string;
 };
 
 /**
@@ -1635,6 +1647,7 @@ export default function injectRSCPayload(
     rscClientManifestStylesheetHrefs = new Set<string>(),
     rscClientChunkStylesheetHrefsByChunkName = loadRSCClientChunkStylesheetHrefsByChunkName(),
     rscStreamObservability = false,
+    railsEnv,
   } = options;
   const sanitizedNonce = sanitizeNonce(cspNonce);
   const htmlStream = new PassThrough();
@@ -2010,7 +2023,12 @@ export default function injectRSCPayload(
             const handleParsedChunk = (content: Uint8Array, metadata: Record<string, unknown>) => {
               const flightData = textDecoder.decode(content);
               if (!hasEmittedDiagnosticScript) {
-                const diagnosticScript = createRSCDiagnosticScript(metadata, rscPayloadKey, sanitizedNonce);
+                const diagnosticScript = createRSCDiagnosticScript(
+                  metadata,
+                  rscPayloadKey,
+                  sanitizedNonce,
+                  railsEnv,
+                );
                 if (diagnosticScript) {
                   rscPayloadBuffers.push(Buffer.from(diagnosticScript));
                   hasEmittedDiagnosticScript = true;

packages/react-on-rails-pro/src/streamServerRenderedReactComponent.ts

@@ -63,6 +63,7 @@ const streamRenderReactComponent = (
     pipeToTransform,
     writeChunk,
     emitError,
+    notifyRenderingError,
     endStream,
     onConsumerAbort,
     isConsumerAborted,
@@ -77,6 +78,8 @@ const streamRenderReactComponent = (
 
     if (throwJsErrors) {
       emitError(error);
+    } else {
+      notifyRenderingError(error);
     }
   };
 
@@ -236,6 +239,7 @@ const streamRenderReactComponent = (
                     ? {}
                     : { rscClientChunkStylesheetHrefsByChunkName: new Map() }),
                   rscStreamObservability: railsContext.rscStreamObservability,
+                  railsEnv: railsContext.railsEnv,
                 },
               ),
             );