From c8898958d2c4fb1b135cdc53c8dc0662ff2c1ffb Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 21 Jul 2024 20:41:25 -0400 Subject: [PATCH] Reverse engineer original stack frames when virtual frames are re-serialized --- .../src/__tests__/ReactFlight-test.js | 104 +++++++++++++++++- .../react-server/src/ReactFlightServer.js | 16 ++- 2 files changed, 118 insertions(+), 2 deletions(-) diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 5d7e8faf49d7f..01d53d4cecb64 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -143,7 +143,14 @@ describe('ReactFlight', () => { this.props.expectedMessage, ); expect(this.state.error.digest).toBe('a dev digest'); - expect(this.state.error.environmentName).toBe('Server'); + expect(this.state.error.environmentName).toBe( + this.props.expectedEnviromentName || 'Server', + ); + if (this.props.expectedErrorStack !== undefined) { + expect(this.state.error.stack).toContain( + this.props.expectedErrorStack, + ); + } } else { expect(this.state.error.message).toBe( 'An error occurred in the Server Components render. The specific message is omitted in production' + @@ -2603,6 +2610,101 @@ describe('ReactFlight', () => { ); }); + it('preserves error stacks passed through server-to-server with source maps', async () => { + async function ServerComponent({transport}) { + // This is a Server Component that receives other Server Components from a third party. + const thirdParty = ReactServer.use( + ReactNoopFlightClient.read(transport, { + findSourceMapURL(url) { + // By giving a source map url we're saying that we can't use the original + // file as the sourceURL, which gives stack traces a rsc://React/ prefix. + return 'source-map://' + url; + }, + }), + ); + // This will throw a third-party error inside the first-party server component. + await thirdParty.model; + return 'Should never render'; + } + + async function bar() { + throw new Error('third-party-error'); + } + + async function foo() { + await bar(); + } + + const rejectedPromise = foo(); + + const thirdPartyTransport = ReactNoopFlightServer.render( + {model: rejectedPromise}, + { + environmentName: 'third-party', + onError(x) { + if (__DEV__) { + return 'a dev digest'; + } + return `digest("${x.message}")`; + }, + }, + ); + + let originalError; + try { + await rejectedPromise; + } catch (x) { + originalError = x; + } + expect(originalError.message).toBe('third-party-error'); + + const transport = ReactNoopFlightServer.render( + , + { + onError(x) { + if (__DEV__) { + return 'a dev digest'; + } + return x.digest; // passthrough + }, + }, + ); + + await 0; + await 0; + await 0; + + const expectedErrorStack = originalError.stack + // Test only the first rows since there's a lot of noise after that is eliminated. + .split('\n') + .slice(0, 4) + .join('\n') + .replaceAll(' (/', ' (file:///'); // The eval will end up normalizing these + + let sawReactPrefix = false; + await act(async () => { + ReactNoop.render( + + {ReactNoopFlightClient.read(transport, { + findSourceMapURL(url) { + if (url.startsWith('rsc://React/')) { + // We don't expect to see any React prefixed URLs here. + sawReactPrefix = true; + } + // My not giving a source map, we should leave it intact. + return null; + }, + })} + , + ); + }); + + expect(sawReactPrefix).toBe(false); + }); + it('can change the environment name inside a component', async () => { let env = 'A'; function Component(props) { diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 73ed634027dd2..9ee0a845463ac 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -146,7 +146,21 @@ function filterStackTrace(error: Error, skipFrames: number): ReactStackTrace { // to save bandwidth even in DEV. We'll also replay these stacks on the client so by // stripping them early we avoid that overhead. Otherwise we'd normally just rely on // the DevTools or framework's ignore lists to filter them out. - return parseStackTrace(error, skipFrames).filter(isNotExternal); + const stack = parseStackTrace(error, skipFrames).filter(isNotExternal); + for (let i = 0; i < stack.length; i++) { + const callsite = stack[i]; + const url = callsite[1]; + if (url.startsWith('rsc://React/')) { + // This callsite is a virtual fake callsite that came from another Flight client. + // We need to reverse it back into the original location by stripping its prefix + // and suffix. + const suffixIdx = url.lastIndexOf('?'); + if (suffixIdx > -1) { + callsite[1] = url.slice(12, suffixIdx); + } + } + } + return stack; } initAsyncDebugInfo();