Skip to content

Commit f90a426

Browse files
brophdawg11brophdawg11
authored
Ensure resource route errors go through handleError w/middleware enabled (#14078)
* Ensure resource route errors go through handleError when middleware is enabled * Update tests * Add changeset
1 parent 542f2a6 commit f90a426

File tree

4 files changed

+161
-71
lines changed

4 files changed

+161
-71
lines changed

‎.changeset/cool-pigs-cheer.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"react-router": patch
3+
---
4+
5+
[UNSTABLE] Ensure resource route errors go through `handleError` w/middleware enabled

‎packages/react-router/__tests__/router/context-middleware-test.ts

Lines changed: 48 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -2704,16 +2704,11 @@ describe("context/middleware", () => {
27042704
},
27052705
]);
27062706

2707-
let res = await handler.queryRoute(
2708-
new Request("http://localhost/parent/"),
2709-
{
2707+
await expect(
2708+
handler.queryRoute(new Request("http://localhost/parent/"), {
27102709
unstable_respond: (v) => v,
2711-
},
2712-
);
2713-
2714-
expect(await res.text()).toBe(
2715-
"Error: You may only call `next()` once per middleware",
2716-
);
2710+
}),
2711+
).rejects.toThrow("You may only call `next()` once per middleware");
27172712
});
27182713
});
27192714

@@ -2756,14 +2751,12 @@ describe("context/middleware", () => {
27562751
]);
27572752

27582753
let requestContext = new unstable_RouterContextProvider();
2759-
let res = await handler.queryRoute(
2760-
new Request("http://localhost/parent/child"),
2761-
{
2754+
await expect(
2755+
handler.queryRoute(new Request("http://localhost/parent/child"), {
27622756
requestContext,
27632757
unstable_respond: (v) => v,
2764-
},
2765-
);
2766-
expect(await res.text()).toBe("Error: PARENT 2");
2758+
}),
2759+
).rejects.toThrow("PARENT 2");
27672760

27682761
expect(requestContext.get(parentContext)).toEqual("PARENT 1");
27692762
expect(requestContext.get(childContext)).toEqual("empty");
@@ -2808,14 +2801,12 @@ describe("context/middleware", () => {
28082801
]);
28092802

28102803
let requestContext = new unstable_RouterContextProvider();
2811-
let res = await handler.queryRoute(
2812-
new Request("http://localhost/parent/child"),
2813-
{
2804+
await expect(
2805+
handler.queryRoute(new Request("http://localhost/parent/child"), {
28142806
requestContext,
28152807
unstable_respond: (v) => v,
2816-
},
2817-
);
2818-
expect(await res.text()).toBe("Error: CHILD UP");
2808+
}),
2809+
).rejects.toThrow("CHILD UP");
28192810

28202811
expect(requestContext.get(parentContext)).toEqual("PARENT DOWN");
28212812
expect(requestContext.get(childContext)).toEqual("CHILD DOWN");
@@ -2883,14 +2874,15 @@ describe("context/middleware", () => {
28832874
]);
28842875

28852876
let requestContext = new unstable_RouterContextProvider();
2886-
let res = await handler.queryRoute(
2887-
new Request("http://localhost/parent/child", {
2888-
method: "post",
2889-
body: createFormData({}),
2890-
}),
2891-
{ requestContext, unstable_respond: (v) => v },
2892-
);
2893-
expect(await res.text()).toEqual("Error: child 1 error");
2877+
await expect(
2878+
handler.queryRoute(
2879+
new Request("http://localhost/parent/child", {
2880+
method: "post",
2881+
body: createFormData({}),
2882+
}),
2883+
{ requestContext, unstable_respond: (v) => v },
2884+
),
2885+
).rejects.toThrow("child 1 error");
28942886

