Rename from handleError -> onUnhandledError

remix-run · brophdawg11 · Jun 1, 2023 · May 26, 2023 · May 26, 2023 · May 26, 2023
commit 049bd04d83d9829ea709faae57933160ba80b3df
diff --git a/.changeset/handle-error.md b/.changeset/handle-error.md
diff --git a/.changeset/on-unhandled-error.md b/.changeset/on-unhandled-error.md
@@ -0,0 +1,22 @@
+---
+"@remix-run/server-runtime": minor
+---
+
+Add optional `onUnhandledError` export for custom server-side error processing.  This is a new optional export from your `entry.server.tsx` that will be called with any encountered error on the Remix server (loader, action, or render error):
+
+```ts
+// entry.server.tsx
+export function onUnhandledError(
+  error: Error | unknown,
+  { request, params, context }: DataFunctionArgs
+): void {
+  if (error instanceof Error) {
+    sendErrorToBugReportingService(error);
+    console.error(formatError(error));
+  } else {
+    let unknownError = new Error("Unknown Server Error");
+    sendErrorToBugReportingService(unknownError);
+    console.error(unknownError);
+  }
+}
+```
diff --git a/docs/file-conventions/entry.server.md b/docs/file-conventions/entry.server.md
@@ -5,10 +5,38 @@ toc: false
 
 # entry.server
 
-By default, Remix will handle generating the HTTP Response for you. If you want to customize this behavior, you can run `npx remix reveal` to generate a `app/entry.server.tsx` (or `.jsx`) that will take precedence. The `default` export of this module is a function that lets you create the response, including HTTP status, headers, and HTML, giving you full control over the way the markup is generated and sent to the client.
+By default, Remix will handle generating the HTTP Response for you. If you want to customize this behavior, you can run `npx remix reveal` to generate an `app/entry.server.tsx` (or `.jsx`) that will take precedence. The `default` export of this module is a function that lets you create the response, including HTTP status, headers, and HTML, giving you full control over the way the markup is generated and sent to the client.
 
 This module should render the markup for the current page using a `<RemixServer>` element with the `context` and `url` for the current request. This markup will (optionally) be re-hydrated once JavaScript loads in the browser using the [browser entry module][browser-entry-module].
 
-You can also export an optional `handleDataRequest` function that will allow you to modify the response of a data request. These are the requests that do not render HTML, but rather return the loader and action data to the browser once client-side hydration has occurred.
+## `handleDataRequest`
+
+You can export an optional `handleDataRequest` function that will allow you to modify the response of a data request. These are the requests that do not render HTML, but rather return the loader and action data to the browser once client-side hydration has occurred.
+
+```tsx
+export function handleDataRequest(
+  response: Response,
+  { request, params, context }: DataFunctionArgs
+) {
+  response.headers.set("X-Custom-Header", "value");
+  return response;
+}
+```
+
+## `onUnhandledError`
+
+By default, Remix will log encountered server-side errors to the console. If you'd like more control over the logging, or would like to also report these errors to an external service, then you can export an optional `onUnhandledError` function which will give you control (and will disable the built-in error logging).
+
+```tsx
+export function onUnhandledError(
+  error: Error | unknown,
+  { request, params, context }: DataFunctionArgs
+) {
+  sendErrorToErrorReportingService(error);
+  console.error(formatErrorForJsonLogging(error));
+}
+```
+
+Note that this does not handle thrown `Response` instances from your `loader`/`action` functions. The intention of this handler is to find bugs in your code which result in unexpected thrown errors. If you are detecting a scenario and throwing a 401/404/etc. `Response` in your `loader`/`action` then it's an expected flow that is handled by your code. If you also wish to log, or send those to an external service, that should be done at the time you throw the response.
 
 [browser-entry-module]: ./entry.client
