diff --git a/.changeset/cool-pigs-cheer.md b/.changeset/cool-pigs-cheer.md new file mode 100644 index 0000000000..864f1038fb --- /dev/null +++ b/.changeset/cool-pigs-cheer.md @@ -0,0 +1,5 @@ +--- +"react-router": patch +--- + +[UNSTABLE] Ensure resource route errors go through `handleError` w/middleware enabled diff --git a/packages/react-router/__tests__/router/context-middleware-test.ts b/packages/react-router/__tests__/router/context-middleware-test.ts index 999fcf460b..75f40e0e0b 100644 --- a/packages/react-router/__tests__/router/context-middleware-test.ts +++ b/packages/react-router/__tests__/router/context-middleware-test.ts @@ -2704,16 +2704,11 @@ describe("context/middleware", () => { }, ]); - let res = await handler.queryRoute( - new Request("http://localhost/parent/"), - { + await expect( + handler.queryRoute(new Request("http://localhost/parent/"), { unstable_respond: (v) => v, - }, - ); - - expect(await res.text()).toBe( - "Error: You may only call `next()` once per middleware", - ); + }), + ).rejects.toThrow("You may only call `next()` once per middleware"); }); }); @@ -2756,14 +2751,12 @@ describe("context/middleware", () => { ]); let requestContext = new unstable_RouterContextProvider(); - let res = await handler.queryRoute( - new Request("http://localhost/parent/child"), - { + await expect( + handler.queryRoute(new Request("http://localhost/parent/child"), { requestContext, unstable_respond: (v) => v, - }, - ); - expect(await res.text()).toBe("Error: PARENT 2"); + }), + ).rejects.toThrow("PARENT 2"); expect(requestContext.get(parentContext)).toEqual("PARENT 1"); expect(requestContext.get(childContext)).toEqual("empty"); @@ -2808,14 +2801,12 @@ describe("context/middleware", () => { ]); let requestContext = new unstable_RouterContextProvider(); - let res = await handler.queryRoute( - new Request("http://localhost/parent/child"), - { + await expect( + handler.queryRoute(new Request("http://localhost/parent/child"), { requestContext, unstable_respond: (v) => v, - }, - ); - expect(await res.text()).toBe("Error: CHILD UP"); + }), + ).rejects.toThrow("CHILD UP"); expect(requestContext.get(parentContext)).toEqual("PARENT DOWN"); expect(requestContext.get(childContext)).toEqual("CHILD DOWN"); @@ -2883,14 +2874,15 @@ describe("context/middleware", () => { ]); let requestContext = new unstable_RouterContextProvider(); - let res = await handler.queryRoute( - new Request("http://localhost/parent/child", { - method: "post", - body: createFormData({}), - }), - { requestContext, unstable_respond: (v) => v }, - ); - expect(await res.text()).toEqual("Error: child 1 error"); + await expect( + handler.queryRoute( + new Request("http://localhost/parent/child", { + method: "post", + body: createFormData({}), + }), + { requestContext, unstable_respond: (v) => v }, + ), + ).rejects.toThrow("child 1 error"); expect(requestContext.get(orderContext)).toEqual([ "parent start", @@ -2966,14 +2958,15 @@ describe("context/middleware", () => { ]); let requestContext = new unstable_RouterContextProvider(); - let res = await handler.queryRoute( - new Request("http://localhost/parent/child", { - method: "post", - body: createFormData({}), - }), - { requestContext, unstable_respond: (v) => v }, - ); - expect(await res.text()).toEqual("Error: child 2 error"); + await expect( + handler.queryRoute( + new Request("http://localhost/parent/child", { + method: "post", + body: createFormData({}), + }), + { requestContext, unstable_respond: (v) => v }, + ), + ).rejects.toThrow("child 2 error"); expect(requestContext.get(orderContext)).toEqual([ "parent start", @@ -3046,14 +3039,15 @@ describe("context/middleware", () => { ]); let requestContext = new unstable_RouterContextProvider(); - let res = await handler.queryRoute( - new Request("http://localhost/parent/child", { - method: "post", - body: createFormData({}), - }), - { requestContext, unstable_respond: (v) => v }, - ); - expect(await res.text()).toEqual("Error: child 1 action error"); + await expect( + handler.queryRoute( + new Request("http://localhost/parent/child", { + method: "post", + body: createFormData({}), + }), + { requestContext, unstable_respond: (v) => v }, + ), + ).rejects.toThrow("child 1 action error"); expect(requestContext.get(orderContext)).toEqual([ "parent action start", @@ -3129,14 +3123,15 @@ describe("context/middleware", () => { ]); let requestContext = new unstable_RouterContextProvider(); - let res = await handler.queryRoute( - new Request("http://localhost/parent/child", { - method: "post", - body: createFormData({}), - }), - { requestContext, unstable_respond: (v) => v }, - ); - expect(await res.text()).toEqual("Error: child 2 error"); + await expect( + handler.queryRoute( + new Request("http://localhost/parent/child", { + method: "post", + body: createFormData({}), + }), + { requestContext, unstable_respond: (v) => v }, + ), + ).rejects.toThrow("child 2 error"); expect(requestContext.get(orderContext)).toEqual([ "parent start", diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index 8062ea7554..0c81350e19 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -450,6 +450,10 @@ export interface StaticHandler { requestContext?: unknown; dataStrategy?: DataStrategyFunction; unstable_respond?: (res: Response) => MaybePromise; + unstable_stream?: ( + context: unstable_RouterContextProvider, + queryRoute: (r: Request) => Promise, + ) => MaybePromise; }, ): Promise; } @@ -3845,6 +3849,7 @@ export function createStaticHandler( requestContext, dataStrategy, unstable_respond: respond, + unstable_stream: stream, }: Parameters[1] = {}, ): Promise { let url = new URL(request.url); @@ -3878,13 +3883,14 @@ export function createStaticHandler( } if ( - respond && - matches.some( - (m) => - m.route.unstable_middleware || - (typeof m.route.lazy === "object" && - m.route.lazy.unstable_middleware), - ) + stream || + (respond && + matches.some( + (m) => + m.route.unstable_middleware || + (typeof m.route.lazy === "object" && + m.route.lazy.unstable_middleware), + )) ) { invariant( requestContext instanceof unstable_RouterContextProvider, @@ -3903,6 +3909,54 @@ export function createStaticHandler( }, true, async () => { + if (stream) { + let res = await stream( + requestContext as unstable_RouterContextProvider, + async (revalidationRequest: Request) => { + let result = await queryImpl( + revalidationRequest, + location, + matches!, + requestContext, + dataStrategy || null, + false, + match!, + null, + false, + ); + + if (isResponse(result)) { + return result; + } + + let error = result.errors + ? Object.values(result.errors)[0] + : undefined; + + if (error !== undefined) { + // If we got back result.errors, that means the loader/action threw + // _something_ that wasn't a Response, but it's not guaranteed/required + // to be an `instanceof Error` either, so we have to use throw here to + // preserve the "error" state outside of queryImpl. + throw error; + } + + // Pick off the right state value to return + let value = result.actionData + ? Object.values(result.actionData)[0] + : Object.values(result.loaderData)[0]; + + return typeof value === "string" + ? new Response(value) + : Response.json(value); + }, + ); + return res; + } + + // Should always be true given the if statement above + invariant(respond, "Expected respond to be defined"); + let result = await queryImpl( request, location, @@ -3936,18 +3990,17 @@ export function createStaticHandler( ? Object.values(result.actionData)[0] : Object.values(result.loaderData)[0]; - return typeof value === "string" - ? new Response(value) - : Response.json(value); + return respond( + typeof value === "string" + ? new Response(value) + : Response.json(value), + ); }, (error) => { if (isResponse(error)) { - return respond(error); + return respond ? respond(error) : error; } - return new Response(String(error), { - status: 500, - statusText: "Unexpected Server Error", - }); + throw error; }, ); return response; @@ -5595,7 +5648,8 @@ async function callRouteMiddleware( return result; } } else { - return next(); + result = await next(); + return result; } } catch (error) { if (!middlewareState.middlewareError) { diff --git a/packages/react-router/lib/server-runtime/server.ts b/packages/react-router/lib/server-runtime/server.ts index 1558267ff7..8eea6d6cdb 100644 --- a/packages/react-router/lib/server-runtime/server.ts +++ b/packages/react-router/lib/server-runtime/server.ts @@ -619,8 +619,44 @@ async function handleResourceRequest( let response = await staticHandler.queryRoute(request, { routeId, requestContext: loadContext, - unstable_respond: build.future.unstable_middleware - ? (ctx) => ctx + unstable_stream: build.future.unstable_middleware + ? async (_, queryRoute) => { + try { + let result = await queryRoute(request); + if (isResponse(result)) { + return result; + } + + if (typeof result === "string") { + return new Response(result); + } + + return Response.json(result); + } catch (error) { + if (isResponse(error)) { + return error; + } + + if (isRouteErrorResponse(error)) { + handleError(error); + return errorResponseToJson(error, serverMode); + } + + if ( + error instanceof Error && + error.message === "Expected a response from queryRoute" + ) { + let newError = new Error( + "Expected a Response to be returned from resource route handler", + ); + handleError(newError); + return returnLastResortErrorResponse(newError, serverMode); + } + + handleError(error); + return returnLastResortErrorResponse(error, serverMode); + } + } : undefined, });