28952887
expect(requestContext.get(orderContext)).toEqual([
28962888
"parent start",
@@ -2966,14 +2958,15 @@ describe("context/middleware", () => {
29662958
]);
29672959

29682960
let requestContext = new unstable_RouterContextProvider();
2969-
let res = await handler.queryRoute(
2970-
new Request("http://localhost/parent/child", {
2971-
method: "post",
2972-
body: createFormData({}),
2973-
}),
2974-
{ requestContext, unstable_respond: (v) => v },
2975-
);
2976-
expect(await res.text()).toEqual("Error: child 2 error");
2961+
await expect(
2962+
handler.queryRoute(
2963+
new Request("http://localhost/parent/child", {
2964+
method: "post",
2965+
body: createFormData({}),
2966+
}),
2967+
{ requestContext, unstable_respond: (v) => v },
2968+
),
2969+
).rejects.toThrow("child 2 error");
29772970

29782971
expect(requestContext.get(orderContext)).toEqual([
29792972
"parent start",
@@ -3046,14 +3039,15 @@ describe("context/middleware", () => {
30463039
]);
30473040

30483041
let requestContext = new unstable_RouterContextProvider();
3049-
let res = await handler.queryRoute(
3050-
new Request("http://localhost/parent/child", {
3051-
method: "post",
3052-
body: createFormData({}),
3053-
}),
3054-
{ requestContext, unstable_respond: (v) => v },
3055-
);
3056-
expect(await res.text()).toEqual("Error: child 1 action error");
3042+
await expect(
3043+
handler.queryRoute(
3044+
new Request("http://localhost/parent/child", {
3045+
method: "post",
3046+
body: createFormData({}),
3047+
}),
3048+
{ requestContext, unstable_respond: (v) => v },
3049+
),
3050+
).rejects.toThrow("child 1 action error");
30573051

30583052
expect(requestContext.get(orderContext)).toEqual([
30593053
"parent action start",
@@ -3129,14 +3123,15 @@ describe("context/middleware", () => {
31293123
]);
31303124

31313125
let requestContext = new unstable_RouterContextProvider();
3132-
let res = await handler.queryRoute(
3133-
new Request("http://localhost/parent/child", {
3134-
method: "post",
3135-
body: createFormData({}),
3136-
}),
3137-
{ requestContext, unstable_respond: (v) => v },
3138-
);
3139-
expect(await res.text()).toEqual("Error: child 2 error");
3126+
await expect(
3127+
handler.queryRoute(
3128+
new Request("http://localhost/parent/child", {
3129+
method: "post",
3130+
body: createFormData({}),
3131+
}),
3132+
{ requestContext, unstable_respond: (v) => v },
3133+
),
3134+
).rejects.toThrow("child 2 error");
31403135

31413136
expect(requestContext.get(orderContext)).toEqual([
31423137
"parent start",

‎packages/react-router/lib/router/router.ts

Lines changed: 70 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,10 @@ export interface StaticHandler {
450450
requestContext?: unknown;
451451
dataStrategy?: DataStrategyFunction<unknown>;
452452
unstable_respond?: (res: Response) => MaybePromise<Response>;
453+
unstable_stream?: (
454+
context: unstable_RouterContextProvider,
455+
queryRoute: (r: Request) => Promise<Response>,
456+
) => MaybePromise<Response>;
453457
},
454458
): Promise<any>;
455459
}
@@ -3845,6 +3849,7 @@ export function createStaticHandler(
38453849
requestContext,
38463850
dataStrategy,
38473851
unstable_respond: respond,
3852+
unstable_stream: stream,
38483853
}: Parameters<StaticHandler["queryRoute"]>[1] = {},
38493854
): Promise<any> {
38503855
let url = new URL(request.url);
@@ -3878,13 +3883,14 @@ export function createStaticHandler(
38783883
}
38793884

38803885
if (
3881-
respond &&
3882-
matches.some(
3883-
(m) =>
3884-
m.route.unstable_middleware ||
3885-
(typeof m.route.lazy === "object" &&
3886-
m.route.lazy.unstable_middleware),
3887-
)
3886+
stream ||
3887+
(respond &&
3888+
matches.some(
3889+
(m) =>
3890+
m.route.unstable_middleware ||
3891+
(typeof m.route.lazy === "object" &&
3892+
m.route.lazy.unstable_middleware),
3893+
))
38883894
) {
38893895
invariant(
38903896
requestContext instanceof unstable_RouterContextProvider,
@@ -3903,6 +3909,54 @@ export function createStaticHandler(
39033909
},
39043910
true,
39053911
async () => {
3912+
if (stream) {
3913+
let res = await stream(
3914+
requestContext as unstable_RouterContextProvider,
3915+
async (revalidationRequest: Request) => {
3916+
let result = await queryImpl(
3917+
revalidationRequest,
3918+
location,
3919+
matches!,
3920+
requestContext,
3921+
dataStrategy || null,
3922+
false,
3923+
match!,
3924+
null,
3925+
false,
3926+
);
3927+
3928+
if (isResponse(result)) {
3929+
return result;
3930+
}
3931+
3932+
let error = result.errors
3933+
? Object.values(result.errors)[0]
3934+
: undefined;
3935+
3936+
if (error !== undefined) {
3937+
// If we got back result.errors, that means the loader/action threw
3938+
// _something_ that wasn't a Response, but it's not guaranteed/required
3939+
// to be an `instanceof Error` either, so we have to use throw here to
3940+
// preserve the "error" state outside of queryImpl.
3941+
throw error;
3942+
}
3943+
3944+
// Pick off the right state value to return
3945+
let value = result.actionData
3946+
? Object.values(result.actionData)[0]
3947+
: Object.values(result.loaderData)[0];
3948+
3949+
return typeof value === "string"
3950+
? new Response(value)
3951+
: Response.json(value);
3952+
},
3953+
);
3954+
return res;
3955+
}
3956+
3957+
// Should always be true given the if statement above
3958+
invariant(respond, "Expected respond to be defined");
3959+
39063960
let result = await queryImpl(
39073961
request,
39083962
location,
@@ -3936,18 +3990,17 @@ export function createStaticHandler(
39363990
? Object.values(result.actionData)[0]
39373991
: Object.values(result.loaderData)[0];
39383992

3939-
return typeof value === "string"
3940-
? new Response(value)
3941-
: Response.json(value);
3993+
return respond(
3994+
typeof value === "string"
3995+
? new Response(value)
3996+
: Response.json(value),
3997+
);
39423998
},
39433999
(error) => {
39444000
if (isResponse(error)) {
3945-
return respond(error);
4001+
return respond ? respond(error) : error;
39464002
}
3947-
return new Response(String(error), {
3948-
status: 500,
3949-
statusText: "Unexpected Server Error",
3950-
});
4003+
throw error;
39514004
},
39524005
);
39534006
return response;
@@ -5595,7 +5648,8 @@ async function callRouteMiddleware(
55955648
return result;
55965649
}
55975650
} else {
5598-
return next();
5651+
result = await next();
5652+
return result;
55995653
}
56005654
} catch (error) {
56015655
if (!middlewareState.middlewareError) {

‎packages/react-router/lib/server-runtime/server.ts

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -619,8 +619,44 @@ async function handleResourceRequest(
619619
let response = await staticHandler.queryRoute(request, {
620620
routeId,
621621
requestContext: loadContext,
622-
unstable_respond: build.future.unstable_middleware
623-
? (ctx) => ctx
622+
unstable_stream: build.future.unstable_middleware
623+
? async (_, queryRoute) => {
624+
try {
625+
let result = await queryRoute(request);
626+
if (isResponse(result)) {
627+
return result;
628+
}
629+
630+
if (typeof result === "string") {
631+
return new Response(result);
632+
}
633+
634+
return Response.json(result);
635+
} catch (error) {
636+
if (isResponse(error)) {
637+
return error;
638+
}
639+
640+
if (isRouteErrorResponse(error)) {
641+
handleError(error);
642+
return errorResponseToJson(error, serverMode);
643+
}
644+
645+
if (
646+
error instanceof Error &&
647+
error.message === "Expected a response from queryRoute"
648+
) {
649+
let newError = new Error(
650+
"Expected a Response to be returned from resource route handler",
651+
);
652+
handleError(newError);
653+
return returnLastResortErrorResponse(newError, serverMode);
654+
}
655+
656+
handleError(error);
657+
return returnLastResortErrorResponse(error, serverMode);
658+
}
659+
}
624660
: undefined,
625661
});
626662

0 commit comments

Comments
 (0)