Include environment name both in the virtual URL and findSourceMapURL

This way you can use the environment to know where to look for the source map in case you have multiple server environments.
facebook · Jul 24, 2024 · 54eeb6a · 54eeb6a
1 parent 933b737
commit 54eeb6a
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 29 deletions.
diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js
@@ -232,7 +232,10 @@ Chunk.prototype.then = function <T>(
   }
 };
 
-export type FindSourceMapURLCallback = (fileName: string) => null | string;
+export type FindSourceMapURLCallback = (
+  fileName: string,
+  environmentName: string,
+) => null | string;
 
 export type Response = {
   _bundlerConfig: SSRModuleMap,
@@ -689,7 +692,15 @@ function createElement(
       writable: true,
       value: null,
     });
+    let env = '';
     if (enableOwnerStacks) {
+      if (owner !== null && owner.env != null) {
+        // Interestingly we don't actually have the environment name of where
+        // this JSX was created if it doesn't have an owner but if it does
+        // it must be the same environment as the owner. We could send it separately
+        // but it seems a bit unnecessary for this edge case.
+        env = owner.env;
+      }
       let normalizedStackTrace: null | Error = null;
       if (stack !== null) {
         // We create a fake stack and then create an Error object inside of it.
@@ -698,7 +709,11 @@ function createElement(
         // source mapping information.
         // This can unfortunately happen within a user space callstack which will
         // remain on the stack.
-        normalizedStackTrace = createFakeJSXCallStackInDEV(response, stack);
+        normalizedStackTrace = createFakeJSXCallStackInDEV(
+          response,
+          stack,
+          env,
+        );
       }
       Object.defineProperty(element, '_debugStack', {
         configurable: false,
@@ -713,7 +728,12 @@ function createElement(
           console,
           getTaskName(type),
         );
-        const callStack = buildFakeCallStack(response, stack, createTaskFn);
+        const callStack = buildFakeCallStack(
+          response,
+          stack,
+          env,
+          createTaskFn,
+        );
         // This owner should ideally have already been initialized to avoid getting
         // user stack frames on the stack.
         const ownerTask =
@@ -1821,6 +1841,7 @@ function resolveErrorDev(
     const callStack = buildFakeCallStack(
       response,
       stack,
+      env,
       // $FlowFixMe[incompatible-use]
       Error.bind(
         null,
@@ -1877,6 +1898,7 @@ function resolvePostponeDev(
   id: number,
   reason: string,
   stack: ReactStackTrace,
+  env: string,
 ): void {
   if (!__DEV__) {
     // These errors should never make it into a build so we don't need to encode them in codes.json
@@ -1902,6 +1924,7 @@ function resolvePostponeDev(
     const callStack = buildFakeCallStack(
       response,
       stack,
+      env,
       // $FlowFixMe[incompatible-use]
       Error.bind(null, reason || ''),
     );
@@ -1946,6 +1969,7 @@ function createFakeFunction<T>(
   sourceMap: null | string,
   line: number,
   col: number,
+  environmentName: string,
 ): FakeFunction<T> {
   // This creates a fake copy of a Server Module. It represents a module that has already
   // executed on the server but we re-execute a blank copy for its stack frames on the client.
@@ -1998,7 +2022,13 @@ function createFakeFunction<T>(
     // 1) A printed stack trace string needs a unique URL to be able to source map it.
     // 2) If source maps are disabled or fails, you should at least be able to tell
     //    which file it was.
-    code += '\n//# sourceURL=rsc://React/' + filename + '?' + fakeFunctionIdx++;
+    code +=
+      '\n//# sourceURL=rsc://React/' +
+      encodeURIComponent(environmentName) +
+      '/' +
+      filename +
+      '?' +
+      fakeFunctionIdx++;
     code += '\n//# sourceMappingURL=' + sourceMap;
   } else if (filename) {
     code += '\n//# sourceURL=' + filename;
@@ -2022,19 +2052,28 @@ function createFakeFunction<T>(
 function buildFakeCallStack<T>(
   response: Response,
   stack: ReactStackTrace,
+  environmentName: string,
   innerCall: () => T,
 ): () => T {
   let callStack = innerCall;
   for (let i = 0; i < stack.length; i++) {
     const frame = stack[i];
-    const frameKey = frame.join('-');
+    const frameKey = frame.join('-') + '-' + environmentName;
     let fn = fakeFunctionCache.get(frameKey);
     if (fn === undefined) {
       const [name, filename, line, col] = frame;
-      const sourceMap = response._debugFindSourceMapURL
-        ? response._debugFindSourceMapURL(filename)
+      const findSourceMapURL = response._debugFindSourceMapURL;
+      const sourceMap = findSourceMapURL
+        ? findSourceMapURL(filename, environmentName)
         : null;
-      fn = createFakeFunction(name, filename, sourceMap, line, col);
+      fn = createFakeFunction(
+        name,
+        filename,
+        sourceMap,
+        line,
+        col,
+        environmentName,
+      );
       // TODO: This cache should technically live on the response since the _debugFindSourceMapURL
       // function is an input and can vary by response.
       fakeFunctionCache.set(frameKey, fn);
@@ -2064,7 +2103,7 @@ function initializeFakeTask(
   }
 
   const stack = debugInfo.stack;
-
+  const env = componentInfo.env == null ? '' : componentInfo.env;
   const ownerTask =
     componentInfo.owner == null
       ? null
@@ -2074,7 +2113,7 @@ function initializeFakeTask(
     console,
     getServerComponentTaskName(componentInfo),
   );
-  const callStack = buildFakeCallStack(response, stack, createTaskFn);
+  const callStack = buildFakeCallStack(response, stack, env, createTaskFn);
 
   let componentTask;
   if (ownerTask === null) {
@@ -2096,10 +2135,12 @@ const createFakeJSXCallStack = {
   'react-stack-bottom-frame': function (
     response: Response,
     stack: ReactStackTrace,
+    environmentName: string,
   ): Error {
     const callStackForError = buildFakeCallStack(
       response,
       stack,
+      environmentName,
       fakeJSXCallSite,
     );
     return callStackForError();
@@ -2109,6 +2150,7 @@ const createFakeJSXCallStack = {
 const createFakeJSXCallStackInDEV: (
   response: Response,
   stack: ReactStackTrace,
+  environmentName: string,
 ) => Error = __DEV__
   ? // We use this technique to trick minifiers to preserve the function name.
     (createFakeJSXCallStack['react-stack-bottom-frame'].bind(
@@ -2132,12 +2174,11 @@ function initializeFakeStack(
     return;
   }
   if (debugInfo.stack != null) {
+    const stack = debugInfo.stack;
+    const env = debugInfo.env == null ? '' : debugInfo.env;
     // $FlowFixMe[cannot-write]
     // $FlowFixMe[prop-missing]
-    debugInfo.debugStack = createFakeJSXCallStackInDEV(
-      response,
-      debugInfo.stack,
-    );
+    debugInfo.debugStack = createFakeJSXCallStackInDEV(response, stack, env);
   }
   if (debugInfo.owner != null) {
     // Initialize any owners not yet initialized.
@@ -2206,6 +2247,7 @@ function resolveConsoleEntry(
   const callStack = buildFakeCallStack(
     response,
     stackTrace,
+    env,
     printToConsole.bind(null, methodName, args, env),
   );
   if (owner != null) {
@@ -2445,6 +2487,7 @@ function processFullStringRow(
             id,
             postponeInfo.reason,
             postponeInfo.stack,
+            postponeInfo.env,
           );
         } else {
           resolvePostponeProd(response, id);

diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js
@@ -1241,10 +1241,10 @@ describe('ReactFlight', () => {
     const ClientErrorBoundary = clientReference(MyErrorBoundary);
 
     function App() {
-      return (
-        <ClientErrorBoundary>
-          <ServerComponent />
-        </ClientErrorBoundary>
+      return ReactServer.createElement(
+        ClientErrorBoundary,
+        null,
+        ReactServer.createElement(ServerComponent),
       );
     }
 
@@ -1301,13 +1301,16 @@ describe('ReactFlight', () => {
         ],
         findSourceMapURLCalls: gate(flags => flags.enableOwnerStacks)
           ? [
-              [__filename],
-              [__filename],
+              [__filename, 'Server'],
+              [__filename, 'Server'],
               // TODO: What should we request here? The outer (<anonymous>) or the inner (inspected-page.html)?
-              ['inspected-page.html:29:11), <anonymous>'],
-              ['file://~/(some)(really)(exotic-directory)/ReactFlight-test.js'],
-              ['file:///testing.js'],
-              [__filename],
+              ['inspected-page.html:29:11), <anonymous>', 'Server'],
+              [
+                'file://~/(some)(really)(exotic-directory)/ReactFlight-test.js',
+                'Server',
+              ],
+              ['file:///testing.js', 'Server'],
+              [__filename, 'Server'],
             ]
           : [],
       });
@@ -2836,18 +2839,20 @@ describe('ReactFlight', () => {
       ); // The eval will end up normalizing these
 
     let sawReactPrefix = false;
+    const environments = [];
     await act(async () => {
       ReactNoop.render(
         <ErrorBoundary
           expectedMessage="third-party-error"
           expectedEnviromentName="third-party"
           expectedErrorStack={expectedErrorStack}>
           {ReactNoopFlightClient.read(transport, {
-            findSourceMapURL(url) {
+            findSourceMapURL(url, environmentName) {
               if (url.startsWith('rsc://React/')) {
                 // We don't expect to see any React prefixed URLs here.
                 sawReactPrefix = true;
               }
+              environments.push(environmentName);
               // My not giving a source map, we should leave it intact.
               return null;
             },
@@ -2857,6 +2862,16 @@ describe('ReactFlight', () => {
     });
 
     expect(sawReactPrefix).toBe(false);
+    if (__DEV__) {
+      expect(environments.slice(0, 4)).toEqual([
+        'Server',
+        'third-party',
+        'third-party',
+        'third-party',
+      ]);
+    } else {
+      expect(environments).toEqual([]);
+    }
   });
 
   it('can change the environment name inside a component', async () => {

diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js
@@ -155,10 +155,12 @@ function filterStackTrace(error: Error, skipFrames: number): ReactStackTrace {
     if (url.startsWith('rsc://React/')) {
       // This callsite is a virtual fake callsite that came from another Flight client.
       // We need to reverse it back into the original location by stripping its prefix
-      // and suffix.
+      // and suffix. We don't need the environment name because it's available on the
+      // parent object that will contain the stack.
+      const envIdx = url.indexOf('/', 12);
       const suffixIdx = url.lastIndexOf('?');
-      if (suffixIdx > -1) {
-        callsite[1] = url.slice(12, suffixIdx);
+      if (envIdx > -1 && suffixIdx > -1) {
+        callsite[1] = url.slice(envIdx + 1, suffixIdx);
       }
     }
   }
@@ -2857,14 +2859,15 @@ function emitPostponeChunk(
   if (__DEV__) {
     let reason = '';
     let stack: ReactStackTrace;
+    const env = request.environmentName();
     try {
       // eslint-disable-next-line react-internal/safe-string-coercion
       reason = String(postponeInstance.message);
       stack = filterStackTrace(postponeInstance, 0);
     } catch (x) {
       stack = [];
     }
-    row = serializeRowHeader('P', id) + stringify({reason, stack}) + '\n';
+    row = serializeRowHeader('P', id) + stringify({reason, stack, env}) + '\n';
   } else {
     // No reason included in prod.
     row = serializeRowHeader('P', id) + '\n';