Unwrap thrown Responses from entry.server into ErrorResponses (#8577)

remix-run · Jan 29, 2024 · 96904e7 · 96904e7
1 parent eb4766c
commit 96904e7
Show file tree

Hide file tree

Showing 8 changed files with 103 additions and 25 deletions.
diff --git a/.changeset/silver-crews-arrive.md b/.changeset/silver-crews-arrive.md
@@ -0,0 +1,5 @@
+---
+"@remix-run/server-runtime": patch
+---
+
+Unwrap thrown `Response`'s from `entry.server` into `ErrorResponse`s and preserve the status code
diff --git a/packages/remix-dev/package.json b/packages/remix-dev/package.json
@@ -29,7 +29,7 @@
     "@mdx-js/mdx": "^2.3.0",
     "@npmcli/package-json": "^4.0.1",
     "@remix-run/node": "2.5.1",
-    "@remix-run/router": "0.0.0-experimental-c9f8a7b2",
+    "@remix-run/router": "0.0.0-experimental-add6f8aa",
     "@remix-run/server-runtime": "2.5.1",
     "@types/mdx": "^2.0.5",
     "@vanilla-extract/integration": "^6.2.0",

diff --git a/packages/remix-react/package.json b/packages/remix-react/package.json
@@ -16,10 +16,10 @@
   "typings": "dist/index.d.ts",
   "module": "dist/esm/index.js",
   "dependencies": {
-    "@remix-run/router": "0.0.0-experimental-c9f8a7b2",
+    "@remix-run/router": "0.0.0-experimental-add6f8aa",
     "@remix-run/server-runtime": "2.5.1",
-    "react-router": "0.0.0-experimental-c9f8a7b2",
-    "react-router-dom": "0.0.0-experimental-c9f8a7b2"
+    "react-router": "0.0.0-experimental-add6f8aa",
+    "react-router-dom": "0.0.0-experimental-add6f8aa"
   },
   "devDependencies": {
     "@testing-library/jest-dom": "^5.17.0",

diff --git a/packages/remix-server-runtime/__tests__/server-test.ts b/packages/remix-server-runtime/__tests__/server-test.ts
@@ -1856,6 +1856,7 @@ describe("shared server runtime", () => {
         },
         "routes/_index": {
           parentId: "root",
+          index: true,
           default: {},
           loader: indexLoader,
         },
@@ -1867,11 +1868,11 @@ describe("shared server runtime", () => {
           throw new Error("thrown");
         }
         calledBefore = true;
-        return ogHandleDocumentRequest.call(null, arguments);
+        return ogHandleDocumentRequest.call(null, ...arguments);
       }) as any;
       let handler = createRequestHandler(build, ServerMode.Development);
 
-      let request = new Request(`${baseUrl}/`, { method: "get" });
+      let request = new Request(`${baseUrl}/404`, { method: "get" });
 
       let result = await handler(request);
       expect(result.status).toBe(500);
@@ -1886,6 +1887,47 @@ describe("shared server runtime", () => {
       expect(context.loaderData).toEqual({});
     });
 
+    test("unwraps responses thrown from handleDocumentRequest", async () => {
+      let rootLoader = jest.fn(() => {
+        return "root";
+      });
+      let indexLoader = jest.fn(() => {
+        return "index";
+      });
+      let build = mockServerBuild({
+        root: {
+          default: {},
+          loader: rootLoader,
+          ErrorBoundary: {},
+        },
+        "routes/_index": {
+          parentId: "root",
+          index: true,
+          default: {},
+          loader: indexLoader,
+        },
+      });
+      let ogHandleDocumentRequest = build.entry.module.default;
+      build.entry.module.default = function (
+        _: Request,
+        responseStatusCode: number
+      ) {
+        if (responseStatusCode === 200) {
+          throw new Response("Uh oh!", {
+            status: 400,
+            statusText: "Bad Request",
+          });
+        }
+        return ogHandleDocumentRequest.call(null, ...arguments);
+      } as any;
+      let handler = createRequestHandler(build, ServerMode.Development);
+
+      let request = new Request(`${baseUrl}/`, { method: "get" });
+
+      let result = await handler(request);
+      expect(result.status).toBe(400);
+    });
+
     test("returns generic message if handleDocumentRequest throws a second time", async () => {
       let rootLoader = jest.fn(() => {
         return "root";

diff --git a/packages/remix-server-runtime/package.json b/packages/remix-server-runtime/package.json
@@ -16,7 +16,7 @@
   "typings": "dist/index.d.ts",
   "module": "dist/esm/index.js",
   "dependencies": {
-    "@remix-run/router": "0.0.0-experimental-c9f8a7b2",
+    "@remix-run/router": "0.0.0-experimental-add6f8aa",
     "@types/cookie": "^0.6.0",
     "@web3-storage/multipart-parser": "^1.0.0",
     "cookie": "^0.6.0",

diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
@@ -9,6 +9,7 @@ import {
   isRouteErrorResponse,
   createStaticHandler,
   json as routerJson,
+  UNSAFE_ErrorResponseImpl as ErrorResponseImpl,
 } from "@remix-run/router";
 
 import type { AppLoadContext } from "./data";
@@ -323,11 +324,41 @@ async function handleDocumentRequestRR(
   } catch (error: unknown) {
     handleError(error);
 
+    let errorForSecondRender = error;
+
+    // If they threw a response, unwrap it into an ErrorResponse like we would
+    // have for a loader/action
+    if (isResponse(error)) {
+      let data;
+      try {
+        let contentType = error.headers.get("Content-Type");
+        // Check between word boundaries instead of startsWith() due to the last
+        // paragraph of https://httpwg.org/specs/rfc9110.html#field.content-type
+        if (contentType && /\bapplication\/json\b/.test(contentType)) {
+          if (error.body == null) {
+            data = null;
+          } else {
+            data = await error.json();
+          }
+        } else {
+          data = await error.text();
+        }
+
+        errorForSecondRender = new ErrorResponseImpl(
+          error.status,
+          error.statusText,
+          data
+        );
+      } catch (e) {
+        // If we can't unwrap the response - just leave it as-is
+      }
+    }
+
     // Get a new StaticHandlerContext that contains the error at the right boundary
     context = getStaticContextFromError(
       staticHandler.dataRoutes,
       context,
-      error
+      errorForSecondRender
     );
 
     // Sanitize errors outside of development environments

diff --git a/packages/remix-testing/package.json b/packages/remix-testing/package.json
@@ -18,8 +18,8 @@
   "dependencies": {
     "@remix-run/node": "2.5.1",
     "@remix-run/react": "2.5.1",
-    "@remix-run/router": "0.0.0-experimental-c9f8a7b2",
-    "react-router-dom": "0.0.0-experimental-c9f8a7b2"
+    "@remix-run/router": "0.0.0-experimental-add6f8aa",
+    "react-router-dom": "0.0.0-experimental-add6f8aa"
   },
   "devDependencies": {
     "@types/node": "^18.17.1",

diff --git a/yarn.lock b/yarn.lock
@@ -2489,10 +2489,10 @@
     "@changesets/types" "^5.0.0"
     dotenv "^8.1.0"
 
-"@remix-run/router@0.0.0-experimental-c9f8a7b2":
-  version "0.0.0-experimental-c9f8a7b2"
-  resolved "https://registry.npmjs.org/@remix-run/router/-/router-0.0.0-experimental-c9f8a7b2.tgz#62b3a99be484463cf671086bfacc62ba2e65c584"
-  integrity sha512-B8zLtDP53mneC4ouOZyPSvtNyO7R3ZeV4ELQ7SC2yeoqlkuxOb2jg4HWCpkoSAMyqW9bhKi/KHA6nX4BJJBDBQ==
+"@remix-run/router@0.0.0-experimental-add6f8aa":
+  version "0.0.0-experimental-add6f8aa"
+  resolved "https://registry.npmjs.org/@remix-run/router/-/router-0.0.0-experimental-add6f8aa.tgz#58232edf157a762ac8d9d50c3d1d43e434791ab6"
+  integrity sha512-jVZ8xXRUXu6MoRMVsAZMmlf1ek7kFLgiY/5ZHaBY6VEBLc1J1nNLAtD1LzSJYE7sB1nBoF3ysNWpZ+QfAb3KXA==
 
 "@remix-run/web-blob@^3.1.0":
   version "3.1.0"
@@ -11307,20 +11307,20 @@ react-refresh@^0.14.0:
   resolved "https://registry.npmjs.org/react-refresh/-/react-refresh-0.14.0.tgz"
   integrity sha512-wViHqhAd8OHeLS/IRMJjTSDHF3U9eWi62F/MledQGPdJGDhodXJ9PBLNGr6WWL7qlH12Mt3TyTpbS+hGXMjCzQ==
 
-react-router-dom@0.0.0-experimental-c9f8a7b2:
-  version "0.0.0-experimental-c9f8a7b2"
-  resolved "https://registry.npmjs.org/react-router-dom/-/react-router-dom-0.0.0-experimental-c9f8a7b2.tgz#2102eb615cfecbab432b0bac5a8ee1e7409393c4"
-  integrity sha512-g1wKTTvBM+WExUiywAd9f9mZipdhtdGRssKqz+tLIMKuWcJ4Y3Z0nXsz1OUZ+QdQr7fNxrsRcEksWo9qbtTlfw==
+react-router-dom@0.0.0-experimental-add6f8aa:
+  version "0.0.0-experimental-add6f8aa"
+  resolved "https://registry.npmjs.org/react-router-dom/-/react-router-dom-0.0.0-experimental-add6f8aa.tgz#89d6fb5f8f9bf6c739584866e592f1345dcaa555"
+  integrity sha512-n9uf5z6OHTIiRzs6nQde4reA61dB4YWZQSJI4ouIW0ZUvcnE6nA6zoR9qF7Y/CFR7lJJ/DtTyj5xNY8I04pL6w==
   dependencies:
-    "@remix-run/router" "0.0.0-experimental-c9f8a7b2"
-    react-router "0.0.0-experimental-c9f8a7b2"
+    "@remix-run/router" "0.0.0-experimental-add6f8aa"
+    react-router "0.0.0-experimental-add6f8aa"
 
-react-router@0.0.0-experimental-c9f8a7b2:
-  version "0.0.0-experimental-c9f8a7b2"
-  resolved "https://registry.npmjs.org/react-router/-/react-router-0.0.0-experimental-c9f8a7b2.tgz#c3d1553364f7ff5399cac24eaa5c80c69f73e82f"
-  integrity sha512-BWDitTIQpn+aqImg7OpY6He7tzgRgE/O0hR/AR+61GvFGx79vKNJa2QDJBn148e6C3sud5FDtQ4pXfb5+yclvg==
+react-router@0.0.0-experimental-add6f8aa:
+  version "0.0.0-experimental-add6f8aa"
+  resolved "https://registry.npmjs.org/react-router/-/react-router-0.0.0-experimental-add6f8aa.tgz#06ed3e248c39cbdd52aef6be9a82fe5ee4067219"
+  integrity sha512-CusUxuUcU3QKX4Di33Tl6DDyQMd02gfNTIBx8fSYKWFoyWeJ/Pf4nF7HkBbQPkBPiTUBgE7MWHynp4JhRzXaYw==
   dependencies:
-    "@remix-run/router" "0.0.0-experimental-c9f8a7b2"
+    "@remix-run/router" "0.0.0-experimental-add6f8aa"
 
 react@^18.2.0:
   version "18.2.0"