diff --git a/integration/error-sanitization-test.ts b/integration/error-sanitization-test.ts
@@ -27,7 +27,7 @@ const routeFiles = {
   `,
 
   "app/routes/index.jsx": js`
-    import { useLoaderData, useLocation } from "@remix-run/react";
+    import { useLoaderData, useLocation, useRouteError } from "@remix-run/react";
 
     export function loader({ request }) {
       if (new URL(request.url).searchParams.has('loader')) {
@@ -52,7 +52,8 @@ const routeFiles = {
       );
     }
 
-    export function ErrorBoundary({ error }) {
+    export function ErrorBoundary() {
+      let error = useRouteError();
       return (
         <>
           <h1>Index Error</h1>
@@ -66,7 +67,7 @@ const routeFiles = {
   "app/routes/defer.jsx": js`
     import * as React from 'react';
     import { defer } from "@remix-run/server-runtime";
-    import { Await, useLoaderData, useRouteError } from "@remix-run/react";
+    import { Await, useAsyncError, useLoaderData, useRouteError  } from "@remix-run/react";
 
     export function loader({ request }) {
       if (new URL(request.url).searchParams.has('loader')) {
@@ -95,19 +96,20 @@ const routeFiles = {
     }
 
     function AwaitError() {
-      let error = useRouteError();
+      let error = useAsyncError();
       return (
         <>
           <h2>Defer Error</h2>
-          <p>{error}</p>
+          <p>{error.message}</p>
         </>
       );
     }
 
-    export function ErrorBoundary({ error }) {
+    export function ErrorBoundary() {
+      let error = useRouteError();
       return (
         <>
-          <h1>Index Error</h1>
+          <h1>Defer Error</h1>
           <p>{"MESSAGE:" + error.message}</p>
           {error.stack ? <p>{"STACK:" + error.stack}</p> : null}
         </>
@@ -144,6 +146,11 @@ test.describe("Error Sanitization", () => {
     test.beforeAll(async () => {
       fixture = await createFixture(
         {
+          config: {
+            future: {
+              v2_errorBoundary: true,
+            },
+          },
           files: routeFiles,
         },
         ServerMode.Production
@@ -278,6 +285,11 @@ test.describe("Error Sanitization", () => {
     test.beforeAll(async () => {
       fixture = await createFixture(
         {
+          config: {
+            future: {
+              v2_errorBoundary: true,
+            },
+          },
           files: routeFiles,
         },
         ServerMode.Development
@@ -418,10 +430,15 @@ test.describe("Error Sanitization", () => {
     });
   });
 
-  test.describe("serverMode=production (user-provided handleError)", () => {
+  test.describe("serverMode=production (user-provided onUnhandledError)", () => {
     test.beforeAll(async () => {
       fixture = await createFixture(
         {
+          config: {
+            future: {
+              v2_errorBoundary: true,
+            },
+          },
           files: {
             "app/entry.server.tsx": js`
               import type { EntryContext } from "@remix-run/node";
@@ -446,13 +463,13 @@ test.describe("Error Sanitization", () => {
                 });
               }
 
-              export function handleError(error: unknown, { request }: { request: Request }) {
+              export function onUnhandledError(
+                error: Error | unknown,
+                { request }: { request: Request },
+              ) {
                 console.error("App Specific Error Logging:");
                 console.error("  Request: " + request.method + " " + request.url);
-                let msg;
-                if (isRouteErrorResponse(error)) {
-                  console.error("  Error: " + error.status + " " + error.statusText);
-                } else if (error instanceof Error) {
+                if (error instanceof Error) {
                   console.error("  Error: " + error.message);
                   console.error("  Stack: " + error.stack);
                 } else {

diff --git a/packages/remix-server-runtime/build.ts b/packages/remix-server-runtime/build.ts
@@ -32,10 +32,8 @@ export interface HandleDataRequestFunction {
   (response: Response, args: DataFunctionArgs): Promise<Response> | Response;
 }
 
-// TODO: Should we make this async?  If we do, do we need to use .catch()
-// to avoid dangling promises?  I'd rather leave that in user land...
-export interface HandleErrorFunction {
-  (error: unknown, args: DataFunctionArgs): void;
+export interface OnUnhandledErrorFunction {
+  (error: Error | unknown, args: DataFunctionArgs): void;
 }
 
 /**
@@ -45,5 +43,5 @@ export interface HandleErrorFunction {
 export interface ServerEntryModule {
   default: HandleDocumentRequestFunction;
   handleDataRequest?: HandleDataRequestFunction;
-  handleError?: HandleErrorFunction;
+  onUnhandledError?: OnUnhandledErrorFunction;
 }
diff --git a/packages/remix-server-runtime/server.ts b/packages/remix-server-runtime/server.ts
@@ -50,7 +50,7 @@ export const createRequestHandler: CreateRequestHandlerFunction = (
   let staticHandler = createStaticHandler(dataRoutes);
 
   let errorHandler =
-    build.entry.module.handleError ||
+    build.entry.module.onUnhandledError ||
     ((error, { request }) => {
       if (serverMode !== ServerMode.Test && !request.signal.aborted) {
         console.error(error);
@@ -130,7 +130,7 @@ async function handleDataRequestRR(
   routeId: string,
   request: Request,
   loadContext: AppLoadContext,
-  handleError: (err: unknown) => void
+  handleError: (err: Error | unknown) => void
 ) {
   try {
     let response = await staticHandler.queryRoute(request, {
@@ -178,7 +178,9 @@ async function handleDataRequestRR(
     }
 
     if (isRouteErrorResponse(error)) {
-      handleError(error.error || error);
+      if (error.error) {
+        handleError(error.error);
+      }
       return errorResponseToJson(error, serverMode);
     }
 
@@ -267,7 +269,11 @@ async function handleDocumentRequestRR(
 
   // Sanitize errors outside of development environments
   if (context.errors) {
-    Object.values(context.errors).forEach((err) => handleError(err));
+    Object.values(context.errors).forEach((err) => {
+      if (!isRouteErrorResponse(err) || err.error) {
+        handleError(err);
+      }
+    });
     context.errors = sanitizeErrors(context.errors, serverMode);
   }
 
@@ -383,7 +389,9 @@ async function handleResourceRequestRR(
     }
 
     if (isRouteErrorResponse(error)) {
-      handleError(error.error || error);
+      if (error.error) {
+        handleError(error.error);
+      }
       return errorResponseToJson(error, serverMode);
     }