Skip to content

Commit f38c22b

Browse files
authored
[Flight] Set Current Owner / Task When Calling console.error or invoking onError/onPostpone (facebook#30206)
Stacked on facebook#30197. This is similar to facebook#30182 and facebook#21610 in Fizz. Track the current owner/stack/task on the task. This tracks it for attribution when serializing child properties. This lets us provide the right owner and createTask when we console.error from inside Flight itself. This also affects the way we print those logs on the client since we need the owner and stack. Now console.errors that originate on the server gets the right stack on the client: <img width="760" alt="Screenshot 2024-07-03 at 6 03 13 PM" src="https://github.com/facebook/react/assets/63648/913300f8-f364-4e66-a19d-362e8d776c64"> Unfortunately, because we don't track the stack we never pop it so it'll keep tracking for serializing sibling properties. We rely on "children" typically being the last property in the common case anyway. However, this can lead to wrong attribution in some cases where the invalid property is a next property (without a wrapping element) and there's a previous element that doesn't. E.g. `<ClientComponent title={<div />} invalid={nonSerializable} />` would use the div as the attribution instead of ClientComponent. I also wrap all of our own console.error, onError and onPostpone in the context of the parent component. It's annoying to have to remember to do this though. We could always wrap the whole rendering in such as context but it would add more overhead since this rarely actually happens. It might make sense to track the whole current task instead to lower the overhead. That's what we do in Fizz. We'd still have to remember to restore the debug task though. I realize now Fizz doesn't do that neither so the debug task isn't wrapping the console.errors that Fizz itself logs. There's something off about that Flight and Fizz implementations don't perfectly align.
1 parent 0b5835a commit f38c22b

File tree

2 files changed

+287
-110
lines changed

2 files changed

+287
-110
lines changed

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

+62-1
Original file line numberDiff line numberDiff line change
@@ -2761,10 +2761,61 @@ describe('ReactFlight', () => {
27612761
);
27622762
});
27632763

2764+
// @gate __DEV__ && enableOwnerStacks
2765+
it('can get the component owner stacks for onError in dev', async () => {
2766+
const thrownError = new Error('hi');
2767+
let caughtError;
2768+
let ownerStack;
2769+
2770+
function Foo() {
2771+
return ReactServer.createElement(Bar, null);
2772+
}
2773+
function Bar() {
2774+
return ReactServer.createElement(
2775+
'div',
2776+
null,
2777+
ReactServer.createElement(Baz, null),
2778+
);
2779+
}
2780+
function Baz() {
2781+
throw thrownError;
2782+
}
2783+
2784+
ReactNoopFlightServer.render(
2785+
ReactServer.createElement(
2786+
'div',
2787+
null,
2788+
ReactServer.createElement(Foo, null),
2789+
),
2790+
{
2791+
onError(error, errorInfo) {
2792+
caughtError = error;
2793+
ownerStack = ReactServer.captureOwnerStack
2794+
? ReactServer.captureOwnerStack()
2795+
: null;
2796+
},
2797+
},
2798+
);
2799+
2800+
expect(caughtError).toBe(thrownError);
2801+
expect(normalizeCodeLocInfo(ownerStack)).toBe(
2802+
'\n in Bar (at **)' + '\n in Foo (at **)',
2803+
);
2804+
});
2805+
27642806
// @gate (enableOwnerStacks && enableServerComponentLogs) || !__DEV__
27652807
it('should not include component stacks in replayed logs (unless DevTools add them)', () => {
2808+
class MyError extends Error {
2809+
toJSON() {
2810+
return 123;
2811+
}
2812+
}
2813+
27662814
function Foo() {
2767-
return 'hi';
2815+
return ReactServer.createElement('div', null, [
2816+
'Womp womp: ',
2817+
new MyError('spaghetti'),
2818+
]);
27682819
}
27692820

27702821
function Bar() {
@@ -2781,11 +2832,18 @@ describe('ReactFlight', () => {
27812832
const transport = ReactNoopFlightServer.render(
27822833
ReactServer.createElement(App),
27832834
);
2835+
27842836
assertConsoleErrorDev([
27852837
'Each child in a list should have a unique "key" prop.' +
27862838
' See https://react.dev/link/warning-keys for more information.\n' +
27872839
' in Bar (at **)\n' +
27882840
' in App (at **)',
2841+
'Error objects cannot be rendered as text children. Try formatting it using toString().\n' +
2842+
' <div>Womp womp: {Error}</div>\n' +
2843+
' ^^^^^^^\n' +
2844+
' in Foo (at **)\n' +
2845+
' in Bar (at **)\n' +
2846+
' in App (at **)',
27892847
]);
27902848

27912849
// Replay logs on the client
@@ -2794,6 +2852,9 @@ describe('ReactFlight', () => {
27942852
[
27952853
'Each child in a list should have a unique "key" prop.' +
27962854
' See https://react.dev/link/warning-keys for more information.',
2855+
'Error objects cannot be rendered as text children. Try formatting it using toString().\n' +
2856+
' <div>Womp womp: {Error}</div>\n' +
2857+
' ^^^^^^^',
27972858
],
27982859
// We should not have a stack in the replay because that should be added either by console.createTask
27992860
// or React DevTools on the client. Neither of which we do here.

0 commit comments

Comments
 (0)