[Flight] Override prepareStackTrace when reading stacks (#29740)
This lets us ensure that we use the original V8 format and it lets us skip source mapping. Source mapping every call can be expensive since we do it eagerly for server components even if an error doesn't happen. In the case of an error being thrown we don't actually always do this in practice because if a try/catch before us touches it or if something in onError touches it (which the default console.error does), it has already been initialized. So we have to be resilient to thrown errors having other formats. These are not as perf sensitive since something actually threw but if you want better perf in these cases, you can simply do something like `onError(error) { console.error(error.message) }` instead. The server has to be aware whether it's looking up original or compiled output. I currently use the file:// check to determine if it's referring to a source mapped file or compiled file in the fixture. A bundled app can more easily check if it's a bundle or not.
Sebastian Markbåge committed
Jun 5, 2024 at 03:41 UTC
1df34bdf626af3e4566364dcdf7f1c387d2f4252
3 files changed
+52
-20
.eslintrc.js
+1
@@ -486,6 +486,7 @@ module.exports = {
486
$ReadOnlyArray: 'readonly',
487
$ArrayBufferView: 'readonly',
488
$Shape: 'readonly',
489
+ CallSite: 'readonly',
490
ConsoleTask: 'readonly', // TOOD: Figure out what the official name of this will be.
491
ReturnType: 'readonly',
492
AnimationFrameID: 'readonly',
fixtures/flight/server/region.js
+14
-12
@@ -187,7 +187,11 @@ if (process.env.NODE_ENV === 'development') {
187
res.set('Content-type', 'application/json');
188
let requestedFilePath = req.query.name;
189
190
+ let isCompiledOutput = false;
191
if (requestedFilePath.startsWith('file://')) {
192
+ // We assume that if it was prefixed with file:// it's referring to the compiled output
193
+ // and if it's a direct file path we assume it's source mapped back to original format.
194
+ isCompiledOutput = true;
195
requestedFilePath = requestedFilePath.slice(7);
196
}
197
@@ -204,11 +208,11 @@ if (process.env.NODE_ENV === 'development') {
208
let map;
209
// There are two ways to return a source map depending on what we observe in error.stack.
210
// A real app will have a similar choice to make for which strategy to pick.
207
- if (!sourceMap || Error.prepareStackTrace === undefined) {
208
- // When --enable-source-maps is enabled, the error.stack that we use to track
209
- // stacks will have had the source map already applied so it's pointing to the
210
- // original source. We return a blank source map that just maps everything to
211
- // the original source in this case.
211
+ if (!sourceMap || !isCompiledOutput) {
212
+ // If a file doesn't have a source map, such as this file, then we generate a blank
213
+ // source map that just contains the original content and segments pointing to the
214
+ // original lines.
215
+ // Similarly
216
const sourceContent = await readFile(requestedFilePath, 'utf8');
217
const lines = sourceContent.split('\n').length;
218
map = {
@@ -222,13 +226,11 @@ if (process.env.NODE_ENV === 'development') {
226
sourceRoot: '',
227
};
228
} else {
225
- // If something has overridden prepareStackTrace it is likely not getting the
226
- // natively applied source mapping to error.stack and so the line will point to
227
- // the compiled output similar to how a browser works.
228
- // E.g. ironically this can happen with the source-map-support library that is
229
- // auto-invoked by @babel/register if external source maps are generated.
230
- // In this case we just use the source map that the native source mapping would
231
- // have used.
229
+ // We always set prepareStackTrace before reading the stack so that we get the stack
230
+ // without source maps applied. Therefore we have to use the original source map.
231
+ // If something read .stack before we did, we might observe the line/column after
232
+ // source mapping back to the original file. We use the isCompiledOutput check above
233
+ // in that case.
234
map = sourceMap.payload;
235
}
236
res.write(JSON.stringify(map));
packages/react-server/src/ReactFlightServer.js
+37
-8
@@ -137,10 +137,41 @@ function isNotExternal(stackFrame: string): boolean {
137
return !externalRegExp.test(stackFrame);
138
}
139
140
+function prepareStackTrace(
141
+ error: Error,
142
+ structuredStackTrace: CallSite[],
143
+): string {
144
+ const name = error.name || 'Error';
145
+ const message = error.message || '';
146
+ let stack = name + ': ' + message;
147
+ for (let i = 0; i < structuredStackTrace.length; i++) {
148
+ stack += '\n at ' + structuredStackTrace[i].toString();
149
+ }
150
+ return stack;
151
+}
152
+
153
+function getStack(error: Error): string {
154
+ // We override Error.prepareStackTrace with our own version that normalizes
155
+ // the stack to V8 formatting even if the server uses other formatting.
156
+ // It also ensures that source maps are NOT applied to this since that can
157
+ // be slow we're better off doing that lazily from the client instead of
158
+ // eagerly on the server. If the stack has already been read, then we might
159
+ // not get a normalized stack and it might still have been source mapped.
160
+ // So the client still needs to be resilient to this.
161
+ const previousPrepare = Error.prepareStackTrace;
162
+ Error.prepareStackTrace = prepareStackTrace;
163
+ try {
164
+ // eslint-disable-next-line react-internal/safe-string-coercion
165
+ return String(error.stack);
166
+ } finally {
167
+ Error.prepareStackTrace = previousPrepare;
168
+ }
169
+}
170
+
171
function initCallComponentFrame(): string {
172
// Extract the stack frame of the callComponentInDEV function.
173
const error = callComponentInDEV(Error, 'react-stack-top-frame', {});
143
- const stack = error.stack;
174
+ const stack = getStack(error);
175
const startIdx = stack.startsWith('Error: react-stack-top-frame\n') ? 29 : 0;
176
const endIdx = stack.indexOf('\n', startIdx);
177
if (endIdx === -1) {
@@ -155,7 +186,7 @@ function initCallIteratorFrame(): string {
186
(callIteratorInDEV: any)({next: null});
187
return '';
188
} catch (error) {
158
- const stack = error.stack;
189
+ const stack = getStack(error);
190
const startIdx = stack.startsWith('TypeError: ')
191
? stack.indexOf('\n') + 1
192
: 0;
@@ -174,7 +205,7 @@ function initCallLazyInitFrame(): string {
205
_init: Error,
206
_payload: 'react-stack-top-frame',
207
});
177
- const stack = error.stack;
208
+ const stack = getStack(error);
209
const startIdx = stack.startsWith('Error: react-stack-top-frame\n') ? 29 : 0;
210
const endIdx = stack.indexOf('\n', startIdx);
211
if (endIdx === -1) {
@@ -188,7 +219,7 @@ function filterDebugStack(error: Error): string {
219
// to save bandwidth even in DEV. We'll also replay these stacks on the client so by
220
// stripping them early we avoid that overhead. Otherwise we'd normally just rely on
221
// the DevTools or framework's ignore lists to filter them out.
191
- let stack = error.stack;
222
+ let stack = getStack(error);
223
if (stack.startsWith('Error: react-stack-top-frame\n')) {
224
// V8's default formatting prefixes with the error message which we
225
// don't want/need.
@@ -2601,8 +2632,7 @@ function emitPostponeChunk(
2632
try {
2633
// eslint-disable-next-line react-internal/safe-string-coercion
2634
reason = String(postponeInstance.message);
2604
- // eslint-disable-next-line react-internal/safe-string-coercion
2605
- stack = String(postponeInstance.stack);
2635
+ stack = getStack(postponeInstance);
2636
} catch (x) {}
2637
row = serializeRowHeader('P', id) + stringify({reason, stack}) + '\n';
2638
} else {
@@ -2627,8 +2657,7 @@ function emitErrorChunk(
2657
if (error instanceof Error) {
2658
// eslint-disable-next-line react-internal/safe-string-coercion
2659
message = String(error.message);
2630
- // eslint-disable-next-line react-internal/safe-string-coercion
2631
- stack = String(error.stack);
2660
+ stack = getStack(error);
2661
} else if (typeof error === 'object' && error !== null) {
2662
message = describeObjectForErrorMessage(error);
2663
} else {