Skip to content

Commit ccea5fd

Browse files
authored
[Flight & DevTools] Strip the whole "async " prefix from V8 stack frame names (react#37608)
## Summary V8 prefixes async call sites with `async ` when it prints a stack, like this: ``` Error: boom at inner (/tmp/asy.js:1:44) at async outerName (/tmp/asy.js:2:30) ``` `parseStackTraceFromChromeStack` captures `async outerName` as the frame name and strips the prefix here: ```js } else if (name.startsWith('async ')) { name = name.slice(5); isAsync = true; } ``` `'async '` is six characters, so `slice(5)` leaves the space behind and the frame name comes back as `' outerName'` rather than `'outerName'`. I noticed it while reading the parser, and it is not purely cosmetic: that name is the first element of the `ReactFunctionLocation` returned by `extractLocationFromComponentStack` and `extractLocationFromOwnerStack`, which `backend/fiber/renderer.js` stores as `instance.source`. Any async component whose frame reaches that path is recorded under a name with a stray leading space. The same line exists in `packages/react-server/src/ReactFlightStackConfigV8.js`, which the DevTools file is a copy of. After review I fixed it in this PR as well, in a second commit. There it only matters on the fallback path that parses an already formatted stack string, when the error's `stack` was read or assigned before React reaches it. react#37130 is open on that file too, but it does not touch these lines. ## How did you test this change? I first confirmed the format V8 actually emits, rather than assuming it: ``` $ node -e 'async function inner(){await null;throw new Error("boom")} async function outerName(){await inner()} outerName().catch(e=>console.log(e.stack))' Error: boom at inner ([eval]:1:52) at async outerName ([eval]:2:33) ``` Then I added a case to the existing `extractLocationFromComponentStack` block in `utils-test.js`. Against `main` it fails with exactly the leading space: ``` ● utils › extractLocationFromComponentStack › should strip the async prefix from a frame name - Expected - 1 + Received + 1 Array [ - "Comments", + " Comments", "https://react.dev/_next/static/chunks/848-122f91e9565d9ffa.js", 5, 9236, ] ``` With the one-character fix applied: ``` $ yarn test --build --project=devtools -r=experimental utils-test PASS packages/react-devtools-shared/src/__tests__/utils-test.js Tests: 63 passed, 63 total ``` I also ran the whole DevTools project before and after to check I was not moving anything else. Both runs end at `9 failed, 4 failed suites`, the same test names each time (`componentStacks`, `console`, `inspectedElement`, `legacy/inspectElement`), so those failures are pre-existing on `main` in my environment and unrelated to this change. The only difference between the two runs is my new test: 586 passed before, 587 after. For the Flight side I added `ReactFlightStackConfigV8-test.js`, which assigns a formatted stack to an error and checks what `parseStackTrace` returns. Against `main` it fails with the same leading space (`" outerName"`); with the fix it passes on stable and experimental in development. It is gated to `__DEV__` because that fallback goes through the DEV-only stack cache. `ReactFlightServer-test` and `ReactFlightAsyncDebugInfo-test` still pass next to it (23 tests). `prettier` and `eslint` are clean on all changed files. `yarn flow dom-node` reported no errors for the DevTools commit; I did not rerun Flow after the one-character Flight change. AI tools used
1 parent 564923c commit ccea5fd

4 files changed

Lines changed: 52 additions & 2 deletions

File tree

packages/react-devtools-shared/src/__tests__/utils-test.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,19 @@ describe('utils', () => {
395395
]);
396396
});
397397

398+
it('should strip the async prefix from a frame name', () => {
399+
expect(
400+
extractLocationFromComponentStack(
401+
' at async Comments (https://react.dev/_next/static/chunks/848-122f91e9565d9ffa.js:5:9236)',
402+
),
403+
).toEqual([
404+
'Comments',
405+
'https://react.dev/_next/static/chunks/848-122f91e9565d9ffa.js',
406+
5,
407+
9236,
408+
]);
409+
});
410+
398411
it('should support Firefox stack', () => {
399412
expect(
400413
extractLocationFromComponentStack(

packages/react-devtools-shared/src/backend/utils/parseStackTrace.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ function parseStackTraceFromChromeStack(
4545
if (name === '<anonymous>') {
4646
name = '';
4747
} else if (name.startsWith('async ')) {
48-
name = name.slice(5);
48+
name = name.slice(6);
4949
isAsync = true;
5050
}
5151
let filename = parsed[2] || parsed[5] || '';

packages/react-server/src/ReactFlightStackConfigV8.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ export function parseStackTrace(
237237
if (name === '<anonymous>') {
238238
name = '';
239239
} else if (name.startsWith('async ')) {
240-
name = name.slice(5);
240+
name = name.slice(6);
241241
isAsync = true;
242242
}
243243
let filename = parsed[2] || parsed[5] || '';
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
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+
* @emails react-core
8+
* @jest-environment node
9+
*/
10+
11+
'use strict';
12+
13+
let parseStackTrace;
14+
15+
describe('ReactFlightStackConfigV8', () => {
16+
beforeEach(() => {
17+
jest.resetModules();
18+
parseStackTrace =
19+
require('react-server/src/ReactFlightStackConfigV8').parseStackTrace;
20+
});
21+
22+
// @gate __DEV__
23+
it('strips the async prefix from a frame name in an already formatted stack', () => {
24+
const error = new Error();
25+
// Assigning the stack means V8 never calls prepareStackTrace, so the
26+
// parser has to fall back to reading the formatted string.
27+
error.stack =
28+
'Error: boom\n' +
29+
' at inner (/tmp/app.js:1:44)\n' +
30+
' at async outerName (/tmp/app.js:2:30)';
31+
32+
expect(parseStackTrace(error, 1)).toEqual([
33+
['inner', '/tmp/app.js', 1, 44, 0, 0, false],
34+
['outerName', '/tmp/app.js', 2, 30, 0, 0, true],
35+
]);
36+
});
37+
});

0 commit comments

Comments
 (0)