Skip to content

Commit f83615a

Browse files
authored
Reverse engineer original stack frames when virtual frames are re-serialized (facebook#30416)
Stacked on facebook#30410. If we've parsed another RSC stream on the server from a different RSC server, while using `findSourceMapURL`, the Flight Client ends up adding a `rsc://React/` prefix and a numeric suffix to the URL. It's a virtual file that represents the virtual eval:ed frame in that environment. If we then see that same stack again, we'd serialize a virtual frame to another virtual. Meaning `findSourceMapURL` on the client would see the virtual frame of the intermediate server and it would have to strip it to figure out what source map to use. This PR strips it in the Server if we see a virtual frame. At each new client it always refers to the original stack. We don't have to do this. We could leave it to each `findSourceMapURL` implementation and `captureOwnerStack` parser to recursively strip each layer. It could maybe be useful to have the environment name in the virtual frame to know which server to look for the source map in.
1 parent 43dac1e commit f83615a

File tree

2 files changed

+121
-2
lines changed

2 files changed

+121
-2
lines changed

packages/react-client/src/__tests__/ReactFlight-test.js

Lines changed: 106 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,14 @@ describe('ReactFlight', () => {
143143
this.props.expectedMessage,
144144
);
145145
expect(this.state.error.digest).toBe('a dev digest');
146-
expect(this.state.error.environmentName).toBe('Server');
146+
expect(this.state.error.environmentName).toBe(
147+
this.props.expectedEnviromentName || 'Server',
148+
);
149+
if (this.props.expectedErrorStack !== undefined) {
150+
expect(this.state.error.stack).toContain(
151+
this.props.expectedErrorStack,
152+
);
153+
}
147154
} else {
148155
expect(this.state.error.message).toBe(
149156
'An error occurred in the Server Components render. The specific message is omitted in production' +
@@ -2603,6 +2610,104 @@ describe('ReactFlight', () => {
26032610
);
26042611
});
26052612

2613+
it('preserves error stacks passed through server-to-server with source maps', async () => {
2614+
async function ServerComponent({transport}) {
2615+
// This is a Server Component that receives other Server Components from a third party.
2616+
const thirdParty = ReactServer.use(
2617+
ReactNoopFlightClient.read(transport, {
2618+
findSourceMapURL(url) {
2619+
// By giving a source map url we're saying that we can't use the original
2620+
// file as the sourceURL, which gives stack traces a rsc://React/ prefix.
2621+
return 'source-map://' + url;
2622+
},
2623+
}),
2624+
);
2625+
// This will throw a third-party error inside the first-party server component.
2626+
await thirdParty.model;
2627+
return 'Should never render';
2628+
}
2629+
2630+
async function bar() {
2631+
throw new Error('third-party-error');
2632+
}
2633+
2634+
async function foo() {
2635+
await bar();
2636+
}
2637+
2638+
const rejectedPromise = foo();
2639+
2640+
const thirdPartyTransport = ReactNoopFlightServer.render(
2641+
{model: rejectedPromise},
2642+
{
2643+
environmentName: 'third-party',
2644+
onError(x) {
2645+
if (__DEV__) {
2646+
return 'a dev digest';
2647+
}
2648+
return `digest("${x.message}")`;
2649+
},
2650+
},
2651+
);
2652+
2653+
let originalError;
2654+
try {
2655+
await rejectedPromise;
2656+
} catch (x) {
2657+
originalError = x;
2658+
}
2659+
expect(originalError.message).toBe('third-party-error');
2660+
2661+
const transport = ReactNoopFlightServer.render(
2662+
<ServerComponent transport={thirdPartyTransport} />,
2663+
{
2664+
onError(x) {
2665+
if (__DEV__) {
2666+
return 'a dev digest';
2667+
}
2668+
return x.digest; // passthrough
2669+
},
2670+
},
2671+
);
2672+
2673+
await 0;
2674+
await 0;
2675+
await 0;
2676+
2677+
const expectedErrorStack = originalError.stack
2678+
// Test only the first rows since there's a lot of noise after that is eliminated.
2679+
.split('\n')
2680+
.slice(0, 4)
2681+
.join('\n')
2682+
.replaceAll(
2683+
' (/',
2684+
gate(flags => flags.enableOwnerStacks) ? ' (file:///' : ' (/',
2685+
); // The eval will end up normalizing these
2686+
2687+
let sawReactPrefix = false;
2688+
await act(async () => {
2689+
ReactNoop.render(
2690+
<ErrorBoundary
2691+
expectedMessage="third-party-error"
2692+
expectedEnviromentName="third-party"
2693+
expectedErrorStack={expectedErrorStack}>
2694+
{ReactNoopFlightClient.read(transport, {
2695+
findSourceMapURL(url) {
2696+
if (url.startsWith('rsc://React/')) {
2697+
// We don't expect to see any React prefixed URLs here.
2698+
sawReactPrefix = true;
2699+
}
2700+
// My not giving a source map, we should leave it intact.
2701+
return null;
2702+
},
2703+
})}
2704+
</ErrorBoundary>,
2705+
);
2706+
});
2707+
2708+
expect(sawReactPrefix).toBe(false);
2709+
});
2710+
26062711
it('can change the environment name inside a component', async () => {
26072712
let env = 'A';
26082713
function Component(props) {

packages/react-server/src/ReactFlightServer.js

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,21 @@ function filterStackTrace(error: Error, skipFrames: number): ReactStackTrace {
146146
// to save bandwidth even in DEV. We'll also replay these stacks on the client so by
147147
// stripping them early we avoid that overhead. Otherwise we'd normally just rely on
148148
// the DevTools or framework's ignore lists to filter them out.
149-
return parseStackTrace(error, skipFrames).filter(isNotExternal);
149+
const stack = parseStackTrace(error, skipFrames).filter(isNotExternal);
150+
for (let i = 0; i < stack.length; i++) {
151+
const callsite = stack[i];
152+
const url = callsite[1];
153+
if (url.startsWith('rsc://React/')) {
154+
// This callsite is a virtual fake callsite that came from another Flight client.
155+
// We need to reverse it back into the original location by stripping its prefix
156+
// and suffix.
157+
const suffixIdx = url.lastIndexOf('?');
158+
if (suffixIdx > -1) {
159+
callsite[1] = url.slice(12, suffixIdx);
160+
}
161+
}
162+
}
163+
return stack;
150164
}
151165

152166
initAsyncDebugInfo();

0 commit comments

Comments
 (0)