fix(data-loader): unify the response logic of the Loader in SSR (#6720)

web-infra-dev · Jan 13, 2025 · 09a91c2 · 09a91c2
1 parent c677bc4
commit 09a91c2
Show file tree

Hide file tree

Showing 9 changed files with 190 additions and 54 deletions.
diff --git a/.changeset/tricky-chairs-press.md b/.changeset/tricky-chairs-press.md
@@ -0,0 +1,7 @@
+---
+'@modern-js/plugin-data-loader': patch
+'@modern-js/runtime': patch
+---
+
+feat: unify the response logic of the Loader in SSR
+feat: 统一 SSR 中 Loader 的响应逻辑
diff --git a/packages/cli/plugin-data-loader/src/cli/createRequest.ts b/packages/cli/plugin-data-loader/src/cli/createRequest.ts
@@ -38,28 +38,45 @@ const handleRedirectResponse = (res: Response) => {
   return res;
 };
 
-const handleDeferredResponse = async (res: Response) => {
-  if (
-    res.headers.get('Content-Type')?.match(CONTENT_TYPE_DEFERRED) &&
-    res.body
-  ) {
-    return await parseDeferredReadableStream(res.body);
-  }
-  return res;
+const isDeferredResponse = (res: Response) => {
+  return (
+    res.headers.get('Content-Type')?.match(CONTENT_TYPE_DEFERRED) && res.body
+  );
+};
+
+const isRedirectResponse = (res: Response) => {
+  return res.headers.get('X-Modernjs-Redirect') != null;
 };
 
 const isErrorResponse = (res: Response) => {
   return res.headers.get('X-Modernjs-Error') != null;
 };
 
+function isOtherErrorResponse(res: Response) {
+  return (
+    res.status >= 400 &&
+    res.headers.get('X-Modernjs-Error') == null &&
+    res.headers.get('X-Modernjs-Catch') == null &&
+    res.headers.get('X-Modernjs-Response') == null
+  );
+}
+
+const isCatchResponse = (res: Response) => {
+  return res.headers.get('X-Modernjs-Catch') != null;
+};
+
 const handleErrorResponse = async (res: Response) => {
-  if (isErrorResponse(res)) {
-    const data = await res.json();
-    const error = new Error(data.message);
-    error.stack = data.stack;
-    throw error;
-  }
-  return res;
+  const data = await res.json();
+  const error = new Error(data.message);
+  error.stack = data.stack;
+  throw error;
+};
+
+const handleNetworkErrorResponse = async (res: Response) => {
+  const text = await res.text();
+  const error = new Error(text);
+  error.stack = undefined;
+  throw error;
 };
 
 export const createRequest = (routeId: string, method = 'get') => {
@@ -75,11 +92,30 @@ export const createRequest = (routeId: string, method = 'get') => {
     res = await fetch(url, {
       method,
       signal: request.signal,
+    }).catch(error => {
+      throw error;
     });
 
-    res = handleRedirectResponse(res);
-    res = await handleErrorResponse(res);
-    res = await handleDeferredResponse(res);
+    if (isRedirectResponse(res)) {
+      return handleRedirectResponse(res);
+    }
+
+    if (isErrorResponse(res)) {
+      return await handleErrorResponse(res);
+    }
+
+    if (isCatchResponse(res)) {
+      throw res;
+    }
+
+    if (isDeferredResponse(res)) {
+      return await parseDeferredReadableStream(res.body!);
+    }
+
+    // some response error not from modern.js
+    if (isOtherErrorResponse(res)) {
+      return await handleNetworkErrorResponse(res);
+    }
 
     return res;
   };

diff --git a/packages/cli/plugin-data-loader/src/runtime/errors.ts b/packages/cli/plugin-data-loader/src/runtime/errors.ts
@@ -65,7 +65,7 @@ export function sanitizeError<T = unknown>(error: T) {
     process.env.NODE_ENV !== 'development' &&
     process.env.NODE_ENV !== 'test'
   ) {
-    const sanitized = new Error('Unexpected Server Error');
+    const sanitized = new Error(error.message || 'Unexpected Server Error');
     sanitized.stack = undefined;
     return sanitized;
   }

diff --git a/packages/cli/plugin-data-loader/src/runtime/index.ts b/packages/cli/plugin-data-loader/src/runtime/index.ts
@@ -130,7 +130,8 @@ export const handleRequest = async ({
           });
     }
     const cost = end();
-
+    // add response header for client know if the response is from modern server
+    response.headers.set('X-Modernjs-Response', 'yes');
     onTiming?.(`${LOADER_REPORTER_NAME}-navigation`, cost);
   } catch (error) {
     if (isResponse(error)) {

diff --git a/packages/runtime/plugin-runtime/src/core/server/requestHandler.ts b/packages/runtime/plugin-runtime/src/core/server/requestHandler.ts
@@ -217,7 +217,7 @@ export const createRequestHandler: CreateRequestHandler =
 
       const initialData = await runBeforeRender(context);
 
-      // Support data loader to return status code
+      // Support data loader to return `new Response` and set status code
       if (
         context.routerContext?.statusCode &&
         context.routerContext?.statusCode !== 200

diff --git a/packages/runtime/plugin-runtime/src/router/runtime/plugin.node.tsx b/packages/runtime/plugin-runtime/src/router/runtime/plugin.node.tsx
@@ -125,18 +125,17 @@ export const routerPlugin = (
             return interrupt(routerContext);
           }
 
-          // requestHandler.ts also has same logic, but handle common status code
-          // maybe we can put the logic in requestHandler.ts in the future
+          // Now `throw new Response` or `throw new Error` is same, both will be caught by errorBoundary by default
+          // If loaderFailureMode is 'clientRender', we will downgrade to csr, and the loader will be request in client again
+          const errors = Object.values(
+            (routerContext.errors || {}) as Record<string, Error>,
+          );
           if (
-            routerContext.statusCode >= 500 &&
-            routerContext.statusCode < 600 &&
             // TODO: if loaderFailureMode is not 'errroBoundary', error log will not be printed.
+            errors.length > 0 &&
             loaderFailureMode === 'clientRender'
           ) {
             routerContext.statusCode = 200;
-            const errors = Object.values(
-              routerContext.errors as Record<string, Error>,
-            );
             throw errors[0];
           }
 

diff --git a/tests/integration/routes/src/three/routes/error/response/page.data.ts b/tests/integration/routes/src/three/routes/error/response/page.data.ts
@@ -1,10 +1,35 @@
-export const loader = () => {
-  throw new Response(
-    JSON.stringify({
-      message: "can't found the user",
-    }),
-    {
-      status: 255,
-    },
-  );
+import type { LoaderFunctionArgs } from '@modern-js/runtime/router';
+
+export const loader = ({ request }: LoaderFunctionArgs) => {
+  const url = new URL(request.url);
+  const responseType = url.searchParams.get('type') as string;
+  const statusCode = url.searchParams.get('code') as string;
+
+  if (responseType === 'throw_error') {
+    throw new Error("can't found the user");
+  }
+
+  if (responseType === 'throw_response') {
+    throw new Response(
+      JSON.stringify({
+        message: "can't found the user",
+      }),
+      {
+        status: Number(statusCode),
+      },
+    );
+  }
+
+  if (responseType === 'return_response') {
+    return new Response(
+      JSON.stringify({
+        message: 'user modern.js',
+      }),
+      {
+        status: Number(statusCode),
+      },
+    );
+  }
+
+  return null;
 };
diff --git a/tests/integration/routes/src/three/routes/error/response/page.tsx b/tests/integration/routes/src/three/routes/error/response/page.tsx
@@ -1,10 +1,6 @@
-import { Outlet, useLoaderData } from '@modern-js/runtime/router';
+import { useLoaderData } from '@modern-js/runtime/router';
 
 export default function Page() {
   const data = useLoaderData();
-  return (
-    <div>
-      <Outlet />
-    </div>
-  );
+  return <div className="response-content">Response Page</div>;
 }
diff --git a/tests/integration/routes/tests/index.test.ts b/tests/integration/routes/tests/index.test.ts
@@ -361,27 +361,67 @@ const supportLoader = async (page: Page, errors: string[], appPort: number) => {
   expect(errors.length).toBe(0);
 };
 
+const supportThrowError = async (
+  page: Page,
+  errors: string[],
+  appPort: number,
+) => {
+  const response = await page.goto(
+    `http://localhost:${appPort}/three/error/response?type=throw_error`,
+    {
+      waitUntil: ['domcontentloaded'],
+    },
+  );
+  expect(response?.status()).toBe(200);
+  await page.waitForSelector('.error-content');
+  const errorStatusElm = await page.$('.error-content');
+  const text = await page.evaluate(el => el?.textContent, errorStatusElm);
+  expect(text?.includes('500')).toBeFalsy();
+  expect(text?.includes("can't found the user")).toBeTruthy();
+};
+
 const supportThrowResponse = async (
   page: Page,
   errors: string[],
   appPort: number,
+  code: number,
 ) => {
   const response = await page.goto(
-    `http://localhost:${appPort}/three/error/response`,
+    `http://localhost:${appPort}/three/error/response?type=throw_response&code=${code}`,
     {
       waitUntil: ['domcontentloaded'],
     },
   );
-  expect(response?.status()).toBe(255);
+  expect(response?.status()).toBe(200);
+  await page.waitForSelector('.response-status');
   const errorStatusElm = await page.$('.response-status');
   const text = await page.evaluate(el => el?.textContent, errorStatusElm);
-  expect(text?.includes('255')).toBeTruthy();
+  expect(text?.includes(`${code}`)).toBeTruthy();
   const errorContentElm = await page.$('.response-content');
   const text1 = await page.evaluate(el => el?.textContent, errorContentElm);
   expect(text1?.includes("can't found the user")).toBeTruthy();
   expect(errors.length).toBe(0);
 };
 
+const supportReturnResponse = async (
+  page: Page,
+  errors: string[],
+  appPort: number,
+  code: number,
+) => {
+  const response = await page.goto(
+    `http://localhost:${appPort}/three/error/response?type=return_response&code=${code}`,
+    {
+      waitUntil: ['domcontentloaded'],
+    },
+  );
+  expect(response?.status()).toBe(code);
+  await page.waitForSelector('.response-content');
+  const el = await page.$('.response-content');
+  const text = await page.evaluate(el => el?.textContent, el);
+  expect(text?.includes('Response Page')).toBeTruthy();
+};
+
 const supportLoaderForSSRAndCSR = async (
   page: Page,
   errors: string[],
@@ -770,8 +810,16 @@ describe('dev', () => {
       supportRedirectForSSR(page, errors, appPort));
     test('support redirect for csr', () =>
       supportRedirectForCSR(page, errors, appPort));
-    test('support throw response', async () =>
-      supportThrowResponse(page, errors, appPort));
+    test('support throw error', async () =>
+      supportThrowError(page, errors, appPort));
+    test('support throw response', async () => {
+      await supportThrowResponse(page, errors, appPort, 500);
+      await supportThrowResponse(page, errors, appPort, 200);
+    });
+    test('support return response', async () => {
+      await supportReturnResponse(page, errors, appPort, 500);
+      await supportReturnResponse(page, errors, appPort, 200);
+    });
   });
 
   describe('global configuration', () => {
@@ -912,8 +960,16 @@ describe('build', () => {
       supportRedirectForSSR(page, errors, appPort));
     test('support redirect for csr', () =>
       supportRedirectForCSR(page, errors, appPort));
-    test('support throw response', async () =>
-      supportThrowResponse(page, errors, appPort));
+    test('support throw error', async () =>
+      supportThrowError(page, errors, appPort));
+    test('support throw response', async () => {
+      await supportThrowResponse(page, errors, appPort, 500);
+      await supportThrowResponse(page, errors, appPort, 200);
+    });
+    test('support return response', async () => {
+      await supportReturnResponse(page, errors, appPort, 500);
+      await supportReturnResponse(page, errors, appPort, 200);
+    });
   });
 
   describe('global configuration', () => {
@@ -1056,8 +1112,16 @@ describe('dev with rspack', () => {
       supportRedirectForSSR(page, errors, appPort));
     test('support redirect for csr', () =>
       supportRedirectForCSR(page, errors, appPort));
-    test('support throw response', async () =>
-      supportThrowResponse(page, errors, appPort));
+    test('support throw error', async () =>
+      supportThrowError(page, errors, appPort));
+    test('support throw response', async () => {
+      await supportThrowResponse(page, errors, appPort, 500);
+      await supportThrowResponse(page, errors, appPort, 200);
+    });
+    test('support return response', async () => {
+      await supportReturnResponse(page, errors, appPort, 500);
+      await supportReturnResponse(page, errors, appPort, 200);
+    });
   });
 
   describe('global configuration', () => {
@@ -1202,8 +1266,16 @@ describe('build with rspack', () => {
       supportRedirectForSSR(page, errors, appPort));
     test('support redirect for csr', () =>
       supportRedirectForCSR(page, errors, appPort));
-    test('support throw response', async () =>
-      supportThrowResponse(page, errors, appPort));
+    test('support throw error', async () =>
+      supportThrowError(page, errors, appPort));
+    test('support throw response', async () => {
+      await supportThrowResponse(page, errors, appPort, 500);
+      await supportThrowResponse(page, errors, appPort, 200);
+    });
+    test('support return response', async () => {
+      await supportReturnResponse(page, errors, appPort, 500);
+      await supportReturnResponse(page, errors, appPort, 200);
+    });
   });
 
   describe('global configuration', () => {