Properly handle ErrorResponse inside resource routes (#6320)

.changeset/error-response-resource-route.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/server-runtime": patch
+---
+
+Properly handle thrown `ErrorResponse` instances inside resource routes

integration/resource-routes-test.ts

@@ -8,10 +8,13 @@ import { PlaywrightFixture } from "./helpers/playwright-fixture";
 test.describe("loader in an app", async () => {
   let appFixture: AppFixture;
   let fixture: Fixture;
+  let _consoleError: typeof console.error;
 
   let SVG_CONTENTS = `<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 100 100" fill="none" stroke="#000" stroke-width="4" aria-label="Chicken"><path d="M48.1 34C22.1 32 1.4 51 2.5 67.2c1.2 16.1 19.8 17 29 17.8H89c15.7-6.6 6.3-18.9.3-20.5A28 28 0 0073 41.7c-.5-7.2 3.4-11.6 6.9-15.3 8.5 2.6 8-8 .8-7.2.6-6.5-12.3-5.9-6.7 2.7l-3.7 5c-6.9 5.4-10.9 5.1-22.2 7zM48.1 34c-38 31.9 29.8 58.4 25 7.7M70.3 26.9l5.4 4.2"/></svg>`;
 
   test.beforeAll(async () => {
+    _consoleError = console.error;
+    console.error = () => {};
     fixture = await createFixture({
       future: { v2_routeConvention: true },
       files: {
@@ -25,6 +28,9 @@ test.describe("loader in an app", async () => {
                 <input name="destination" defaultValue="/redirect-destination" />
                 <button type="submit">Redirect</button>
               </Form>
+              <Form action="/no-action" method="post">
+                <button type="submit">Submit to no action route</button>
+              </Form>
             </>
           )
         `,
@@ -94,13 +100,20 @@ test.describe("loader in an app", async () => {
             throw { but: 'why' };
           }
         `,
+        "app/routes/no-action.jsx": js`
+          import { json } from "@remix-run/node";
+          export let loader = () => {
+            return json({ ok: true });
+          }
+        `,
       },
     });
     appFixture = await createAppFixture(fixture, ServerMode.Test);
   });
 
   test.afterAll(() => {
     appFixture.close();
+    console.error = _consoleError;
   });
 
   test.describe("with JavaScript", () => {
@@ -190,6 +203,28 @@ test.describe("loader in an app", async () => {
       "Unexpected Server Error\n\n[object Object]"
     );
   });
+
+  test("should handle ErrorResponses thrown from resource routes on document requests", async () => {
+    let res = await fixture.postDocument("/no-action", new FormData());
+    expect(res.status).toBe(405);
+    expect(res.statusText).toBe("Method Not Allowed");
+    expect(await res.text()).toBe('{"message":"Unexpected Server Error"}');
+  });
+
+  test("should handle ErrorResponses thrown from resource routes on client submissions", async ({
+    page,
+  }) => {
+    let logs: string[] = [];
+    page.on("console", (msg) => logs.push(msg.text()));
+    let app = new PlaywrightFixture(appFixture, page);
+    await app.goto("/");
+    await app.clickSubmitButton("/no-action");
+    let html = await app.getHtml();
+    expect(html).toMatch("Application Error");
+    expect(logs[0]).toContain(
+      'Route "routes/no-action" does not have an action'
+    );
+  });
 });
 
 test.describe("Development server", async () => {

packages/remix-server-runtime/server.ts

@@ -1,5 +1,6 @@
 import type {
   UNSAFE_DeferredData as DeferredData,
+  ErrorResponse,
   StaticHandler,
   StaticHandlerContext,
 } from "@remix-run/router";
@@ -8,6 +9,7 @@ import {
   getStaticContextFromError,
   isRouteErrorResponse,
   createStaticHandler,
+  json as routerJson,
 } from "@remix-run/router";
 
 import type { AppLoadContext } from "./data";
@@ -23,7 +25,6 @@ import type { ServerRouteManifest } from "./routes";
 import { createStaticHandlerDataRoutes, createRoutes } from "./routes";
 import {
   createDeferredReadableStream,
-  json,
   isRedirectResponse,
   isResponse,
 } from "./responses";
@@ -158,18 +159,16 @@ async function handleDataRequestRR(
       return error;
     }
 
-    let status = isRouteErrorResponse(error) ? error.status : 500;
-    let errorInstance =
-      isRouteErrorResponse(error) && error.error
-        ? error.error
-        : error instanceof Error
-        ? error
-        : new Error("Unexpected Server Error");
+    if (isRouteErrorResponse(error)) {
+      logServerErrorIfNotAborted(error.error || error, request, serverMode);
+      return errorResponseToJson(error, serverMode);
+    }
 
+    let errorInstance =
+      error instanceof Error ? error : new Error("Unexpected Server Error");
     logServerErrorIfNotAborted(errorInstance, request, serverMode);
-
-    return json(serializeError(errorInstance, serverMode), {
-      status,
+    return routerJson(serializeError(errorInstance, serverMode), {
+      status: 500,
       headers: {
         "X-Remix-Error": "yes",
       },
@@ -359,6 +358,12 @@ async function handleResourceRequestRR(
       error.headers.set("X-Remix-Catch", "yes");
       return error;
     }
+
+    if (isRouteErrorResponse(error)) {
+      logServerErrorIfNotAborted(error.error || error, request, serverMode);
+      return errorResponseToJson(error, serverMode);
+    }
+
     logServerErrorIfNotAborted(error, request, serverMode);
     return returnLastResortErrorResponse(error, serverMode);
   }
@@ -374,6 +379,25 @@ function logServerErrorIfNotAborted(
   }
 }
 
+function errorResponseToJson(
+  errorResponse: ErrorResponse,
+  serverMode: ServerMode
+): Response {
+  return routerJson(
+    serializeError(
+      errorResponse.error || new Error("Unexpected Server Error"),
+      serverMode
+    ),
+    {
+      status: errorResponse.status,
+      statusText: errorResponse.statusText,
+      headers: {
+        "X-Remix-Error": "yes",
+      },
+    }
+  );
+}
+
 function returnLastResortErrorResponse(error: any, serverMode?: ServerMode) {
   let message = "Unexpected Server Error";