diff --git a/.changeset/cold-geese-turn.md b/.changeset/cold-geese-turn.md new file mode 100644 index 0000000000..5a89283859 --- /dev/null +++ b/.changeset/cold-geese-turn.md @@ -0,0 +1,5 @@ +--- +"react-router": patch +--- + +[UNSTABLE] Add ``/`` prop for client side error reporting diff --git a/integration/browser-entry-test.ts b/integration/browser-entry-test.ts index c018893bac..ae4e831296 100644 --- a/integration/browser-entry-test.ts +++ b/integration/browser-entry-test.ts @@ -129,3 +129,70 @@ test("allows users to pass a client side context to HydratedRouter", async ({ appFixture.close(); }); + +test("allows users to pass an onError function to HydratedRouter", async ({ + page, + browserName, +}) => { + let fixture = await createFixture({ + files: { + "app/entry.client.tsx": js` + import { HydratedRouter } from "react-router/dom"; + import { startTransition, StrictMode } from "react"; + import { hydrateRoot } from "react-dom/client"; + + startTransition(() => { + hydrateRoot( + document, + + { + console.log(error.message, JSON.stringify(errorInfo)) + }} + /> + + ); + }); + `, + "app/routes/_index.tsx": js` + import { Link } from "react-router"; + export default function Index() { + return Go to Page; + } + `, + "app/routes/page.tsx": js` + export default function Page() { + throw new Error("Render error"); + } + export function ErrorBoundary({ error }) { + return

Error: {error.message}

+ } + `, + }, + }); + + let logs: string[] = []; + page.on("console", (msg) => logs.push(msg.text())); + + let appFixture = await createAppFixture(fixture); + let app = new PlaywrightFixture(appFixture, page); + + await app.goto("/", true); + await page.click('a[href="/page"]'); + await page.waitForSelector("[data-error]"); + + expect(await app.getHtml()).toContain("Error: Render error"); + expect(logs.length).toBe(2); + // First one is react logging the error + if (browserName === "firefox") { + expect(logs[0]).toContain("Error"); + } else { + expect(logs[0]).toContain("Error: Render error"); + } + expect(logs[0]).not.toContain("componentStack"); + // Second one is ours + expect(logs[1]).toContain("Render error"); + expect(logs[1]).toContain('"componentStack":'); + + appFixture.close(); +}); diff --git a/packages/react-router/__tests__/dom/client-on-error-test.tsx b/packages/react-router/__tests__/dom/client-on-error-test.tsx new file mode 100644 index 0000000000..405397224c --- /dev/null +++ b/packages/react-router/__tests__/dom/client-on-error-test.tsx @@ -0,0 +1,424 @@ +import { + act, + fireEvent, + render, + screen, + waitFor, +} from "@testing-library/react"; +import * as React from "react"; + +import { + Await, + RouterProvider, + createMemoryRouter, + useFetcher, + useLoaderData, +} from "../../index"; + +import { createFormData } from "../router/utils/utils"; +import getHtml from "../utils/getHtml"; + +describe(`handleError`, () => { + let consoleError: jest.SpyInstance; + + beforeEach(() => { + consoleError = jest.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + consoleError.mockRestore(); + }); + + it("handles navigation loader errors", async () => { + let spy = jest.fn(); + let router = createMemoryRouter([ + { + path: "/", + Component: () =>

Home

, + }, + { + path: "/page", + loader() { + throw new Error("loader error!"); + }, + Component: () =>

Page

, + ErrorBoundary: () =>

Error

, + }, + ]); + + let { container } = render( + , + ); + + await act(() => router.navigate("/page")); + + expect(spy).toHaveBeenCalledWith(new Error("loader error!")); + expect(spy).toHaveBeenCalledTimes(1); + expect(getHtml(container)).toContain("Error"); + }); + + it("handles navigation action errors", async () => { + let spy = jest.fn(); + let router = createMemoryRouter([ + { + path: "/", + Component: () =>

Home

, + }, + { + path: "/page", + action() { + throw new Error("action error!"); + }, + Component: () =>

Page

, + ErrorBoundary: () =>

Error

, + }, + ]); + + let { container } = render( + , + ); + + await act(() => + router.navigate("/page", { + formMethod: "post", + formData: createFormData({}), + }), + ); + + expect(spy).toHaveBeenCalledWith(new Error("action error!")); + expect(spy).toHaveBeenCalledTimes(1); + expect(getHtml(container)).toContain("Error"); + }); + + it("handles fetcher loader errors", async () => { + let spy = jest.fn(); + let router = createMemoryRouter([ + { + path: "/", + Component: () =>

Home

, + }, + { + path: "/fetch", + loader() { + throw new Error("loader error!"); + }, + }, + ]); + + let { container } = render( + , + ); + + await act(() => router.fetch("key", "0", "/fetch")); + + expect(spy).toHaveBeenCalledWith(new Error("loader error!")); + expect(spy).toHaveBeenCalledTimes(1); + expect(getHtml(container)).toContain("Error"); + }); + + it("handles fetcher action errors", async () => { + let spy = jest.fn(); + let router = createMemoryRouter([ + { + path: "/", + Component: () =>

Home

, + }, + { + path: "/fetch", + action() { + throw new Error("action error!"); + }, + }, + ]); + + let { container } = render( + , + ); + + await act(() => + router.fetch("key", "0", "/fetch", { + formMethod: "post", + formData: createFormData({}), + }), + ); + + expect(spy).toHaveBeenCalledWith(new Error("action error!")); + expect(spy).toHaveBeenCalledTimes(1); + expect(getHtml(container)).toContain("Error"); + }); + + it("handles render errors", async () => { + let spy = jest.fn(); + let router = createMemoryRouter([ + { + path: "/", + Component: () =>

Home

, + }, + { + path: "/page", + Component: () => { + throw new Error("render error!"); + }, + ErrorBoundary: () =>

Error

, + }, + ]); + + let { container } = render( + , + ); + + await act(() => router.navigate("/page")); + + expect(spy).toHaveBeenCalledWith( + new Error("render error!"), + expect.objectContaining({ + componentStack: expect.any(String), + }), + ); + expect(spy).toHaveBeenCalledTimes(1); + expect(getHtml(container)).toContain("Error"); + }); + + it("handles deferred data rejections from ", async () => { + let spy = jest.fn(); + let router = createMemoryRouter([ + { + path: "/", + Component: () =>

Home

, + }, + { + path: "/page", + loader() { + return { + promise: new Promise((_, r) => + setTimeout(() => r(new Error("await error!")), 1), + ), + }; + }, + Component() { + let data = useLoaderData(); + return ( + Await Error}> + {() =>

Should not see me

} +
+ ); + }, + }, + ]); + + let { container } = render( + , + ); + + await act(() => router.navigate("/page")); + await waitFor(() => screen.getByText("Await Error")); + + expect(spy).toHaveBeenCalledWith(new Error("await error!")); + expect(spy).toHaveBeenCalledTimes(1); + expect(getHtml(container)).toContain("Await Error"); + }); + + it("handles render errors from Await components", async () => { + let spy = jest.fn(); + let router = createMemoryRouter([ + { + path: "/", + Component: () =>

Home

, + }, + { + path: "/page", + loader() { + return { + promise: new Promise((r) => setTimeout(() => r("data"), 10)), + }; + }, + Component() { + let data = useLoaderData(); + return ( + Loading...

}> + Await Error}> + + +
+ ); + }, + }, + ]); + + function RenderAwait() { + throw new Error("await error!"); + // eslint-disable-next-line no-unreachable + return

should not see me

; + } + + let { container } = render( + , + ); + + await act(() => router.navigate("/page")); + await waitFor(() => screen.getByText("Await Error")); + + expect(spy).toHaveBeenCalledWith( + new Error("await error!"), + expect.objectContaining({ + componentStack: expect.any(String), + }), + ); + expect(spy).toHaveBeenCalledTimes(1); + expect(getHtml(container)).toContain("Await Error"); + }); + + it("handles render errors from Await errorElement", async () => { + let spy = jest.fn(); + let router = createMemoryRouter([ + { + path: "/", + Component: () =>

Home

, + }, + { + path: "/page", + loader() { + return { + promise: new Promise((_, r) => + setTimeout(() => r(new Error("await error!")), 10), + ), + }; + }, + Component() { + let data = useLoaderData(); + return ( + Loading...

}> + }> + {() =>

Should not see me

} +
+
+ ); + }, + ErrorBoundary: () =>

Route Error

, + }, + ]); + + function RenderError() { + throw new Error("errorElement error!"); + // eslint-disable-next-line no-unreachable + return

should not see me

; + } + + let { container } = render( + , + ); + + await act(() => router.navigate("/page")); + await waitFor(() => screen.getByText("Route Error")); + + expect(spy).toHaveBeenCalledWith(new Error("await error!")); + expect(spy).toHaveBeenCalledWith( + new Error("errorElement error!"), + expect.objectContaining({ + componentStack: expect.any(String), + }), + ); + expect(spy).toHaveBeenCalledTimes(2); + expect(getHtml(container)).toContain("Route Error"); + }); + + it("doesn't double report on state updates during an error boundary from a data error", async () => { + let spy = jest.fn(); + let router = createMemoryRouter([ + { + path: "/", + Component: () =>

Home

, + }, + { + path: "/page", + loader() { + throw new Error("loader error!"); + }, + Component: () =>

Page

, + ErrorBoundary() { + let fetcher = useFetcher(); + return ( + <> +

Error

+ +

{fetcher.data}

+ + ); + }, + }, + { + path: "/fetch", + loader() { + return "FETCH"; + }, + }, + ]); + + let { container } = render( + , + ); + + await act(() => router.navigate("/page")); + + expect(spy).toHaveBeenCalledWith(new Error("loader error!")); + expect(spy).toHaveBeenCalledTimes(1); + expect(getHtml(container)).toContain("Error"); + + // Doesn't re-call on a fetcher update from a rendered error boundary + await fireEvent.click(container.querySelector("button")!); + await waitFor(() => screen.getByText("FETCH")); + expect(spy.mock.calls.length).toBe(1); + }); + + it("doesn't double report on state updates during an error boundary from a render error", async () => { + let spy = jest.fn(); + let router = createMemoryRouter([ + { + path: "/", + Component: () =>

Home

, + }, + { + path: "/page", + Component: () => { + throw new Error("render error!"); + }, + ErrorBoundary() { + let fetcher = useFetcher(); + return ( + <> +

Error

+ +

{fetcher.data}

+ + ); + }, + }, + { + path: "/fetch", + loader() { + return "FETCH"; + }, + }, + ]); + + let { container } = render( + , + ); + + await act(() => router.navigate("/page")); + + expect(spy).toHaveBeenCalledWith( + new Error("render error!"), + expect.objectContaining({ + componentStack: expect.any(String), + }), + ); + expect(spy).toHaveBeenCalledTimes(1); + expect(getHtml(container)).toContain("Error"); + + // Doesn't re-call on a fetcher update from a rendered error boundary + await fireEvent.click(container.querySelector("button")!); + await waitFor(() => screen.getByText("FETCH")); + expect(spy.mock.calls.length).toBe(1); + }); +}); diff --git a/packages/react-router/index.ts b/packages/react-router/index.ts index 9920d59237..b873d7a3d3 100644 --- a/packages/react-router/index.ts +++ b/packages/react-router/index.ts @@ -97,6 +97,7 @@ export type { export type { AwaitProps, IndexRouteProps, + unstable_ClientOnErrorFunction, LayoutRouteProps, MemoryRouterOpts, MemoryRouterProps, diff --git a/packages/react-router/lib/components.tsx b/packages/react-router/lib/components.tsx index 1ba32f0150..c2c7e733c9 100644 --- a/packages/react-router/lib/components.tsx +++ b/packages/react-router/lib/components.tsx @@ -245,6 +245,14 @@ class Deferred { } } +/** + * Function signature for client side error handling for loader/actions errors + * and rendering errors via `componentDidCatch` + */ +export interface unstable_ClientOnErrorFunction { + (error: unknown, errorInfo?: React.ErrorInfo): void; +} + /** * @category Types */ @@ -263,6 +271,24 @@ export interface RouterProviderProps { * `RouterProvider` from `react-router` and ignore this prop */ flushSync?: (fn: () => unknown) => undefined; + /** + * An error handler function that will be called for any loader/action/render + * errors that are encountered in your application. This is useful for + * logging or reporting errors instead of the `ErrorBoundary` because it's not + * subject to re-rendering and will only run one time per error. + * + * The `errorInfo` parameter is passed along from + * [`componentDidCatch`](https://react.dev/reference/react/Component#componentdidcatch) + * and is only present for render errors. + * + * ```tsx + * { + * console.error(error, errorInfo); + * reportToErrorService(error, errorInfo); + * }} /> + * ``` + */ + unstable_onError?: unstable_ClientOnErrorFunction; } /** @@ -292,12 +318,14 @@ export interface RouterProviderProps { * @mode data * @param props Props * @param {RouterProviderProps.flushSync} props.flushSync n/a + * @param {RouterProviderProps.unstable_onError} props.unstable_onError n/a * @param {RouterProviderProps.router} props.router n/a * @returns React element for the rendered router */ export function RouterProvider({ router, flushSync: reactDomFlushSyncImpl, + unstable_onError, }: RouterProviderProps): React.ReactElement { let [state, setStateImpl] = React.useState(router.state); let [pendingState, setPendingState] = React.useState(); @@ -312,6 +340,22 @@ export function RouterProvider({ nextLocation: Location; }>(); let fetcherData = React.useRef>(new Map()); + let logErrorsAndSetState = React.useCallback( + (newState: RouterState) => { + setStateImpl((prevState) => { + // Send loader/action errors through handleError + if (newState.errors && unstable_onError) { + Object.entries(newState.errors).forEach(([routeId, error]) => { + if (prevState.errors?.[routeId] !== error) { + unstable_onError(error); + } + }); + } + return newState; + }); + }, + [unstable_onError], + ); let setState = React.useCallback( ( @@ -351,9 +395,9 @@ export function RouterProvider({ // just update and be done with it if (!viewTransitionOpts || !isViewTransitionAvailable) { if (reactDomFlushSyncImpl && flushSync) { - reactDomFlushSyncImpl(() => setStateImpl(newState)); + reactDomFlushSyncImpl(() => logErrorsAndSetState(newState)); } else { - React.startTransition(() => setStateImpl(newState)); + React.startTransition(() => logErrorsAndSetState(newState)); } return; } @@ -377,7 +421,7 @@ export function RouterProvider({ // Update the DOM let t = router.window!.document.startViewTransition(() => { - reactDomFlushSyncImpl(() => setStateImpl(newState)); + reactDomFlushSyncImpl(() => logErrorsAndSetState(newState)); }); // Clean up after the animation completes @@ -416,7 +460,13 @@ export function RouterProvider({ }); } }, - [router.window, reactDomFlushSyncImpl, transition, renderDfd], + [ + router.window, + reactDomFlushSyncImpl, + transition, + renderDfd, + logErrorsAndSetState, + ], ); // Need to use a layout effect here so we are subscribed early enough to @@ -439,7 +489,7 @@ export function RouterProvider({ let newState = pendingState; let renderPromise = renderDfd.promise; let transition = router.window.document.startViewTransition(async () => { - React.startTransition(() => setStateImpl(newState)); + React.startTransition(() => logErrorsAndSetState(newState)); await renderPromise; }); transition.finished.finally(() => { @@ -450,7 +500,7 @@ export function RouterProvider({ }); setTransition(transition); } - }, [pendingState, renderDfd, router.window]); + }, [pendingState, renderDfd, router.window, logErrorsAndSetState]); // When the new location finally renders and is committed to the DOM, this // effect will run to resolve the transition @@ -506,8 +556,9 @@ export function RouterProvider({ navigator, static: false, basename, + unstable_onError, }), - [router, navigator, basename], + [router, navigator, basename, unstable_onError], ); // The fragment and {null} here are important! We need them to keep React 18's @@ -532,6 +583,7 @@ export function RouterProvider({ routes={router.routes} future={router.future} state={state} + unstable_onError={unstable_onError} /> @@ -550,12 +602,14 @@ function DataRoutes({ routes, future, state, + unstable_onError, }: { routes: DataRouteObject[]; future: DataRouter["future"]; state: RouterState; + unstable_onError: unstable_ClientOnErrorFunction | undefined; }): React.ReactElement | null { - return useRoutesImpl(routes, undefined, state, future); + return useRoutesImpl(routes, undefined, state, unstable_onError, future); } /** @@ -1338,8 +1392,13 @@ export function Await({ errorElement, resolve, }: AwaitProps) { + let dataRouterContext = React.useContext(DataRouterContext); return ( - + {children} ); @@ -1348,6 +1407,7 @@ export function Await({ type AwaitErrorBoundaryProps = React.PropsWithChildren<{ errorElement?: React.ReactNode; resolve: TrackedPromise | any; + unstable_onError?: unstable_ClientOnErrorFunction; }>; type AwaitErrorBoundaryState = { @@ -1373,12 +1433,17 @@ class AwaitErrorBoundary extends React.Component< return { error }; } - componentDidCatch(error: any, errorInfo: any) { - console.error( - " caught the following error during render", - error, - errorInfo, - ); + componentDidCatch(error: any, errorInfo: React.ErrorInfo) { + if (this.props.unstable_onError) { + // Log render errors + this.props.unstable_onError(error, errorInfo); + } else { + console.error( + " caught the following error during render", + error, + errorInfo, + ); + } } render() { @@ -1416,8 +1481,11 @@ class AwaitErrorBoundary extends React.Component< promise = resolve.then( (data: any) => Object.defineProperty(resolve, "_data", { get: () => data }), - (error: any) => - Object.defineProperty(resolve, "_error", { get: () => error }), + (error: any) => { + // Log promise rejections + this.props.unstable_onError?.(error); + Object.defineProperty(resolve, "_error", { get: () => error }); + }, ); } diff --git a/packages/react-router/lib/context.ts b/packages/react-router/lib/context.ts index 21b9c380a4..5c1e09c425 100644 --- a/packages/react-router/lib/context.ts +++ b/packages/react-router/lib/context.ts @@ -1,8 +1,9 @@ import * as React from "react"; +import type { unstable_ClientOnErrorFunction } from "./components"; import type { History, - Action as NavigationType, Location, + Action as NavigationType, To, } from "./router/history"; import type { @@ -90,6 +91,7 @@ export interface DataRouterContextObject extends Omit { router: Router; staticContext?: StaticHandlerContext; + unstable_onError?: unstable_ClientOnErrorFunction; } export const DataRouterContext = diff --git a/packages/react-router/lib/dom-export/hydrated-router.tsx b/packages/react-router/lib/dom-export/hydrated-router.tsx index b6486d3952..f7b1df4dbf 100644 --- a/packages/react-router/lib/dom-export/hydrated-router.tsx +++ b/packages/react-router/lib/dom-export/hydrated-router.tsx @@ -6,6 +6,7 @@ import type { DataRouter, HydrationState, RouterInit, + unstable_ClientOnErrorFunction, } from "react-router"; import { UNSAFE_getHydrationData as getHydrationData, @@ -222,6 +223,24 @@ export interface HydratedRouterProps { * functions */ unstable_getContext?: RouterInit["unstable_getContext"]; + /** + * An error handler function that will be called for any loader/action/render + * errors that are encountered in your application. This is useful for + * logging or reporting errors instead of the `ErrorBoundary` because it's not + * subject to re-rendering and will only run one time per error. + * + * The `errorInfo` parameter is passed along from + * [`componentDidCatch`](https://react.dev/reference/react/Component#componentdidcatch) + * and is only present for render errors. + * + * ```tsx + * { + * console.error(error, errorInfo); + * reportToErrorService(error, errorInfo); + * }} /> + * ``` + */ + unstable_onError?: unstable_ClientOnErrorFunction; } /** @@ -233,6 +252,7 @@ export interface HydratedRouterProps { * @mode framework * @param props Props * @param {dom.HydratedRouterProps.unstable_getContext} props.unstable_getContext n/a + * @param {dom.HydratedRouterProps.unstable_onError} props.unstable_onError n/a * @returns A React element that represents the hydrated application. */ export function HydratedRouter(props: HydratedRouterProps) { @@ -324,7 +344,10 @@ export function HydratedRouter(props: HydratedRouterProps) { }} > - + {/* diff --git a/packages/react-router/lib/dom/server.tsx b/packages/react-router/lib/dom/server.tsx index 985d1b5717..b294433999 100644 --- a/packages/react-router/lib/dom/server.tsx +++ b/packages/react-router/lib/dom/server.tsx @@ -235,7 +235,7 @@ function DataRoutes({ future: DataRouter["future"]; state: RouterState; }): React.ReactElement | null { - return useRoutesImpl(routes, undefined, state, future); + return useRoutesImpl(routes, undefined, state, undefined, future); } function serializeErrors( diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index fcdfa9b232..ce264d853b 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -51,6 +51,7 @@ import { stripBasename, } from "./router/utils"; import type { SerializeFrom } from "./types/route-data"; +import type { unstable_ClientOnErrorFunction } from "./components"; /** * Resolves a URL against the current {@link Location}. @@ -707,6 +708,7 @@ export function useRoutesImpl( routes: RouteObject[], locationArg?: Partial | string, dataRouterState?: DataRouter["state"], + unstable_onError?: unstable_ClientOnErrorFunction, future?: DataRouter["future"], ): React.ReactElement | null { invariant( @@ -848,6 +850,7 @@ export function useRoutesImpl( ), parentMatches, dataRouterState, + unstable_onError, future, ); @@ -926,6 +929,7 @@ type RenderErrorBoundaryProps = React.PropsWithChildren<{ error: any; component: React.ReactNode; routeContext: RouteContextObject; + unstable_onError: unstable_ClientOnErrorFunction | null; }>; type RenderErrorBoundaryState = { @@ -985,12 +989,15 @@ export class RenderErrorBoundary extends React.Component< }; } - componentDidCatch(error: any, errorInfo: any) { - console.error( - "React Router caught the following error during render", - error, - errorInfo, - ); + componentDidCatch(error: any, errorInfo: React.ErrorInfo) { + if (this.props.unstable_onError) { + this.props.unstable_onError(error, errorInfo); + } else { + console.error( + "React Router caught the following error during render", + error, + ); + } } render() { @@ -1038,6 +1045,7 @@ export function _renderMatches( matches: RouteMatch[] | null, parentMatches: RouteMatch[] = [], dataRouterState: DataRouter["state"] | null = null, + unstable_onError: unstable_ClientOnErrorFunction | null = null, future: DataRouter["future"] | null = null, ): React.ReactElement | null { if (matches == null) { @@ -1194,6 +1202,7 @@ export function _renderMatches( error={error} children={getChildren()} routeContext={{ outlet: null, matches, isDataRoute: true }} + unstable_onError={unstable_onError} /> ) : ( getChildren()