diff --git a/.changeset/tidy-jokes-sparkle.md b/.changeset/tidy-jokes-sparkle.md new file mode 100644 index 00000000000..9862266a585 --- /dev/null +++ b/.changeset/tidy-jokes-sparkle.md @@ -0,0 +1,5 @@ +--- +"@remix-run/server-runtime": patch +--- + +Ensure stack traces are removed from all server side errors in production diff --git a/integration/error-boundary-test.ts b/integration/error-boundary-test.ts index 5f19ed4309a..16a2406a406 100644 --- a/integration/error-boundary-test.ts +++ b/integration/error-boundary-test.ts @@ -40,256 +40,259 @@ test.describe("ErrorBoundary", () => { test.beforeAll(async () => { _consoleError = console.error; console.error = () => {}; - fixture = await createFixture({ - future: { v2_routeConvention: true }, - files: { - "app/root.jsx": js` - import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; + fixture = await createFixture( + { + future: { v2_routeConvention: true }, + files: { + "app/root.jsx": js` + import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; - export default function Root() { - return ( - - - - - - -
- -
- - - - ); - } + export default function Root() { + return ( + + + + + + +
+ +
+ + + + ); + } - export function ErrorBoundary() { - return ( - - - -
-
${ROOT_BOUNDARY_TEXT}
-
- - - - ) - } - `, + export function ErrorBoundary() { + return ( + + + +
+
${ROOT_BOUNDARY_TEXT}
+
+ + + + ) + } + `, - "app/routes/_index.jsx": js` - import { Link, Form } from "@remix-run/react"; - export default function () { - return ( -
- ${NOT_FOUND_HREF} + "app/routes/_index.jsx": js` + import { Link, Form } from "@remix-run/react"; + export default function () { + return ( +
+ ${NOT_FOUND_HREF} + +
+ + + + +
+ + + ${HAS_BOUNDARY_LOADER} + + + ${NO_BOUNDARY_LOADER} + + + ${HAS_BOUNDARY_RENDER} + + + ${NO_BOUNDARY_RENDER} + +
+ ) + } + `, + [`app/routes${HAS_BOUNDARY_ACTION_FILE}.jsx`]: js` + import { Form } from "@remix-run/react"; + export async function action() { + throw new Error("Kaboom!") + } + export function ErrorBoundary() { + return

${OWN_BOUNDARY_TEXT}

+ } + export default function () { + return (
- - - -
+ ); + } + `, - - ${HAS_BOUNDARY_LOADER} - - - ${NO_BOUNDARY_LOADER} - - - ${HAS_BOUNDARY_RENDER} - - - ${NO_BOUNDARY_RENDER} - -
- ) - } - `, - - [`app/routes${HAS_BOUNDARY_ACTION_FILE}.jsx`]: js` - import { Form } from "@remix-run/react"; - export async function action() { - throw new Error("Kaboom!") - } - export function ErrorBoundary() { - return

${OWN_BOUNDARY_TEXT}

- } - export default function () { - return ( -
- -
- ); - } - `, - - [`app/routes${NO_BOUNDARY_ACTION_FILE}.jsx`]: js` - import { Form } from "@remix-run/react"; - export function action() { - throw new Error("Kaboom!") - } - export default function () { - return ( -
- -
- ) - } - `, + [`app/routes${NO_BOUNDARY_ACTION_FILE}.jsx`]: js` + import { Form } from "@remix-run/react"; + export function action() { + throw new Error("Kaboom!") + } + export default function () { + return ( +
+ +
+ ) + } + `, - [`app/routes${HAS_BOUNDARY_LOADER_FILE}.jsx`]: js` - export function loader() { - throw new Error("Kaboom!") - } - export function ErrorBoundary() { - return
${OWN_BOUNDARY_TEXT}
- } - export default function () { - return
- } - `, + [`app/routes${HAS_BOUNDARY_LOADER_FILE}.jsx`]: js` + export function loader() { + throw new Error("Kaboom!") + } + export function ErrorBoundary() { + return
${OWN_BOUNDARY_TEXT}
+ } + export default function () { + return
+ } + `, - [`app/routes${NO_BOUNDARY_LOADER_FILE}.jsx`]: js` - export function loader() { - throw new Error("Kaboom!") - } - export default function () { - return
- } - `, + [`app/routes${NO_BOUNDARY_LOADER_FILE}.jsx`]: js` + export function loader() { + throw new Error("Kaboom!") + } + export default function () { + return
+ } + `, - [`app/routes${NO_BOUNDARY_RENDER_FILE}.jsx`]: js` - export default function () { - throw new Error("Kaboom!") - return
- } - `, + [`app/routes${NO_BOUNDARY_RENDER_FILE}.jsx`]: js` + export default function () { + throw new Error("Kaboom!") + return
+ } + `, - [`app/routes${HAS_BOUNDARY_RENDER_FILE}.jsx`]: js` - export default function () { - throw new Error("Kaboom!") - return
- } + [`app/routes${HAS_BOUNDARY_RENDER_FILE}.jsx`]: js` + export default function () { + throw new Error("Kaboom!") + return
+ } - export function ErrorBoundary() { - return
${OWN_BOUNDARY_TEXT}
- } - `, + export function ErrorBoundary() { + return
${OWN_BOUNDARY_TEXT}
+ } + `, - [`app/routes${HAS_BOUNDARY_NO_LOADER_OR_ACTION_FILE}.jsx`]: js` - export function ErrorBoundary() { - return
${OWN_BOUNDARY_TEXT}
- } - export default function Index() { - return
- } - `, + [`app/routes${HAS_BOUNDARY_NO_LOADER_OR_ACTION_FILE}.jsx`]: js` + export function ErrorBoundary() { + return
${OWN_BOUNDARY_TEXT}
+ } + export default function Index() { + return
+ } + `, - [`app/routes${NO_BOUNDARY_NO_LOADER_OR_ACTION_FILE}.jsx`]: js` - export default function Index() { - return
- } - `, + [`app/routes${NO_BOUNDARY_NO_LOADER_OR_ACTION_FILE}.jsx`]: js` + export default function Index() { + return
+ } + `, - "app/routes/fetcher-boundary.jsx": js` - import { useFetcher } from "@remix-run/react"; - export function ErrorBoundary() { - return

${OWN_BOUNDARY_TEXT}

- } - export default function() { - let fetcher = useFetcher(); + "app/routes/fetcher-boundary.jsx": js` + import { useFetcher } from "@remix-run/react"; + export function ErrorBoundary() { + return

${OWN_BOUNDARY_TEXT}

+ } + export default function() { + let fetcher = useFetcher(); - return ( -
- -
- ) - } - `, + return ( +
+ +
+ ) + } + `, - "app/routes/fetcher-no-boundary.jsx": js` - import { useFetcher } from "@remix-run/react"; - export default function() { - let fetcher = useFetcher(); + "app/routes/fetcher-no-boundary.jsx": js` + import { useFetcher } from "@remix-run/react"; + export default function() { + let fetcher = useFetcher(); - return ( -
- - - -
- ) - } - `, + return ( +
+ + + +
+ ) + } + `, - "app/routes/action.jsx": js` - import { Outlet, useLoaderData } from "@remix-run/react"; + "app/routes/action.jsx": js` + import { Outlet, useLoaderData } from "@remix-run/react"; - export function loader() { - return "PARENT"; - } + export function loader() { + return "PARENT"; + } - export default function () { - return ( -
-

{useLoaderData()}

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

{useLoaderData()}

+ +
+ ) + } + `, - "app/routes/action.child-error.jsx": js` - import { Form, useLoaderData } from "@remix-run/react"; + "app/routes/action.child-error.jsx": js` + import { Form, useLoaderData } from "@remix-run/react"; - export function loader() { - return "CHILD"; - } + export function loader() { + return "CHILD"; + } - export function action() { - throw new Error("Broken!"); - } + export function action() { + throw new Error("Broken!"); + } - export default function () { - return ( - <> -

{useLoaderData()}

-
- -
- - ) - } + export default function () { + return ( + <> +

{useLoaderData()}

+
+ +
+ + ) + } - export function ErrorBoundary({ error }) { - return

{error.message}

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

{error.message}

; + } + `, + }, }, - }); + ServerMode.Development + ); - appFixture = await createAppFixture(fixture); + appFixture = await createAppFixture(fixture, ServerMode.Development); }); test.afterAll(() => { @@ -428,7 +431,7 @@ test.describe("ErrorBoundary", () => { await page.waitForSelector("#child-error"); // Preserves parent loader data expect(await app.getHtml("#parent-data")).toMatch("PARENT"); - expect(await app.getHtml("#child-error")).toMatch("Broken!"); + expect(await app.getHtml("#child-error")).toMatch("Broken"); }); test("renders own boundary in fetcher action submission without action from other routes", async ({ @@ -1002,12 +1005,15 @@ test.describe("Default ErrorBoundary", () => { test.describe("When the root route does not have a boundary", () => { test.beforeAll(async () => { - fixture = await createFixture({ - future: { - v2_routeConvention: true, + fixture = await createFixture( + { + files: getFiles({ includeRootErrorBoundary: false }), + future: { + v2_routeConvention: true, + }, }, - files: getFiles({ includeRootErrorBoundary: false }), - }); + ServerMode.Development + ); appFixture = await createAppFixture(fixture, ServerMode.Development); }); @@ -1073,12 +1079,15 @@ test.describe("Default ErrorBoundary", () => { test.describe("When the root route has a boundary", () => { test.beforeAll(async () => { - fixture = await createFixture({ - future: { - v2_routeConvention: true, + fixture = await createFixture( + { + future: { + v2_routeConvention: true, + }, + files: getFiles({ includeRootErrorBoundary: true }), }, - files: getFiles({ includeRootErrorBoundary: true }), - }); + ServerMode.Development + ); appFixture = await createAppFixture(fixture, ServerMode.Development); }); @@ -1139,15 +1148,18 @@ test.describe("Default ErrorBoundary", () => { test.describe("When the root route has a boundary but it also throws 😦", () => { test.beforeAll(async () => { - fixture = await createFixture({ - future: { - v2_routeConvention: true, + fixture = await createFixture( + { + future: { + v2_routeConvention: true, + }, + files: getFiles({ + includeRootErrorBoundary: true, + rootErrorBoundaryThrows: true, + }), }, - files: getFiles({ - includeRootErrorBoundary: true, - rootErrorBoundaryThrows: true, - }), - }); + ServerMode.Development + ); appFixture = await createAppFixture(fixture, ServerMode.Development); }); @@ -1260,260 +1272,263 @@ test.describe("v2_errorBoundary", () => { test.beforeAll(async () => { _consoleError = console.error; console.error = () => {}; - fixture = await createFixture({ - future: { - v2_routeConvention: true, - v2_errorBoundary: true, - }, - files: { - "app/root.jsx": js` - import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; + fixture = await createFixture( + { + future: { + v2_routeConvention: true, + v2_errorBoundary: true, + }, + files: { + "app/root.jsx": js` + import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; - export default function Root() { - return ( - - - - - - -
- -
- - - - ); - } + export default function Root() { + return ( + + + + + + +
+ +
+ + + + ); + } - export function ErrorBoundary() { - return ( - - - -
-
${ROOT_BOUNDARY_TEXT}
-
- - - - ) - } - `, + export function ErrorBoundary() { + return ( + + + +
+
${ROOT_BOUNDARY_TEXT}
+
+ + + + ) + } + `, - "app/routes/_index.jsx": js` - import { Link, Form } from "@remix-run/react"; - export default function () { - return ( -
- ${NOT_FOUND_HREF} + "app/routes/_index.jsx": js` + import { Link, Form } from "@remix-run/react"; + export default function () { + return ( +
+ ${NOT_FOUND_HREF} -
- - - - + + + +
+ + + ${HAS_BOUNDARY_LOADER} + + + ${NO_BOUNDARY_LOADER} + + + ${HAS_BOUNDARY_RENDER} + + + ${NO_BOUNDARY_RENDER} + +
+ ) + } + `, + + [`app/routes${HAS_BOUNDARY_ACTION_FILE}.jsx`]: js` + import { Form } from "@remix-run/react"; + export async function action() { + throw new Error("Kaboom!") + } + export function ErrorBoundary() { + return

${OWN_BOUNDARY_TEXT}

+ } + export default function () { + return ( +
+
+ ); + } + `, - - ${HAS_BOUNDARY_LOADER} - - - ${NO_BOUNDARY_LOADER} - - - ${HAS_BOUNDARY_RENDER} - - - ${NO_BOUNDARY_RENDER} - -
- ) - } - `, - - [`app/routes${HAS_BOUNDARY_ACTION_FILE}.jsx`]: js` - import { Form } from "@remix-run/react"; - export async function action() { - throw new Error("Kaboom!") - } - export function ErrorBoundary() { - return

${OWN_BOUNDARY_TEXT}

- } - export default function () { - return ( -
- -
- ); - } - `, - - [`app/routes${NO_BOUNDARY_ACTION_FILE}.jsx`]: js` - import { Form } from "@remix-run/react"; - export function action() { - throw new Error("Kaboom!") - } - export default function () { - return ( -
- -
- ) - } - `, + [`app/routes${NO_BOUNDARY_ACTION_FILE}.jsx`]: js` + import { Form } from "@remix-run/react"; + export function action() { + throw new Error("Kaboom!") + } + export default function () { + return ( +
+ +
+ ) + } + `, - [`app/routes${HAS_BOUNDARY_LOADER_FILE}.jsx`]: js` - export function loader() { - throw new Error("Kaboom!") - } - export function ErrorBoundary() { - return
${OWN_BOUNDARY_TEXT}
- } - export default function () { - return
- } - `, + [`app/routes${HAS_BOUNDARY_LOADER_FILE}.jsx`]: js` + export function loader() { + throw new Error("Kaboom!") + } + export function ErrorBoundary() { + return
${OWN_BOUNDARY_TEXT}
+ } + export default function () { + return
+ } + `, - [`app/routes${NO_BOUNDARY_LOADER_FILE}.jsx`]: js` - export function loader() { - throw new Error("Kaboom!") - } - export default function () { - return
- } - `, + [`app/routes${NO_BOUNDARY_LOADER_FILE}.jsx`]: js` + export function loader() { + throw new Error("Kaboom!") + } + export default function () { + return
+ } + `, - [`app/routes${NO_BOUNDARY_RENDER_FILE}.jsx`]: js` - export default function () { - throw new Error("Kaboom!") - return
- } - `, + [`app/routes${NO_BOUNDARY_RENDER_FILE}.jsx`]: js` + export default function () { + throw new Error("Kaboom!") + return
+ } + `, - [`app/routes${HAS_BOUNDARY_RENDER_FILE}.jsx`]: js` - export default function () { - throw new Error("Kaboom!") - return
- } + [`app/routes${HAS_BOUNDARY_RENDER_FILE}.jsx`]: js` + export default function () { + throw new Error("Kaboom!") + return
+ } - export function ErrorBoundary() { - return
${OWN_BOUNDARY_TEXT}
- } - `, + export function ErrorBoundary() { + return
${OWN_BOUNDARY_TEXT}
+ } + `, - [`app/routes${HAS_BOUNDARY_NO_LOADER_OR_ACTION_FILE}.jsx`]: js` - export function ErrorBoundary() { - return
${OWN_BOUNDARY_TEXT}
- } - export default function Index() { - return
- } - `, + [`app/routes${HAS_BOUNDARY_NO_LOADER_OR_ACTION_FILE}.jsx`]: js` + export function ErrorBoundary() { + return
${OWN_BOUNDARY_TEXT}
+ } + export default function Index() { + return
+ } + `, - [`app/routes${NO_BOUNDARY_NO_LOADER_OR_ACTION_FILE}.jsx`]: js` - export default function Index() { - return
- } - `, + [`app/routes${NO_BOUNDARY_NO_LOADER_OR_ACTION_FILE}.jsx`]: js` + export default function Index() { + return
+ } + `, - "app/routes/fetcher-boundary.jsx": js` - import { useFetcher } from "@remix-run/react"; - export function ErrorBoundary() { - return

${OWN_BOUNDARY_TEXT}

- } - export default function() { - let fetcher = useFetcher(); + "app/routes/fetcher-boundary.jsx": js` + import { useFetcher } from "@remix-run/react"; + export function ErrorBoundary() { + return

${OWN_BOUNDARY_TEXT}

+ } + export default function() { + let fetcher = useFetcher(); - return ( -
- -
- ) - } - `, + return ( +
+ +
+ ) + } + `, - "app/routes/fetcher-no-boundary.jsx": js` - import { useFetcher } from "@remix-run/react"; - export default function() { - let fetcher = useFetcher(); + "app/routes/fetcher-no-boundary.jsx": js` + import { useFetcher } from "@remix-run/react"; + export default function() { + let fetcher = useFetcher(); - return ( -
- - - -
- ) - } - `, + return ( +
+ + + +
+ ) + } + `, - "app/routes/action.jsx": js` - import { Outlet, useLoaderData } from "@remix-run/react"; + "app/routes/action.jsx": js` + import { Outlet, useLoaderData } from "@remix-run/react"; - export function loader() { - return "PARENT"; - } + export function loader() { + return "PARENT"; + } - export default function () { - return ( -
-

{useLoaderData()}

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

{useLoaderData()}

+ +
+ ) + } + `, - "app/routes/action.child-error.jsx": js` - import { Form, useLoaderData, useRouteError } from "@remix-run/react"; + "app/routes/action.child-error.jsx": js` + import { Form, useLoaderData, useRouteError } from "@remix-run/react"; - export function loader() { - return "CHILD"; - } + export function loader() { + return "CHILD"; + } - export function action() { - throw new Error("Broken!"); - } + export function action() { + throw new Error("Broken!"); + } - export default function () { - return ( - <> -

{useLoaderData()}

-
- -
- - ) - } + export default function () { + return ( + <> +

{useLoaderData()}

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

{error.message}

; - } - `, + export function ErrorBoundary() { + let error = useRouteError(); + return

{error.message}

; + } + `, + }, }, - }); + ServerMode.Development + ); - appFixture = await createAppFixture(fixture); + appFixture = await createAppFixture(fixture, ServerMode.Development); }); test.afterAll(() => { @@ -2249,13 +2264,16 @@ test.describe("v2_errorBoundary", () => { test.describe("When the root route does not have a boundary", () => { test.beforeAll(async () => { - fixture = await createFixture({ - future: { - v2_routeConvention: true, - v2_errorBoundary: true, + fixture = await createFixture( + { + future: { + v2_routeConvention: true, + v2_errorBoundary: true, + }, + files: getFiles({ includeRootErrorBoundary: false }), }, - files: getFiles({ includeRootErrorBoundary: false }), - }); + ServerMode.Development + ); appFixture = await createAppFixture(fixture, ServerMode.Development); }); @@ -2321,12 +2339,15 @@ test.describe("v2_errorBoundary", () => { test.describe("When the root route has a boundary", () => { test.beforeAll(async () => { - fixture = await createFixture({ - future: { - v2_routeConvention: true, + fixture = await createFixture( + { + future: { + v2_routeConvention: true, + }, + files: getFiles({ includeRootErrorBoundary: true }), }, - files: getFiles({ includeRootErrorBoundary: true }), - }); + ServerMode.Development + ); appFixture = await createAppFixture(fixture, ServerMode.Development); }); diff --git a/integration/error-sanitization-test.ts b/integration/error-sanitization-test.ts new file mode 100644 index 00000000000..4ed62a49faa --- /dev/null +++ b/integration/error-sanitization-test.ts @@ -0,0 +1,372 @@ +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"; + +const routeFiles = { + "app/root.jsx": js` + import { Links, Meta, Outlet, Scripts } from "@remix-run/react"; + + export default function Root() { + return ( + + + + + + +
+ +
+ + + + ); + } + `, + + "app/routes/index.jsx": js` + import { useLoaderData, useLocation } from "@remix-run/react"; + + export function loader({ request }) { + if (new URL(request.url).searchParams.has('loader')) { + throw new Error("Loader Error"); + } + return "LOADER" + } + + export default function Component() { + let data = useLoaderData(); + let location = useLocation(); + + if (location.search.includes('render')) { + throw new Error("Render Error"); + } + + return ( + <> +

Index Route

+

{JSON.stringify(data)}

+ + ); + } + + export function ErrorBoundary({ error }) { + return ( + <> +

Index Error

+

{"MESSAGE:" + error.message}

+ {error.stack ?

{"STACK:" + error.stack}

: null} + + ); + } + `, + + "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"; + + export function loader({ request }) { + if (new URL(request.url).searchParams.has('loader')) { + return defer({ + lazy: Promise.reject(new Error("REJECTED")), + }) + } + return defer({ + lazy: Promise.resolve("RESOLVED"), + }) + } + + export default function Component() { + let data = useLoaderData(); + + return ( + <> +

Defer Route

+ Loading...

}> + }> + {(val) =>

{val}

} + + + + ); + } + + function AwaitError() { + let error = useRouteError(); + return ( + <> +

Defer Error

+

{error}

+ + ); + } + + export function ErrorBoundary({ error }) { + return ( + <> +

Index Error

+

{"MESSAGE:" + error.message}

+ {error.stack ?

{"STACK:" + error.stack}

: null} + + ); + } + `, + + "app/routes/resource.jsx": js` + export function loader({ request }) { + if (new URL(request.url).searchParams.has('loader')) { + throw new Error("Loader Error"); + } + return "RESOURCE LOADER" + } + `, +}; + +test.describe("Error Sanitization", () => { + let fixture: Fixture; + let oldConsoleError: () => void; + + test.beforeEach(() => { + oldConsoleError = console.error; + console.error = () => {}; + }); + + test.afterEach(() => { + console.error = oldConsoleError; + }); + + test.describe("serverMode=production", () => { + test.beforeAll(async () => { + fixture = await createFixture( + { + files: 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); + }); + + 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); + }); + + 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;"); + }); + + 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"}'); + }); + + 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' + ); + }); + + 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"}'); + }); + + 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"}'); + }); + }); + + test.describe("serverMode=development", () => { + test.beforeAll(async () => { + fixture = await createFixture( + { + files: routeFiles, + }, + ServerMode.Development + ); + }); + let ogEnv = process.env.NODE_ENV; + test.beforeEach(() => { + ogEnv = process.env.NODE_ENV; + process.env.NODE_ENV = "development"; + }); + test.afterEach(() => { + process.env.NODE_ENV = ogEnv; + }); + + 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("does not sanitize 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:Loader Error"); + expect(html).toMatch("

