fix(remix-dev): update handleError to properly receive ErrorResponse instances

.changeset/handle-error-response.md

@@ -0,0 +1,5 @@
+---
+"@remix-run/server-runtime": patch
+---
+
+Fix `handleError` method to correctly receive `ErrorResponse` instances on `?_data` and resource route requests. It now receives the `ErrorResponse` instance the same way a document request would.  Users can leverage `isRouteErrorResponse`to detect these error instances and log accordingly.

integration/error-sanitization-test.ts

@@ -500,7 +500,11 @@ test.describe("Error Sanitization", () => {
               ) {
                 console.error("App Specific Error Logging:");
                 console.error("  Request: " + request.method + " " + request.url);
-                if (error instanceof Error) {
+                if (isRouteErrorResponse(error)) {
+                  console.error("  Status: " + error.status + " " + error.statusText);
+                  console.error("  Error: " + error.error.message);
+                  console.error("  Stack: " + error.error.stack);
+                } else if (error instanceof Error) {
                   console.error("  Error: " + error.message);
                   console.error("  Stack: " + error.stack);
                 } else {
@@ -647,11 +651,12 @@ test.describe("Error Sanitization", () => {
       expect(errorLogs[1][0]).toEqual(
         "  Request: GET test://test/?_data=not-a-route"
       );
-      expect(errorLogs[2][0]).toEqual(
+      expect(errorLogs[2][0]).toEqual("  Status: 403 Forbidden");
+      expect(errorLogs[3][0]).toEqual(
         '  Error: Route "not-a-route" does not match URL "/"'
       );
-      expect(errorLogs[3][0]).toMatch(" at ");
-      expect(errorLogs.length).toBe(4);
+      expect(errorLogs[4][0]).toMatch(" at ");
+      expect(errorLogs.length).toBe(5);
     });
   });
 });

packages/remix-server-runtime/__tests__/handle-error-test.ts

@@ -0,0 +1,226 @@
+import { ErrorResponse } from "@remix-run/router";
+
+import type { ServerBuild } from "../build";
+import { createRequestHandler } from "../server";
+import { json } from "../responses";
+
+function getHandler(routeModule = {}, entryServerModule = {}) {
+  let routeId = "root";
+  let handleErrorSpy = jest.fn();
+  let build = {
+    routes: {
+      [routeId]: {
+        id: routeId,
+        path: "/",
+        module: {
+          default() {},
+          ...routeModule,
+        },
+      },
+    },
+    entry: {
+      module: {
+        handleError: handleErrorSpy,
+        default() {},
+        ...entryServerModule,
+      },
+    },
+  } as unknown as ServerBuild;
+
+  return {
+    handler: createRequestHandler(build),
+    handleErrorSpy,
+  };
+}
+
+describe("handleError", () => {
+  describe("document request", () => {
+    it("provides user-thrown Error", async () => {
+      let error = new Error("💥");
+      let { handler, handleErrorSpy } = getHandler({
+        loader() {
+          throw error;
+        },
+      });
+      let request = new Request("http://example.com/");
+      await handler(request);
+      expect(handleErrorSpy).toHaveBeenCalledWith(error, {
+        request,
+        params: {},
+        context: {},
+      });
+    });
+
+    it("provides router-thrown ErrorResponse", async () => {
+      let { handler, handleErrorSpy } = getHandler({});
+      let request = new Request("http://example.com/", { method: "post" });
+      await handler(request);
+      expect(handleErrorSpy).toHaveBeenCalledWith(
+        new ErrorResponse(
+          405,
+          "Method Not Allowed",
+          new Error(
+            'You made a POST request to "/" but did not provide an `action` for route "root", so there is no way to handle the request.'
+          ),
+          true
+        ),
+        {
+          request,
+          params: {},
+          context: {},
+        }
+      );
+    });
+
+    it("provides render-thrown Error", async () => {
+      let { handler, handleErrorSpy } = getHandler(undefined, {
+        default() {
+          throw new Error("Render error");
+        },
+      });
+      let request = new Request("http://example.com/");
+      await handler(request);
+      expect(handleErrorSpy).toHaveBeenCalledWith(new Error("Render error"), {
+        request,
+        params: {},
+        context: {},
+      });
+    });
+
+    it("does not provide user-thrown Responses to handleError", async () => {
+      let { handler, handleErrorSpy } = getHandler({
+        loader() {
+          throw json(
+            { message: "not found" },
+            { status: 404, statusText: "Not Found" }
+          );
+        },
+      });
+      let request = new Request("http://example.com/");
+      await handler(request);
+      expect(handleErrorSpy).not.toHaveBeenCalled();
+    });
+  });
+
+  describe("data request", () => {
+    it("provides user-thrown Error", async () => {
+      let error = new Error("💥");
+      let { handler, handleErrorSpy } = getHandler({
+        loader() {
+          throw error;
+        },
+      });
+      let request = new Request("http://example.com/?_data=root");
+      await handler(request);
+      expect(handleErrorSpy).toHaveBeenCalledWith(error, {
+        request,
+        params: {},
+        context: {},
+      });
+    });
+
+    it("provides router-thrown ErrorResponse", async () => {
+      let { handler, handleErrorSpy } = getHandler({});
+      let request = new Request("http://example.com/?_data=root", {
+        method: "post",
+      });
+      await handler(request);
+      expect(handleErrorSpy).toHaveBeenCalledWith(
+        new ErrorResponse(
+          405,
+          "Method Not Allowed",
+          new Error(
+            'You made a POST request to "/" but did not provide an `action` for route "root", so there is no way to handle the request.'
+          ),
+          true
+        ),
+        {
+          request,
+          params: {},
+          context: {},
+        }
+      );
+    });
+
+    it("does not provide user-thrown Responses to handleError", async () => {
+      let { handler, handleErrorSpy } = getHandler({
+        loader() {
+          throw json(
+            { message: "not found" },
+            { status: 404, statusText: "Not Found" }
+          );
+        },
+      });
+      let request = new Request("http://example.com/?_data=root");
+      await handler(request);
+      expect(handleErrorSpy).not.toHaveBeenCalled();
+    });
+  });
+
+  describe("resource request", () => {
+    it("provides user-thrown Error", async () => {
+      let error = new Error("💥");
+      let { handler, handleErrorSpy } = getHandler({
+        loader() {
+          throw error;
+        },
+        default: null,
+      });
+      let request = new Request("http://example.com/");
+      await handler(request);
+      expect(handleErrorSpy).toHaveBeenCalledWith(error, {
+        request,
+        params: {},
+        context: {},
+      });
+    });
+
+    it("provides router-thrown ErrorResponse", async () => {
+      let { handler, handleErrorSpy } = getHandler({ default: null });
+      let request = new Request("http://example.com/", {
+        method: "post",
+      });
+      await handler(request);
+      expect(handleErrorSpy).toHaveBeenCalledWith(
+        new ErrorResponse(
+          405,
+          "Method Not Allowed",
+          new Error(
+            'You made a POST request to "/" but did not provide an `action` for route "root", so there is no way to handle the request.'
+          ),
+          true
+        ),
+        {
+          request,
+          params: {},
+          context: {},
+        }
+      );
+    });
+
+    it("does not provide user-thrown Responses to handleError", async () => {
+      let { handler, handleErrorSpy } = getHandler({
+        loader() {
+          throw json(
+            { message: "not found" },
+            { status: 404, statusText: "Not Found" }
+          );
+        },
+        default: null,
+      });
+      let request = new Request("http://example.com/");
+      await handler(request);
+      expect(handleErrorSpy).not.toHaveBeenCalled();
+    });
+  });
+});
+
+// let request = new Request(
+//   "http://example.com/random?_data=routes/random&foo=bar",
+//   {
+//     method: "post",
+//     // headers: {
+//     //   "Content-Type": "application/json",
+//     // },
+//   }
+// );

packages/remix-server-runtime/server.ts

@@ -51,7 +51,7 @@ export const createRequestHandler: CreateRequestHandlerFunction = (
     build.entry.module.handleError ||
     ((error, { request }) => {
       if (serverMode !== ServerMode.Test && !request.signal.aborted) {
-        console.error(error);
+        console.error(isRouteErrorResponse(error) ? error.error : error);
       }
     });
 
@@ -182,8 +182,8 @@ async function handleDataRequestRR(
     }
 
     if (isRouteErrorResponse(error)) {
-      if (error.error) {
-        handleError(error.error);
+      if (error) {
+        handleError(error);
       }
       return errorResponseToJson(error, serverMode);
     }
@@ -336,8 +336,8 @@ async function handleResourceRequestRR(
     }
 
     if (isRouteErrorResponse(error)) {
-      if (error.error) {
-        handleError(error.error);
+      if (error) {
+        handleError(error);
       }
       return errorResponseToJson(error, serverMode);
     }