diff --git a/.changeset/log-server-errors.md b/.changeset/log-server-errors.md new file mode 100644 index 00000000000..5d646cf5f61 --- /dev/null +++ b/.changeset/log-server-errors.md @@ -0,0 +1,5 @@ +--- +"@remix-run/server-runtime": patch +--- + +Ensure un-sanitized server errors are logged on the server during document requests diff --git a/.changeset/on-unhandled-error.md b/.changeset/on-unhandled-error.md new file mode 100644 index 00000000000..ebd7f740166 --- /dev/null +++ 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: 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 index cc25b6729f5..4019095551d 100644 --- a/docs/file-conventions/entry.server.md +++ b/docs/file-conventions/entry.server.md @@ -5,10 +5,53 @@ 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 `` 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: unknown, + { request, params, context }: DataFunctionArgs +) { + sendErrorToErrorReportingService(error); + console.error(formatErrorForJsonLogging(error)); +} +``` + +### Streaming Rendering Errors + +When you are streaming your HTML responses via [`renderToPipeableStream`][rendertopipeablestream] or [`renderToReadableStream`][rendertoreadablestream], your own `onUnhandledError` implementation will only handle errors encountered uring the initial shell render. If you encounter a rendering error during subsequent streamed rendering you will need handle these errors manually since the Remix server has already sent the Response by that point. + +- For `renderToPipeableStream`, you can handle these errors in the `onError` callback function. You will need to toggle a boolean when the in `onShellReady` so you know if the error was a shell rendering error (and can be ignored) or an async rendering error (and must be handled). + - For an exmaple, please see the default [`entry.server.tsx`][node-streaming-entry-server] for Node. +- For `renderToReadableStream`, you can handle these errors in the `onError` callback function + - For an example, please see the default [`entry.server.tsx`][cloudflare-streaming-entry-server] for Cloudflare + +### Thrown Responses + +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 +[rendertopipeablestream]: https://react.dev/reference/react-dom/server/renderToPipeableStream +[rendertoreadablestream]: https://react.dev/reference/react-dom/server/renderToReadableStream +[node-streaming-entry-server]: https://github.com/remix-run/remix/blob/main/packages/remix-dev/config/defaults/node/entry.server.react-stream.tsx +[cloudflare-streaming-entry-server]: https://github.com/remix-run/remix/blob/main/packages/remix-dev/config/defaults/cloudflare/entry.server.react-stream.tsx diff --git a/integration/error-sanitization-test.ts b/integration/error-sanitization-test.ts index 4ed62a49faa..ad918301485 100644 --- 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 ( <>

Index Error

@@ -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 ( <>

Defer Error

-

{error}

+

{error.message}

); } - export function ErrorBoundary({ error }) { + export function ErrorBoundary() { + let error = useRouteError(); return ( <> -

Index Error

+

Defer Error

{"MESSAGE:" + error.message}

{error.stack ?

{"STACK:" + error.stack}

: null} @@ -128,10 +130,12 @@ const routeFiles = { test.describe("Error Sanitization", () => { let fixture: Fixture; let oldConsoleError: () => void; + let errorLogs: any[] = []; test.beforeEach(() => { oldConsoleError = console.error; - console.error = () => {}; + errorLogs = []; + console.error = (...args) => errorLogs.push(args); }); test.afterEach(() => { @@ -142,6 +146,11 @@ test.describe("Error Sanitization", () => { test.beforeAll(async () => { fixture = await createFixture( { + config: { + future: { + v2_errorBoundary: true, + }, + }, files: routeFiles, }, ServerMode.Production @@ -167,6 +176,9 @@ test.describe("Error Sanitization", () => { '{"routes/index":{"message":"Unexpected Server Error","__type":"Error"}}' ); expect(html).not.toMatch(/stack/i); + expect(errorLogs.length).toBe(1); + expect(errorLogs[0][0].message).toMatch("Loader Error"); + expect(errorLogs[0][0].stack).toMatch(" at "); }); test("sanitizes render errors in document requests", async () => { @@ -178,6 +190,9 @@ test.describe("Error Sanitization", () => { '{"routes/index":{"message":"Unexpected Server Error","__type":"Error"}}' ); expect(html).not.toMatch(/stack/i); + expect(errorLogs.length).toBe(1); + expect(errorLogs[0][0].message).toMatch("Render Error"); + expect(errorLogs[0][0].stack).toMatch(" at "); }); test("renders deferred document without errors", async () => { @@ -200,6 +215,9 @@ test.describe("Error Sanitization", () => { // Defer errors are not not part of the JSON blob but rather rejected // against a pending promise and therefore are inlined JS. expect(html).toMatch("x.stack=undefined;"); + // defer errors are not logged to the server console since the request + // has "succeeded" + expect(errorLogs.length).toBe(0); }); test("returns data without errors", async () => { @@ -214,6 +232,9 @@ test.describe("Error Sanitization", () => { let response = await fixture.requestData("/?loader", "routes/index"); let text = await response.text(); expect(text).toBe('{"message":"Unexpected Server Error"}'); + expect(errorLogs.length).toBe(1); + expect(errorLogs[0][0].message).toMatch("Loader Error"); + expect(errorLogs[0][0].stack).toMatch(" at "); }); test("returns deferred data without errors", async () => { @@ -231,6 +252,9 @@ test.describe("Error Sanitization", () => { '{"lazy":"__deferred_promise:lazy"}\n\n' + 'error:{"lazy":{"message":"Unexpected Server Error"}}\n\n' ); + // defer errors are not logged to the server console since the request + // has "succeeded" + expect(errorLogs.length).toBe(0); }); test("sanitizes loader errors in resource requests", async () => { @@ -240,12 +264,20 @@ test.describe("Error Sanitization", () => { ); let text = await response.text(); expect(text).toBe('{"message":"Unexpected Server Error"}'); + expect(errorLogs.length).toBe(1); + expect(errorLogs[0][0].message).toMatch("Loader Error"); + expect(errorLogs[0][0].stack).toMatch(" at "); }); test("sanitizes mismatched route errors in data requests", async () => { let response = await fixture.requestData("/", "not-a-route"); let text = await response.text(); expect(text).toBe('{"message":"Unexpected Server Error"}'); + expect(errorLogs.length).toBe(1); + expect(errorLogs[0][0].message).toMatch( + 'Route "not-a-route" does not match URL "/"' + ); + expect(errorLogs[0][0].stack).toMatch(" at "); }); }); @@ -253,6 +285,11 @@ test.describe("Error Sanitization", () => { test.beforeAll(async () => { fixture = await createFixture( { + config: { + future: { + v2_errorBoundary: true, + }, + }, files: routeFiles, }, ServerMode.Development @@ -286,6 +323,9 @@ test.describe("Error Sanitization", () => { expect(html).toMatch( 'errors":{"routes/index":{"message":"Loader Error","stack":"Error: Loader Error\\n' ); + expect(errorLogs.length).toBe(1); + expect(errorLogs[0][0].message).toMatch("Loader Error"); + expect(errorLogs[0][0].stack).toMatch(" at "); }); test("does not sanitize render errors in document requests", async () => { @@ -297,6 +337,9 @@ test.describe("Error Sanitization", () => { expect(html).toMatch( 'errors":{"routes/index":{"message":"Render Error","stack":"Error: Render Error\\n' ); + expect(errorLogs.length).toBe(1); + expect(errorLogs[0][0].message).toMatch("Render Error"); + expect(errorLogs[0][0].stack).toMatch(" at "); }); test("renders deferred document without errors", async () => { @@ -316,6 +359,9 @@ test.describe("Error Sanitization", () => { // Defer errors are not not part of the JSON blob but rather rejected // against a pending promise and therefore are inlined JS. expect(html).toMatch("x.stack=e.stack;"); + // defer errors are not logged to the server console since the request + // has "succeeded" + expect(errorLogs.length).toBe(0); }); test("returns data without errors", async () => { @@ -332,6 +378,9 @@ test.describe("Error Sanitization", () => { expect(text).toMatch( '{"message":"Loader Error","stack":"Error: Loader Error' ); + expect(errorLogs.length).toBe(1); + expect(errorLogs[0][0].message).toMatch("Loader Error"); + expect(errorLogs[0][0].stack).toMatch(" at "); }); test("returns deferred data without errors", async () => { @@ -348,6 +397,9 @@ test.describe("Error Sanitization", () => { expect(text).toMatch( 'error:{"lazy":{"message":"REJECTED","stack":"Error: REJECTED' ); + // defer errors are not logged to the server console since the request + // has "succeeded" + expect(errorLogs.length).toBe(0); }); test("does not sanitize loader errors in resource requests", async () => { @@ -359,6 +411,9 @@ test.describe("Error Sanitization", () => { expect(text).toMatch( '{"message":"Loader Error","stack":"Error: Loader Error' ); + expect(errorLogs.length).toBe(1); + expect(errorLogs[0][0].message).toMatch("Loader Error"); + expect(errorLogs[0][0].stack).toMatch(" at "); }); test("sanitizes mismatched route errors in data requests", async () => { @@ -367,6 +422,205 @@ test.describe("Error Sanitization", () => { expect(text).toMatch( '{"message":"Route \\"not-a-route\\" does not match URL \\"/\\"","stack":"Error: Route \\"not-a-route\\" does not match URL \\"/\\"' ); + expect(errorLogs.length).toBe(1); + expect(errorLogs[0][0].message).toMatch( + 'Route "not-a-route" does not match URL "/"' + ); + expect(errorLogs[0][0].stack).toMatch(" at "); + }); + }); + + 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"; + import { RemixServer, isRouteErrorResponse } from "@remix-run/react"; + import { renderToString } from "react-dom/server"; + + export default function handleRequest( + request: Request, + responseStatusCode: number, + responseHeaders: Headers, + remixContext: EntryContext + ) { + let markup = renderToString( + + ); + + responseHeaders.set("Content-Type", "text/html"); + + return new Response("" + markup, { + status: responseStatusCode, + headers: responseHeaders, + }); + } + + export function onUnhandledError( + error: unknown, + { request }: { request: Request }, + ) { + console.error("App Specific Error Logging:"); + console.error(" Request: " + request.method + " " + request.url); + if (error instanceof Error) { + console.error(" Error: " + error.message); + console.error(" Stack: " + error.stack); + } else { + console.error("Dunno what this is"); + } + } + `, + ...routeFiles, + }, + }, + ServerMode.Production + ); + }); + + test("renders document without errors", async () => { + let response = await fixture.requestDocument("/"); + let html = await response.text(); + expect(html).toMatch("Index Route"); + expect(html).toMatch("LOADER"); + expect(html).not.toMatch("MESSAGE:"); + expect(html).not.toMatch(/stack/i); + }); + + test("sanitizes loader errors in document requests", async () => { + let response = await fixture.requestDocument("/?loader"); + let html = await response.text(); + expect(html).toMatch("Index Error"); + expect(html).not.toMatch("LOADER"); + expect(html).toMatch("MESSAGE:Unexpected Server Error"); + expect(html).toMatch( + '{"routes/index":{"message":"Unexpected Server Error","__type":"Error"}}' + ); + expect(html).not.toMatch(/stack/i); + expect(errorLogs[0][0]).toEqual("App Specific Error Logging:"); + expect(errorLogs[1][0]).toEqual(" Request: GET test://test/?loader"); + expect(errorLogs[2][0]).toEqual(" Error: Loader Error"); + expect(errorLogs[3][0]).toMatch(" at "); + expect(errorLogs.length).toBe(4); + }); + + test("sanitizes render errors in document requests", async () => { + let response = await fixture.requestDocument("/?render"); + let html = await response.text(); + expect(html).toMatch("Index Error"); + expect(html).toMatch("MESSAGE:Unexpected Server Error"); + expect(html).toMatch( + '{"routes/index":{"message":"Unexpected Server Error","__type":"Error"}}' + ); + expect(html).not.toMatch(/stack/i); + expect(errorLogs[0][0]).toEqual("App Specific Error Logging:"); + expect(errorLogs[1][0]).toEqual(" Request: GET test://test/?render"); + expect(errorLogs[2][0]).toEqual(" Error: Render Error"); + expect(errorLogs[3][0]).toMatch(" at "); + expect(errorLogs.length).toBe(4); + }); + + test("renders deferred document without errors", async () => { + let response = await fixture.requestDocument("/defer"); + let html = await response.text(); + expect(html).toMatch("Defer Route"); + expect(html).toMatch("RESOLVED"); + expect(html).not.toMatch("MESSAGE:"); + // Defer errors are not not part of the JSON blob but rather rejected + // against a pending promise and therefore are inlined JS. + expect(html).not.toMatch("x.stack=e.stack;"); + }); + + test("sanitizes defer errors in document requests", async () => { + let response = await fixture.requestDocument("/defer?loader"); + let html = await response.text(); + expect(html).toMatch("Defer Error"); + expect(html).not.toMatch("RESOLVED"); + expect(html).toMatch('{"message":"Unexpected Server Error"}'); + // Defer errors are not not part of the JSON blob but rather rejected + // against a pending promise and therefore are inlined JS. + expect(html).toMatch("x.stack=undefined;"); + // defer errors are not logged to the server console since the request + // has "succeeded" + expect(errorLogs.length).toBe(0); + }); + + test("returns data without errors", async () => { + let response = await fixture.requestData("/", "routes/index"); + let text = await response.text(); + expect(text).toMatch("LOADER"); + expect(text).not.toMatch("MESSAGE:"); + expect(text).not.toMatch(/stack/i); + }); + + test("sanitizes loader errors in data requests", async () => { + let response = await fixture.requestData("/?loader", "routes/index"); + let text = await response.text(); + expect(text).toBe('{"message":"Unexpected Server Error"}'); + expect(errorLogs[0][0]).toEqual("App Specific Error Logging:"); + expect(errorLogs[1][0]).toEqual( + " Request: GET test://test/?loader=&_data=routes%2Findex" + ); + expect(errorLogs[2][0]).toEqual(" Error: Loader Error"); + expect(errorLogs[3][0]).toMatch(" at "); + expect(errorLogs.length).toBe(4); + }); + + test("returns deferred data without errors", async () => { + let response = await fixture.requestData("/defer", "routes/defer"); + let text = await response.text(); + expect(text).toMatch("RESOLVED"); + expect(text).not.toMatch("REJECTED"); + expect(text).not.toMatch(/stack/i); + }); + + test("sanitizes loader errors in deferred data requests", async () => { + let response = await fixture.requestData("/defer?loader", "routes/defer"); + let text = await response.text(); + expect(text).toBe( + '{"lazy":"__deferred_promise:lazy"}\n\n' + + 'error:{"lazy":{"message":"Unexpected Server Error"}}\n\n' + ); + // defer errors are not logged to the server console since the request + // has "succeeded" + expect(errorLogs.length).toBe(0); + }); + + test("sanitizes loader errors in resource requests", async () => { + let response = await fixture.requestData( + "/resource?loader", + "routes/resource" + ); + let text = await response.text(); + expect(text).toBe('{"message":"Unexpected Server Error"}'); + expect(errorLogs[0][0]).toEqual("App Specific Error Logging:"); + expect(errorLogs[1][0]).toEqual( + " Request: GET test://test/resource?loader=&_data=routes%2Fresource" + ); + expect(errorLogs[2][0]).toEqual(" Error: Loader Error"); + expect(errorLogs[3][0]).toMatch(" at "); + expect(errorLogs.length).toBe(4); + }); + + test("sanitizes mismatched route errors in data requests", async () => { + let response = await fixture.requestData("/", "not-a-route"); + let text = await response.text(); + expect(text).toBe('{"message":"Unexpected Server Error"}'); + expect(errorLogs[0][0]).toEqual("App Specific Error Logging:"); + expect(errorLogs[1][0]).toEqual( + " Request: GET test://test/?_data=not-a-route" + ); + expect(errorLogs[2][0]).toEqual( + ' Error: Route "not-a-route" does not match URL "/"' + ); + expect(errorLogs[3][0]).toMatch(" at "); + expect(errorLogs.length).toBe(4); }); }); }); diff --git a/integration/headers-test.ts b/integration/headers-test.ts index 454c83ac319..a34dfe72854 100644 --- a/integration/headers-test.ts +++ b/integration/headers-test.ts @@ -1,4 +1,5 @@ import { test, expect } from "@playwright/test"; +import { ServerMode } from "@remix-run/server-runtime/mode"; import { createFixture, js } from "./helpers/create-fixture"; import type { Fixture } from "./helpers/create-fixture"; @@ -12,195 +13,198 @@ test.describe("headers export", () => { let appFixture: Fixture; test.beforeAll(async () => { - appFixture = await createFixture({ - config: { - future: { - v2_routeConvention: true, - v2_errorBoundary: true, - v2_headers: true, + appFixture = await createFixture( + { + config: { + future: { + v2_routeConvention: true, + v2_errorBoundary: true, + v2_headers: true, + }, }, - }, - files: { - "app/root.jsx": js` - import { json } from "@remix-run/node"; - import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; - - export const loader = () => json({}); - - export default function Root() { - return ( - - - - - - - - - - - ); - } - `, - - "app/routes/_index.jsx": js` - import { json } from "@remix-run/node"; - - export function loader() { - return json(null, { - headers: { - "${ROOT_HEADER_KEY}": "${ROOT_HEADER_VALUE}" + files: { + "app/root.jsx": js` + import { json } from "@remix-run/node"; + import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; + + export const loader = () => json({}); + + export default function Root() { + return ( + + + + + + + + + + + ); + } + `, + + "app/routes/_index.jsx": js` + import { json } from "@remix-run/node"; + + export function loader() { + return json(null, { + headers: { + "${ROOT_HEADER_KEY}": "${ROOT_HEADER_VALUE}" + } + }) + } + + export function headers({ loaderHeaders }) { + return { + "${ROOT_HEADER_KEY}": loaderHeaders.get("${ROOT_HEADER_KEY}") } - }) - } + } - export function headers({ loaderHeaders }) { - return { - "${ROOT_HEADER_KEY}": loaderHeaders.get("${ROOT_HEADER_KEY}") + export default function Index() { + return
Heyo!
} - } + `, - export default function Index() { - return
Heyo!
- } - `, + "app/routes/action.jsx": js` + import { json } from "@remix-run/node"; - "app/routes/action.jsx": js` - import { json } from "@remix-run/node"; + export function action() { + return json(null, { + headers: { + "${ACTION_HKEY}": "${ACTION_HVALUE}" + } + }) + } - export function action() { - return json(null, { - headers: { - "${ACTION_HKEY}": "${ACTION_HVALUE}" + export function headers({ actionHeaders }) { + return { + "${ACTION_HKEY}": actionHeaders.get("${ACTION_HKEY}") } - }) - } + } - export function headers({ actionHeaders }) { - return { - "${ACTION_HKEY}": actionHeaders.get("${ACTION_HKEY}") + export default function Action() { return
} + `, + + "app/routes/parent.jsx": js` + export function headers({ actionHeaders, errorHeaders, loaderHeaders, parentHeaders }) { + return new Headers([ + ...(parentHeaders ? Array.from(parentHeaders.entries()) : []), + ...(actionHeaders ? Array.from(actionHeaders.entries()) : []), + ...(loaderHeaders ? Array.from(loaderHeaders.entries()) : []), + ...(errorHeaders ? Array.from(errorHeaders.entries()) : []), + ]); } - } - - export default function Action() { return
} - `, - - "app/routes/parent.jsx": js` - export function headers({ actionHeaders, errorHeaders, loaderHeaders, parentHeaders }) { - return new Headers([ - ...(parentHeaders ? Array.from(parentHeaders.entries()) : []), - ...(actionHeaders ? Array.from(actionHeaders.entries()) : []), - ...(loaderHeaders ? Array.from(loaderHeaders.entries()) : []), - ...(errorHeaders ? Array.from(errorHeaders.entries()) : []), - ]); - } - - export function loader({ request }) { - if (new URL(request.url).searchParams.get('throw') === "parent") { - throw new Response(null, { - status: 400, - headers: { 'X-Parent-Loader': 'error' }, + + export function loader({ request }) { + if (new URL(request.url).searchParams.get('throw') === "parent") { + throw new Response(null, { + status: 400, + headers: { 'X-Parent-Loader': 'error' }, + }) + } + return new Response(null, { + headers: { 'X-Parent-Loader': 'success' }, }) } - return new Response(null, { - headers: { 'X-Parent-Loader': 'success' }, - }) - } - - export async function action({ request }) { - let fd = await request.formData(); - if (fd.get('throw') === "parent") { - throw new Response(null, { - status: 400, - headers: { 'X-Parent-Action': 'error' }, + + export async function action({ request }) { + let fd = await request.formData(); + if (fd.get('throw') === "parent") { + throw new Response(null, { + status: 400, + headers: { 'X-Parent-Action': 'error' }, + }) + } + return new Response(null, { + headers: { 'X-Parent-Action': 'success' }, }) } - return new Response(null, { - headers: { 'X-Parent-Action': 'success' }, - }) - } - export default function Component() { return
} + export default function Component() { return
} - export function ErrorBoundary() { - return

Error!

- } - `, + export function ErrorBoundary() { + return

Error!

+ } + `, + + "app/routes/parent.child.jsx": js` + export function loader({ request }) { + if (new URL(request.url).searchParams.get('throw') === "child") { + throw new Response(null, { + status: 400, + headers: { 'X-Child-Loader': 'error' }, + }) + } + return null + } - "app/routes/parent.child.jsx": js` - export function loader({ request }) { - if (new URL(request.url).searchParams.get('throw') === "child") { - throw new Response(null, { - status: 400, - headers: { 'X-Child-Loader': 'error' }, - }) + export async function action({ request }) { + let fd = await request.formData(); + if (fd.get('throw') === "child") { + throw new Response(null, { + status: 400, + headers: { 'X-Child-Action': 'error' }, + }) + } + return null } - return null - } - export async function action({ request }) { - let fd = await request.formData(); - if (fd.get('throw') === "child") { + export default function Component() { return
} + `, + + "app/routes/parent.child.grandchild.jsx": js` + export function loader({ request }) { throw new Response(null, { status: 400, - headers: { 'X-Child-Action': 'error' }, + headers: { 'X-Child-Grandchild': 'error' }, }) } - return null - } - - export default function Component() { return
} - `, - - "app/routes/parent.child.grandchild.jsx": js` - export function loader({ request }) { - throw new Response(null, { - status: 400, - headers: { 'X-Child-Grandchild': 'error' }, - }) - } - - export default function Component() { return
} - `, - - "app/routes/cookie.jsx": js` - import { json } from "@remix-run/server-runtime"; - import { Outlet } from "@remix-run/react"; - - export function loader({ request }) { - if (new URL(request.url).searchParams.has("parent-throw")) { - throw json(null, { headers: { "Set-Cookie": "parent-thrown-cookie=true" } }); + + export default function Component() { return
} + `, + + "app/routes/cookie.jsx": js` + import { json } from "@remix-run/server-runtime"; + import { Outlet } from "@remix-run/react"; + + export function loader({ request }) { + if (new URL(request.url).searchParams.has("parent-throw")) { + throw json(null, { headers: { "Set-Cookie": "parent-thrown-cookie=true" } }); + } + return null + }; + + export default function Parent() { + return ; } - return null - }; - export default function Parent() { - return ; - } + export function ErrorBoundary() { + return

Caught!

; + } + `, - export function ErrorBoundary() { - return

Caught!

; - } - `, + "app/routes/cookie.child.jsx": js` + import { json } from "@remix-run/node"; - "app/routes/cookie.child.jsx": js` - import { json } from "@remix-run/node"; + export function loader({ request }) { + if (new URL(request.url).searchParams.has("throw")) { + throw json(null, { headers: { "Set-Cookie": "thrown-cookie=true" } }); + } + return json(null, { + headers: { "Set-Cookie": "normal-cookie=true" }, + }); + }; - export function loader({ request }) { - if (new URL(request.url).searchParams.has("throw")) { - throw json(null, { headers: { "Set-Cookie": "thrown-cookie=true" } }); + export default function Child() { + return

Child

; } - return json(null, { - headers: { "Set-Cookie": "normal-cookie=true" }, - }); - }; - - export default function Child() { - return

Child

; - } - `, + `, + }, }, - }); + ServerMode.Test + ); }); test("can use `action` headers", async () => { @@ -220,53 +224,56 @@ test.describe("headers export", () => { let HEADER_KEY = "X-Test"; let HEADER_VALUE = "SUCCESS"; - let fixture = await createFixture({ - config: { - future: { v2_routeConvention: true }, - }, - files: { - "app/root.jsx": js` - import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; - - export default function Root() { - return ( - - - - - - - - - - - ); - } - `, - - "app/routes/_index.jsx": js` - import { json } from "@remix-run/node"; - - export function loader() { - return json(null, { - headers: { - "${HEADER_KEY}": "${HEADER_VALUE}" - } - }) - } + let fixture = await createFixture( + { + config: { + future: { v2_routeConvention: true }, + }, + files: { + "app/root.jsx": js` + import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; + + export default function Root() { + return ( + + + + + + + + + + + ); + } + `, - export function headers({ loaderHeaders }) { - return { - "${HEADER_KEY}": loaderHeaders.get("${HEADER_KEY}") + "app/routes/_index.jsx": js` + import { json } from "@remix-run/node"; + + export function loader() { + return json(null, { + headers: { + "${HEADER_KEY}": "${HEADER_VALUE}" + } + }) } - } - export default function Index() { - return
Heyo!
- } - `, + export function headers({ loaderHeaders }) { + return { + "${HEADER_KEY}": loaderHeaders.get("${HEADER_KEY}") + } + } + + export default function Index() { + return
Heyo!
+ } + `, + }, }, - }); + ServerMode.Test + ); let response = await fixture.requestDocument("/"); expect(response.headers.get(HEADER_KEY)).toBe(HEADER_VALUE); }); @@ -427,102 +434,105 @@ test.describe("v1 behavior (future.v2_headers=false)", () => { let appFixture: Fixture; test.beforeAll(async () => { - appFixture = await createFixture({ - config: { - future: { - v2_routeConvention: true, - v2_errorBoundary: true, - v2_headers: false, + appFixture = await createFixture( + { + config: { + future: { + v2_routeConvention: true, + v2_errorBoundary: true, + v2_headers: false, + }, }, - }, - files: { - "app/root.jsx": js` - import { json } from "@remix-run/node"; - import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; - - export const loader = () => json({}); - - export default function Root() { - return ( - - - - - - - - - - - ); - } - `, - - "app/routes/parent.jsx": js` - export function headers({ actionHeaders, errorHeaders, loaderHeaders, parentHeaders }) { - return new Headers([ - ...(parentHeaders ? Array.from(parentHeaders.entries()) : []), - ...(actionHeaders ? Array.from(actionHeaders.entries()) : []), - ...(loaderHeaders ? Array.from(loaderHeaders.entries()) : []), - ...(errorHeaders ? Array.from(errorHeaders.entries()) : []), - ]); - } - - export function loader({ request }) { - return new Response(null, { - headers: { 'X-Parent-Loader': 'success' }, - }) - } - - export default function Component() { return
} - `, - - "app/routes/parent.child.jsx": js` - export async function action({ request }) { - return null; - } - - export default function Component() { return
} - `, - - "app/routes/cookie.jsx": js` - import { json } from "@remix-run/server-runtime"; - import { Outlet } from "@remix-run/react"; - - export function loader({ request }) { - if (new URL(request.url).searchParams.has("parent-throw")) { - throw json(null, { headers: { "Set-Cookie": "parent-thrown-cookie=true" } }); + files: { + "app/root.jsx": js` + import { json } from "@remix-run/node"; + import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; + + export const loader = () => json({}); + + export default function Root() { + return ( + + + + + + + + + + + ); + } + `, + + "app/routes/parent.jsx": js` + export function headers({ actionHeaders, errorHeaders, loaderHeaders, parentHeaders }) { + return new Headers([ + ...(parentHeaders ? Array.from(parentHeaders.entries()) : []), + ...(actionHeaders ? Array.from(actionHeaders.entries()) : []), + ...(loaderHeaders ? Array.from(loaderHeaders.entries()) : []), + ...(errorHeaders ? Array.from(errorHeaders.entries()) : []), + ]); } - return null - }; - export default function Parent() { - return ; - } + export function loader({ request }) { + return new Response(null, { + headers: { 'X-Parent-Loader': 'success' }, + }) + } + + export default function Component() { return
} + `, + + "app/routes/parent.child.jsx": js` + export async function action({ request }) { + return null; + } + + export default function Component() { return
} + `, + + "app/routes/cookie.jsx": js` + import { json } from "@remix-run/server-runtime"; + import { Outlet } from "@remix-run/react"; + + export function loader({ request }) { + if (new URL(request.url).searchParams.has("parent-throw")) { + throw json(null, { headers: { "Set-Cookie": "parent-thrown-cookie=true" } }); + } + return null + }; + + export default function Parent() { + return ; + } + + export function ErrorBoundary() { + return

Caught!

; + } + `, - export function ErrorBoundary() { - return

Caught!

; - } - `, + "app/routes/cookie.child.jsx": js` + import { json } from "@remix-run/node"; - "app/routes/cookie.child.jsx": js` - import { json } from "@remix-run/node"; + export function loader({ request }) { + if (new URL(request.url).searchParams.has("throw")) { + throw json(null, { headers: { "Set-Cookie": "thrown-cookie=true" } }); + } + return json(null, { + headers: { "Set-Cookie": "normal-cookie=true" }, + }); + }; - export function loader({ request }) { - if (new URL(request.url).searchParams.has("throw")) { - throw json(null, { headers: { "Set-Cookie": "thrown-cookie=true" } }); + export default function Child() { + return

Child

; } - return json(null, { - headers: { "Set-Cookie": "normal-cookie=true" }, - }); - }; - - export default function Child() { - return

Child

; - } - `, + `, + }, }, - }); + ServerMode.Test + ); }); test("returns no headers when the leaf route doesn't export a header function (GET)", async () => { diff --git a/packages/remix-cloudflare/index.ts b/packages/remix-cloudflare/index.ts index e5850c1acb6..9d27898e2e4 100644 --- a/packages/remix-cloudflare/index.ts +++ b/packages/remix-cloudflare/index.ts @@ -68,6 +68,7 @@ export type { MemoryUploadHandlerOptions, MetaDescriptor, MetaFunction, + OnUnhandledErrorFunction, PageLinkDescriptor, RequestHandler, RouteComponent, diff --git a/packages/remix-deno/index.ts b/packages/remix-deno/index.ts index 2eee795dfd6..a4ec61f2d34 100644 --- a/packages/remix-deno/index.ts +++ b/packages/remix-deno/index.ts @@ -56,6 +56,7 @@ export type { MemoryUploadHandlerOptions, MetaDescriptor, MetaFunction, + OnUnhandledErrorFunction, PageLinkDescriptor, RequestHandler, RouteComponent, diff --git a/packages/remix-dev/codemod/replace-remix-magic-imports/utils/export.ts b/packages/remix-dev/codemod/replace-remix-magic-imports/utils/export.ts index 5be1b002c6f..26b0e315398 100644 --- a/packages/remix-dev/codemod/replace-remix-magic-imports/utils/export.ts +++ b/packages/remix-dev/codemod/replace-remix-magic-imports/utils/export.ts @@ -68,6 +68,7 @@ const defaultRuntimeExports: ExportNames = { "MemoryUploadHandlerOptions", "MetaDescriptor", "MetaFunction", + "OnUnhandledErrorFunction", "PageLinkDescriptor", "RequestHandler", "RouteComponent", diff --git a/packages/remix-dev/config/defaults/cloudflare/entry.server.react-stream.tsx b/packages/remix-dev/config/defaults/cloudflare/entry.server.react-stream.tsx index 3e08f46fe53..a393a5228ea 100644 --- a/packages/remix-dev/config/defaults/cloudflare/entry.server.react-stream.tsx +++ b/packages/remix-dev/config/defaults/cloudflare/entry.server.react-stream.tsx @@ -14,6 +14,7 @@ export default async function handleRequest( { signal: request.signal, onError(error: unknown) { + // Log streaming rendering errors from inside the shell console.error(error); responseStatusCode = 500; }, diff --git a/packages/remix-dev/config/defaults/deno/entry.server.react-stream.tsx b/packages/remix-dev/config/defaults/deno/entry.server.react-stream.tsx index 7be6c748061..91a1d11e52b 100644 --- a/packages/remix-dev/config/defaults/deno/entry.server.react-stream.tsx +++ b/packages/remix-dev/config/defaults/deno/entry.server.react-stream.tsx @@ -14,6 +14,7 @@ export default async function handleRequest( { signal: request.signal, onError(error: unknown) { + // Log streaming rendering errors from inside the shell console.error(error); responseStatusCode = 500; }, diff --git a/packages/remix-dev/config/defaults/node/entry.server.react-stream.tsx b/packages/remix-dev/config/defaults/node/entry.server.react-stream.tsx index e9f6ac9846a..efe28dbcfb9 100644 --- a/packages/remix-dev/config/defaults/node/entry.server.react-stream.tsx +++ b/packages/remix-dev/config/defaults/node/entry.server.react-stream.tsx @@ -36,6 +36,7 @@ function handleBotRequest( remixContext: EntryContext ) { return new Promise((resolve, reject) => { + let shellRendered = false; const { pipe, abort } = renderToPipeableStream( , { onAllReady() { + shellRendered = true; const body = new PassThrough(); responseHeaders.set("Content-Type", "text/html"); @@ -62,7 +64,12 @@ function handleBotRequest( }, onError(error: unknown) { responseStatusCode = 500; - console.error(error); + // Log streaming rendering errors from inside the shell. Don't log + // errors encountered during initial shell rendering since they'll + // reject and get logged in handleDocumentRequest. + if (shellRendered) { + console.error(error); + } }, } ); @@ -78,6 +85,7 @@ function handleBrowserRequest( remixContext: EntryContext ) { return new Promise((resolve, reject) => { + let shellRendered = false; const { pipe, abort } = renderToPipeableStream( , { onShellReady() { + shellRendered = true; const body = new PassThrough(); responseHeaders.set("Content-Type", "text/html"); @@ -103,8 +112,13 @@ function handleBrowserRequest( reject(error); }, onError(error: unknown) { - console.error(error); responseStatusCode = 500; + // Log streaming rendering errors from inside the shell. Don't log + // errors encountered during initial shell rendering since they'll + // reject and get logged in handleDocumentRequest. + if (shellRendered) { + console.error(error); + } }, } ); diff --git a/packages/remix-eslint-config/rules/packageExports.js b/packages/remix-eslint-config/rules/packageExports.js index b3bfa1a3f94..49892da0e93 100644 --- a/packages/remix-eslint-config/rules/packageExports.js +++ b/packages/remix-eslint-config/rules/packageExports.js @@ -46,6 +46,7 @@ const defaultRuntimeExports = { "MemoryUploadHandlerOptions", "MetaDescriptor", "MetaFunction", + "OnUnhandledErrorFunction", "PageLinkDescriptor", "RequestHandler", "RouteComponent", diff --git a/packages/remix-node/index.ts b/packages/remix-node/index.ts index 8e0d62dbeeb..20fa129bb1b 100644 --- a/packages/remix-node/index.ts +++ b/packages/remix-node/index.ts @@ -79,6 +79,7 @@ export type { MemoryUploadHandlerOptions, MetaDescriptor, MetaFunction, + OnUnhandledErrorFunction, PageLinkDescriptor, RequestHandler, RouteComponent, diff --git a/packages/remix-react/errorBoundaries.tsx b/packages/remix-react/errorBoundaries.tsx index 32eb75b4e1d..24d63344aed 100644 --- a/packages/remix-react/errorBoundaries.tsx +++ b/packages/remix-react/errorBoundaries.tsx @@ -70,7 +70,10 @@ export class RemixErrorBoundary extends React.Component< * When app's don't provide a root level ErrorBoundary, we default to this. */ export function RemixRootDefaultErrorBoundary({ error }: { error: Error }) { - console.error(error); + // Only log client side to avoid double-logging on the server + React.useEffect(() => { + console.error(error); + }, [error]); return ( @@ -84,16 +87,18 @@ export function RemixRootDefaultErrorBoundary({ error }: { error: Error }) {

Application Error

-
-            {error.stack}
-          
+ {error.stack ? ( +
+              {error.stack}
+            
+ ) : null}