STACK:Error: Loader Error"); + expect(html).toMatch( + 'errors":{"routes/index":{"message":"Loader Error","stack":"Error: Loader Error\\n' + ); + }); + + test("does not sanitize 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:Render Error"); + expect(html).toMatch("

STACK:Error: Render Error"); + expect(html).toMatch( + 'errors":{"routes/index":{"message":"Render Error","stack":"Error: Render Error\\n' + ); + }); + + 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:"); + expect(html).not.toMatch(/"stack":/i); + }); + + test("does not sanitize 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"); + // 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;"); + }); + + 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("does not sanitize loader errors in data requests", async () => { + let response = await fixture.requestData("/?loader", "routes/index"); + let text = await response.text(); + expect(text).toMatch( + '{"message":"Loader Error","stack":"Error: Loader Error' + ); + }); + + 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("does not sanitize loader errors in deferred data requests", async () => { + let response = await fixture.requestData("/defer?loader", "routes/defer"); + let text = await response.text(); + expect(text).toMatch( + 'error:{"lazy":{"message":"REJECTED","stack":"Error: REJECTED' + ); + }); + + test("does not sanitize loader errors in resource requests", async () => { + let response = await fixture.requestData( + "/resource?loader", + "routes/resource" + ); + let text = await response.text(); + expect(text).toMatch( + '{"message":"Loader Error","stack":"Error: Loader Error' + ); + }); + + test("sanitizes mismatched route errors in data requests", async () => { + let response = await fixture.requestData("/", "not-a-route"); + let text = await response.text(); + expect(text).toMatch( + '{"message":"Route \\"not-a-route\\" does not match URL \\"/\\"","stack":"Error: Route \\"not-a-route\\" does not match URL \\"/\\"' + ); + }); + }); +}); diff --git a/integration/helpers/create-fixture.ts b/integration/helpers/create-fixture.ts index f10aca44dc1..3920557bcd9 100644 --- a/integration/helpers/create-fixture.ts +++ b/integration/helpers/create-fixture.ts @@ -6,7 +6,7 @@ import getPort from "get-port"; import stripIndent from "strip-indent"; import { sync as spawnSync } from "cross-spawn"; import type { JsonObject } from "type-fest"; -import type { ServerMode } from "@remix-run/server-runtime/mode"; +import { ServerMode } from "@remix-run/server-runtime/mode"; import type { FutureConfig } from "@remix-run/server-runtime/entry"; import type { ServerBuild } from "../../build/node_modules/@remix-run/server-runtime"; @@ -34,11 +34,11 @@ export function json(value: JsonObject) { return JSON.stringify(value, null, 2); } -export async function createFixture(init: FixtureInit) { - let projectDir = await createFixtureProject(init); +export async function createFixture(init: FixtureInit, mode?: ServerMode) { + let projectDir = await createFixtureProject(init, mode); let buildPath = path.resolve(projectDir, "build"); let app: ServerBuild = await import(buildPath); - let handler = createRequestHandler(app, "production"); + let handler = createRequestHandler(app, mode || ServerMode.Production); let requestDocument = async (href: string, init?: RequestInit) => { let url = new URL(href, "test://test"); @@ -106,7 +106,7 @@ export async function createAppFixture(fixture: Fixture, mode?: ServerMode) { "*", createExpressHandler({ build: fixture.build, - mode: mode || "production", + mode: mode || ServerMode.Production, }) ); @@ -141,7 +141,8 @@ export async function createAppFixture(fixture: Fixture, mode?: ServerMode) { //////////////////////////////////////////////////////////////////////////////// export async function createFixtureProject( - init: FixtureInit = {} + init: FixtureInit = {}, + mode?: ServerMode ): Promise { let template = init.template ?? "node-template"; let integrationTemplateDir = path.join(__dirname, template); @@ -193,17 +194,29 @@ export async function createFixtureProject( } await writeTestFiles(init, projectDir); - build(projectDir, init.buildStdio, init.sourcemap); + build(projectDir, init.buildStdio, init.sourcemap, mode); return projectDir; } -function build(projectDir: string, buildStdio?: Writable, sourcemap?: boolean) { +function build( + projectDir: string, + buildStdio?: Writable, + sourcemap?: boolean, + mode?: ServerMode +) { let buildArgs = ["node_modules/@remix-run/dev/dist/cli.js", "build"]; if (sourcemap) { buildArgs.push("--sourcemap"); } - let buildSpawn = spawnSync("node", buildArgs, { cwd: projectDir }); + + let buildSpawn = spawnSync("node", buildArgs, { + cwd: projectDir, + env: { + ...process.env, + NODE_ENV: mode || ServerMode.Production, + }, + }); // These logs are helpful for debugging. Remove comments if needed. // console.log("spawning @remix-run/dev/cli.js `build`:\n"); diff --git a/packages/remix-react/components.tsx b/packages/remix-react/components.tsx index eea487082d4..f0898abd129 100644 --- a/packages/remix-react/components.tsx +++ b/packages/remix-react/components.tsx @@ -853,8 +853,9 @@ export function Scripts(props: ScriptProps) { : [ "__remixContext.p = function(v,e,p,x) {", " if (typeof e !== 'undefined') {", - " x=new Error(e.message);", - process.env.NODE_ENV === "development" ? `x.stack=e.stack;` : "", + process.env.NODE_ENV === "development" + ? " x=new Error(e.message);\n x.stack=e.stack;" + : ' x=new Error("Unexpected Server Error");\n x.stack=undefined;', " p=Promise.reject(x);", " } else {", " p=Promise.resolve(v);", @@ -873,8 +874,9 @@ export function Scripts(props: ScriptProps) { "__remixContext.r = function(i,k,v,e,p,x) {", " p = __remixContext.t[i][k];", " if (typeof e !== 'undefined') {", - " x=new Error(e.message);", - process.env.NODE_ENV === "development" ? `x.stack=e.stack;` : "", + process.env.NODE_ENV === "development" + ? " x=new Error(e.message);\n x.stack=e.stack;" + : ' x=new Error("Unexpected Server Error");\n x.stack=undefined;', " p.e(x);", " } else {", " p.r(v);", @@ -904,13 +906,16 @@ export function Scripts(props: ScriptProps) { } else { let trackedPromise = deferredData.data[key] as TrackedPromise; if (typeof trackedPromise._error !== "undefined") { - let toSerialize: { message: string; stack?: string } = { - message: trackedPromise._error.message, - stack: undefined, - }; - if (process.env.NODE_ENV === "development") { - toSerialize.stack = trackedPromise._error.stack; - } + let toSerialize: { message: string; stack?: string } = + process.env.NODE_ENV === "development" + ? { + message: trackedPromise._error.message, + stack: trackedPromise._error.stack, + } + : { + message: "Unexpected Server Error", + stack: undefined, + }; return `${JSON.stringify( key )}:__remixContext.p(!1, ${escapeHtml( @@ -1116,13 +1121,16 @@ function ErrorDeferredHydrationScript({ routeId: string; }) { let error = useAsyncError() as Error; - let toSerialize: { message: string; stack?: string } = { - message: error.message, - stack: undefined, - }; - if (process.env.NODE_ENV === "development") { - toSerialize.stack = error.stack; - } + let toSerialize: { message: string; stack?: string } = + process.env.NODE_ENV === "development" + ? { + message: error.message, + stack: error.stack, + } + : { + message: "Unexpected Server Error", + stack: undefined, + }; return (