@samitouri / QOS-React-1 / commits / 0b5835a46f

[Flight] Implement captureStackTrace and owner stacks on the Server (#30197)

Wire up owner stacks in Flight to the shared internals. This exposes it to `captureOwnerStack()`. In this case we install it permanently as we only allow one RSC renderer which then supports async contexts. Same thing we do for owner. This also ends up adding it to errors logged by React through `consoleWithStackDev`. The plan is to eventually remove that but this is inline with what we do in Fizz and Fiber already. However, at the same time we've instrumented the console so we need to strip them back out before sending to the client. This lets the client control whether to add the stack back in or allowing `console.createTask` to control it. This is another reason we shouldn't append them from React but for now we hack it by removing them after the fact.

Sebastian Markbåge committed Jul 4, 2024 at 12:15 UTC 0b5835a46f6e19a0a0ead0f4cd30b0db14bc72f7
5 files changed +221 -5
packages/react-client/src/__tests__/ReactFlight-test.js
+75 -3
@@ -81,6 +81,7 @@ let ErrorBoundary;
81 let NoErrorExpected;
82 let Scheduler;
83 let assertLog;
84 +let assertConsoleErrorDev;
85
86 describe('ReactFlight', () => {
87 beforeEach(() => {
@@ -102,6 +103,7 @@ describe('ReactFlight', () => {
103 Scheduler = require('scheduler');
104 const InternalTestUtils = require('internal-test-utils');
105 assertLog = InternalTestUtils.assertLog;
106 + assertConsoleErrorDev = InternalTestUtils.assertConsoleErrorDev;
107
108 ErrorBoundary = class extends React.Component {
109 state = {hasError: false, error: null};
@@ -1441,9 +1443,7 @@ describe('ReactFlight', () => {
1443 <div>{Array(6).fill(<NoKey />)}</div>,
1444 );
1445 ReactNoopFlightClient.read(transport);
1444 - }).toErrorDev('Each child in a list should have a unique "key" prop.', {
1445 - withoutStack: gate(flags => flags.enableOwnerStacks),
1446 - });
1446 + }).toErrorDev('Each child in a list should have a unique "key" prop.');
1447 });
1448
1449 it('should warn in DEV a child is missing keys in client component', async () => {
@@ -2728,4 +2728,76 @@ describe('ReactFlight', () => {
2728
2729 expect(ReactNoop).toMatchRenderedOutput(<span>Hello, Seb</span>);
2730 });
2731 +
2732 + // @gate __DEV__ && enableOwnerStacks
2733 + it('can get the component owner stacks during rendering in dev', () => {
2734 + let stack;
2735 +
2736 + function Foo() {
2737 + return ReactServer.createElement(Bar, null);
2738 + }
2739 + function Bar() {
2740 + return ReactServer.createElement(
2741 + 'div',
2742 + null,
2743 + ReactServer.createElement(Baz, null),
2744 + );
2745 + }
2746 +
2747 + function Baz() {
2748 + stack = ReactServer.captureOwnerStack();
2749 + return ReactServer.createElement('span', null, 'hi');
2750 + }
2751 + ReactNoopFlightServer.render(
2752 + ReactServer.createElement(
2753 + 'div',
2754 + null,
2755 + ReactServer.createElement(Foo, null),
2756 + ),
2757 + );
2758 +
2759 + expect(normalizeCodeLocInfo(stack)).toBe(
2760 + '\n in Bar (at **)' + '\n in Foo (at **)',
2761 + );
2762 + });
2763 +
2764 + // @gate (enableOwnerStacks && enableServerComponentLogs) || !__DEV__
2765 + it('should not include component stacks in replayed logs (unless DevTools add them)', () => {
2766 + function Foo() {
2767 + return 'hi';
2768 + }
2769 +
2770 + function Bar() {
2771 + const array = [];
2772 + // Trigger key warning
2773 + array.push(ReactServer.createElement(Foo));
2774 + return ReactServer.createElement('div', null, array);
2775 + }
2776 +
2777 + function App() {
2778 + return ReactServer.createElement(Bar);
2779 + }
2780 +
2781 + const transport = ReactNoopFlightServer.render(
2782 + ReactServer.createElement(App),
2783 + );
2784 + assertConsoleErrorDev([
2785 + 'Each child in a list should have a unique "key" prop.' +
2786 + ' See https://react.dev/link/warning-keys for more information.\n' +
2787 + ' in Bar (at **)\n' +
2788 + ' in App (at **)',
2789 + ]);
2790 +
2791 + // Replay logs on the client
2792 + ReactNoopFlightClient.read(transport);
2793 + assertConsoleErrorDev(
2794 + [
2795 + 'Each child in a list should have a unique "key" prop.' +
2796 + ' See https://react.dev/link/warning-keys for more information.',
2797 + ],
2798 + // We should not have a stack in the replay because that should be added either by console.createTask
2799 + // or React DevTools on the client. Neither of which we do here.
2800 + {withoutStack: true},
2801 + );
2802 + });
2803 });
packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js
+47
@@ -39,6 +39,15 @@ let ReactServerDOMServer;
39 let ReactServerDOMClient;
40 let use;
41
42 +function normalizeCodeLocInfo(str) {
43 + return (
44 + str &&
45 + str.replace(/^ +(?:at|in) ([\S]+)[^\n]*/gm, function (m, name) {
46 + return ' in ' + name + (/\d/.test(m) ? ' (at **)' : '');
47 + })
48 + );
49 +}
50 +
51 describe('ReactFlightDOMEdge', () => {
52 beforeEach(() => {
53 jest.resetModules();
@@ -883,4 +892,42 @@ describe('ReactFlightDOMEdge', () => {
892 );
893 }
894 });
895 +
896 + // @gate __DEV__ && enableOwnerStacks
897 + it('can get the component owner stacks asynchronously', async () => {
898 + let stack;
899 +
900 + function Foo() {
901 + return ReactServer.createElement(Bar, null);
902 + }
903 + function Bar() {
904 + return ReactServer.createElement(
905 + 'div',
906 + null,
907 + ReactServer.createElement(Baz, null),
908 + );
909 + }
910 +
911 + const promise = Promise.resolve(0);
912 +
913 + async function Baz() {
914 + await promise;
915 + stack = ReactServer.captureOwnerStack();
916 + return ReactServer.createElement('span', null, 'hi');
917 + }
918 +
919 + const stream = ReactServerDOMServer.renderToReadableStream(
920 + ReactServer.createElement(
921 + 'div',
922 + null,
923 + ReactServer.createElement(Foo, null),
924 + ),
925 + webpackMap,
926 + );
927 + await readResult(stream);
928 +
929 + expect(normalizeCodeLocInfo(stack)).toBe(
930 + '\n in Bar (at **)' + '\n in Foo (at **)',
931 + );
932 + });
933 });
packages/react-server/src/ReactFlightServer.js
+43 -2
@@ -97,6 +97,10 @@ import {DefaultAsyncDispatcher} from './flight/ReactFlightAsyncDispatcher';
97
98 import {resolveOwner, setCurrentOwner} from './flight/ReactFlightCurrentOwner';
99
100 +import {getOwnerStackByComponentInfoInDev} from './flight/ReactFlightComponentStack';
101 +
102 +import {isWritingAppendedStack} from 'shared/consoleWithStackDev';
103 +
104 import {
105 getIteratorFn,
106 REACT_ELEMENT_TYPE,
@@ -263,8 +267,9 @@ function patchConsole(consoleInst: typeof console, methodName: string) {
267 'name',
268 );
269 const wrapperMethod = function (this: typeof console) {
270 + let args = arguments;
271 const request = resolveRequest();
267 - if (methodName === 'assert' && arguments[0]) {
272 + if (methodName === 'assert' && args[0]) {
273 // assert doesn't emit anything unless first argument is falsy so we can skip it.
274 } else if (request !== null) {
275 // Extract the stack. Not all console logs print the full stack but they have at
@@ -276,7 +281,22 @@ function patchConsole(consoleInst: typeof console, methodName: string) {
281 // refer to previous logs in debug info to associate them with a component.
282 const id = request.nextChunkId++;
283 const owner: null | ReactComponentInfo = resolveOwner();
279 - emitConsoleChunk(request, id, methodName, owner, stack, arguments);
284 + if (
285 + isWritingAppendedStack &&
286 + (methodName === 'error' || methodName === 'warn') &&
287 + args.length > 1 &&
288 + typeof args[0] === 'string' &&
289 + args[0].endsWith('%s')
290 + ) {
291 + // This looks like we've appended the component stack to the error from our own logs.
292 + // We don't want those added to the replayed logs since those have the opportunity to add
293 + // their own stacks or use console.createTask on the client as needed.
294 + // TODO: Remove this special case once we remove consoleWithStackDev.
295 + // $FlowFixMe[method-unbinding]
296 + args = Array.prototype.slice.call(args, 0, args.length - 1);
297 + args[0] = args[0].slice(0, args[0].length - 2);
298 + }
299 + emitConsoleChunk(request, id, methodName, owner, stack, args);
300 }
301 // $FlowFixMe[prop-missing]
302 return originalMethod.apply(this, arguments);
@@ -317,6 +337,21 @@ if (
337 patchConsole(console, 'warn');
338 }
339
340 +function getCurrentStackInDEV(): string {
341 + if (__DEV__) {
342 + if (enableOwnerStacks) {
343 + const owner: null | ReactComponentInfo = resolveOwner();
344 + if (owner === null) {
345 + return '';
346 + }
347 + return getOwnerStackByComponentInfoInDev(owner);
348 + }
349 + // We don't have Parent Stacks in Flight.
350 + return '';
351 + }
352 + return '';
353 +}
354 +
355 const ObjectPrototype = Object.prototype;
356
357 type JSONValue =
@@ -491,6 +526,12 @@ function RequestInstance(
526 );
527 }
528 ReactSharedInternals.A = DefaultAsyncDispatcher;
529 + if (__DEV__) {
530 + // Unlike Fizz or Fiber, we don't reset this and just keep it on permanently.
531 + // This lets it act more like the AsyncDispatcher so that we can get the
532 + // stack asynchronously too.
533 + ReactSharedInternals.getCurrentStack = getCurrentStackInDEV;
534 + }
535
536 const abortSet: Set<Task> = new Set();
537 const pingedTasks: Array<Task> = [];
packages/react-server/src/flight/ReactFlightComponentStack.js new
+52
@@ -0,0 +1,52 @@
1 +/**
2 + * Copyright (c) Meta Platforms, Inc. and affiliates.
3 + *
4 + * This source code is licensed under the MIT license found in the
5 + * LICENSE file in the root directory of this source tree.
6 + *
7 + * @flow
8 + */
9 +
10 +import type {ReactComponentInfo} from 'shared/ReactTypes';
11 +
12 +import {describeBuiltInComponentFrame} from 'shared/ReactComponentStackFrame';
13 +
14 +import {enableOwnerStacks} from 'shared/ReactFeatureFlags';
15 +
16 +export function getOwnerStackByComponentInfoInDev(
17 + componentInfo: ReactComponentInfo,
18 +): string {
19 + if (!enableOwnerStacks || !__DEV__) {
20 + return '';
21 + }
22 + try {
23 + let info = '';
24 +
25 + // The owner stack of the current component will be where it was created, i.e. inside its owner.
26 + // There's no actual name of the currently executing component. Instead, that is available
27 + // on the regular stack that's currently executing. However, if there is no owner at all, then
28 + // there's no stack frame so we add the name of the root component to the stack to know which
29 + // component is currently executing.
30 + if (!componentInfo.owner && typeof componentInfo.name === 'string') {
31 + return describeBuiltInComponentFrame(componentInfo.name);
32 + }
33 +
34 + let owner: void | null | ReactComponentInfo = componentInfo;
35 +
36 + while (owner) {
37 + if (typeof owner.stack === 'string') {
38 + // Server Component
39 + const ownerStack: string = owner.stack;
40 + owner = owner.owner;
41 + if (owner && ownerStack !== '') {
42 + info += '\n' + ownerStack;
43 + }
44 + } else {
45 + break;
46 + }
47 + }
48 + return info;
49 + } catch (x) {
50 + return '\nError generating stack: ' + x.message + '\n' + x.stack;
51 + }
52 +}
packages/shared/consoleWithStackDev.js
+4
@@ -40,6 +40,8 @@ export function error(format, ...args) {
40 // eslint-disable-next-line react-internal/no-production-logging
41 const supportsCreateTask = __DEV__ && enableOwnerStacks && !!console.createTask;
42
43 +export let isWritingAppendedStack = false;
44 +
45 function printWarning(level, format, args, currentStack) {
46 // When changing this logic, you might want to also
47 // update consoleWithStackDev.www.js as well.
@@ -50,6 +52,7 @@ function printWarning(level, format, args, currentStack) {
52 // can be lost while DevTools isn't open but we can't detect this.
53 const stack = ReactSharedInternals.getCurrentStack(currentStack);
54 if (stack !== '') {
55 + isWritingAppendedStack = true;
56 format += '%s';
57 args = args.concat([stack]);
58 }
@@ -60,5 +63,6 @@ function printWarning(level, format, args, currentStack) {
63 // breaks IE9: https://github.com/facebook/react/issues/13610
64 // eslint-disable-next-line react-internal/no-production-logging
65 Function.prototype.apply.call(console[level], console, args);
66 + isWritingAppendedStack = false;
67 }
68 }