Skip to content

Commit 0a00110

Browse files
brophdawg11brophdawg11
authored
feat: better handle throws in queryRoute (#9353)
1 parent d8e6d7f commit 0a00110

File tree

2 files changed

+300
-22
lines changed

2 files changed

+300
-22
lines changed

‎packages/router/__tests__/router-test.ts‎

Lines changed: 275 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10069,7 +10069,66 @@ describe("a router", () => {
1006910069
});
1007010070

1007110071
describe("singular route requests", () => {
10072-
it("should support singular route load navigations", async () => {
10072+
function setupFlexRouteTest() {
10073+
function queryRoute(
10074+
req: Request,
10075+
routeId: string,
10076+
type: "loader" | "action",
10077+
data: any,
10078+
isError = false
10079+
) {
10080+
let handler = createStaticHandler([
10081+
{
10082+
id: "flex",
10083+
path: "/flex",
10084+
[type]: () =>
10085+
isError ? Promise.reject(data) : Promise.resolve(data),
10086+
},
10087+
]);
10088+
return handler.queryRoute(req, routeId);
10089+
}
10090+
10091+
return {
10092+
resolveLoader(data: any) {
10093+
return queryRoute(
10094+
createRequest("/flex"),
10095+
"flex",
10096+
"loader",
10097+
data,
10098+
false
10099+
);
10100+
},
10101+
rejectLoader(data: any) {
10102+
return queryRoute(
10103+
createRequest("/flex"),
10104+
"flex",
10105+
"loader",
10106+
data,
10107+
true
10108+
);
10109+
},
10110+
resolveAction(data: any) {
10111+
return queryRoute(
10112+
createSubmitRequest("/flex"),
10113+
"flex",
10114+
"action",
10115+
data,
10116+
false
10117+
);
10118+
},
10119+
rejectAction(data: any) {
10120+
return queryRoute(
10121+
createSubmitRequest("/flex"),
10122+
"flex",
10123+
"action",
10124+
data,
10125+
true
10126+
);
10127+
},
10128+
};
10129+
}
10130+
10131+
it("should support singular route load navigations (primitives)", async () => {
1007310132
let { queryRoute } = createStaticHandler(SSR_ROUTES);
1007410133
let data;
1007510134

@@ -10088,9 +10147,116 @@ describe("a router", () => {
1008810147
// Child in nested route
1008910148
data = await queryRoute(createRequest("/parent/child"), "child");
1009010149
expect(data).toBe("CHILD LOADER");
10150+
10151+
// Non-undefined falsey values should count
10152+
let T = setupFlexRouteTest();
10153+
data = await T.resolveLoader(null);
10154+
expect(data).toBeNull();
10155+
data = await T.resolveLoader(false);
10156+
expect(data).toBe(false);
10157+
data = await T.resolveLoader("");
10158+
expect(data).toBe("");
10159+
});
10160+
10161+
it("should support singular route load navigations (Responses)", async () => {
10162+
let T = setupFlexRouteTest();
10163+
let data;
10164+
10165+
// When Responses are returned or thrown, it should always resolve the
10166+
// raw Response from queryRoute
10167+
10168+
// Returned Success Response
10169+
data = await T.resolveLoader(new Response("Created!", { status: 201 }));
10170+
expect(data.status).toBe(201);
10171+
expect(await data.text()).toBe("Created!");
10172+
10173+
// Thrown Success Response
10174+
data = await T.rejectLoader(new Response("Created!", { status: 201 }));
10175+
expect(data.status).toBe(201);
10176+
expect(await data.text()).toBe("Created!");
10177+
10178+
// Returned Redirect Response
10179+
data = await T.resolveLoader(
10180+
new Response(null, {
10181+
status: 302,
10182+
headers: { Location: "/" },
10183+
})
10184+
);
10185+
expect(data.status).toBe(302);
10186+
expect(data.headers.get("Location")).toBe("/");
10187+
10188+
// Thrown Redirect Response
10189+
data = await T.rejectLoader(
10190+
new Response(null, {
10191+
status: 301,
10192+
headers: { Location: "/" },
10193+
})
10194+
);
10195+
expect(data.status).toBe(301);
10196+
expect(data.headers.get("Location")).toBe("/");
10197+
10198+
// Returned Error Response
10199+
data = await T.resolveLoader(new Response("Why?", { status: 400 }));
10200+
expect(data.status).toBe(400);
10201+
expect(await data.text()).toBe("Why?");
10202+
10203+
// Thrown Error Response
10204+
data = await T.rejectLoader(new Response("Oh no!", { status: 401 }));
10205+
expect(data.status).toBe(401);
10206+
expect(await data.text()).toBe("Oh no!");
10207+
});
10208+
10209+
it("should support singular route load navigations (Errors)", async () => {
10210+
let T = setupFlexRouteTest();
10211+
let data;
10212+
10213+
// Returned Error instance is treated as data since it was not thrown
10214+
data = await T.resolveLoader(new Error("Why?"));
10215+
expect(data).toEqual(new Error("Why?"));
10216+
10217+
// Anything thrown (Error instance or not) will throw from queryRoute
10218+
// so we know to handle it as an errorPath in the server. Generally
10219+
// though in queryRoute, we would expect responses to be coming back -
10220+
// not
10221+
10222+
// Thrown Error
10223+
try {
10224+
await T.rejectLoader(new Error("Oh no!"));
10225+
} catch (e) {
10226+
data = e;
10227+
}
10228+
expect(data).toEqual(new Error("Oh no!"));
10229+
10230+
// Thrown non-Error
10231+
try {
10232+
await T.rejectLoader("This is weird?");
10233+
} catch (e) {
10234+
data = e;
10235+
}
10236+
expect(data).toEqual("This is weird?");
10237+
10238+
// Non-undefined falsey values should count
10239+
try {
10240+
await T.rejectLoader(null);
10241+
} catch (e) {
10242+
data = e;
10243+
}
10244+
expect(data).toBeNull();
10245+
try {
10246+
await T.rejectLoader(false);
10247+
} catch (e) {
10248+
data = e;
10249+
}
10250+
expect(data).toBe(false);
10251+
try {
10252+
await T.rejectLoader("");
10253+
} catch (e) {
10254+
data = e;
10255+
}
10256+
expect(data).toBe("");
1009110257
});
1009210258

10093-
it("should support singular route submit navigations", async () => {
10259+
it("should support singular route submit navigations (primitives)", async () => {
1009410260
let { queryRoute } = createStaticHandler(SSR_ROUTES);
1009510261
let data;
1009610262

@@ -10109,6 +10275,113 @@ describe("a router", () => {
1010910275
// Child in nested route
1011010276
data = await queryRoute(createSubmitRequest("/parent/child"), "child");
1011110277
expect(data).toBe("CHILD ACTION");
10278+
10279+
// Non-undefined falsey values should count
10280+
let T = setupFlexRouteTest();
10281+
data = await T.resolveAction(null);
10282+
expect(data).toBeNull();
10283+
data = await T.resolveAction(false);
10284+
expect(data).toBe(false);
10285+
data = await T.resolveAction("");
10286+
expect(data).toBe("");
10287+
});
10288+
10289+
it("should support singular route submit navigations (Responses)", async () => {
10290+
let T = setupFlexRouteTest();
10291+
let data;
10292+
10293+
// When Responses are returned or thrown, it should always resolve the
10294+
// raw Response from queryRoute
10295+
10296+
// Returned Success Response
10297+
data = await T.resolveAction(new Response("Created!", { status: 201 }));
10298+
expect(data.status).toBe(201);
10299+
expect(await data.text()).toBe("Created!");
10300+
10301+
// Thrown Success Response
10302+
data = await T.rejectAction(new Response("Created!", { status: 201 }));
10303+
expect(data.status).toBe(201);
10304+
expect(await data.text()).toBe("Created!");
10305+
10306+
// Returned Redirect Response
10307+
data = await T.resolveAction(
10308+
new Response(null, {
10309+
status: 302,
10310+
headers: { Location: "/" },
10311+
})
10312+
);
10313+
expect(data.status).toBe(302);
10314+
expect(data.headers.get("Location")).toBe("/");
10315+
10316+
// Thrown Redirect Response
10317+
data = await T.rejectAction(
10318+
new Response(null, {
10319+
status: 301,
10320+
headers: { Location: "/" },
10321+
})
10322+
);
10323+
expect(data.status).toBe(301);
10324+
expect(data.headers.get("Location")).toBe("/");
10325+
10326+
// Returned Error Response
10327+
data = await T.resolveAction(new Response("Why?", { status: 400 }));
10328+
expect(data.status).toBe(400);
10329+
expect(await data.text()).toBe("Why?");
10330+
10331+
// Thrown Error Response
10332+
data = await T.rejectAction(new Response("Oh no!", { status: 401 }));
10333+
expect(data.status).toBe(401);
10334+
expect(await data.text()).toBe("Oh no!");
10335+
});
10336+
10337+
it("should support singular route submit navigations (Errors)", async () => {
10338+
let T = setupFlexRouteTest();
10339+
let data;
10340+
10341+
// Returned Error instance is treated as data since it was not thrown
10342+
data = await T.resolveAction(new Error("Why?"));
10343+
expect(data).toEqual(new Error("Why?"));
10344+
10345+
// Anything thrown (Error instance or not) will throw from queryRoute
10346+
// so we know to handle it as an errorPath in the server. Generally
10347+
// though in queryRoute, we would expect responses to be coming back -
10348+
// not
10349+
10350+
// Thrown Error
10351+
try {
10352+
await T.rejectAction(new Error("Oh no!"));
10353+
} catch (e) {
10354+
data = e;
10355+
}
10356+
expect(data).toEqual(new Error("Oh no!"));
10357+
10358+
// Thrown non-Error
10359+
try {
10360+
await T.rejectAction("This is weird?");
10361+
} catch (e) {
10362+
data = e;
10363+
}
10364+
expect(data).toEqual("This is weird?");
10365+
10366+
// Non-undefined falsey values should count
10367+
try {
10368+
await T.rejectAction(null);
10369+
} catch (e) {
10370+
data = e;
10371+
}
10372+
expect(data).toBeNull();
10373+
try {
10374+
await T.rejectAction(false);
10375+
} catch (e) {
10376+
data = e;
10377+
}
10378+
expect(data).toBe(false);
10379+
try {
10380+
await T.rejectAction("");
10381+
} catch (e) {
10382+
data = e;
10383+
}
10384+
expect(data).toBe("");
1011210385
});
1011310386

1011410387
it("should not unwrap responses returned from loaders", async () => {
@@ -10141,22 +10414,6 @@ describe("a router", () => {
1014110414
expect(await data.json()).toEqual({ key: "value" });
1014210415
});
1014310416

10144-
it("should handle load error responses", async () => {
10145-
let { queryRoute } = createStaticHandler(SSR_ROUTES);
10146-
let data;
10147-
10148-
data = await queryRoute(createRequest("/parent/error"), "error");
10149-
expect(data).toBe("ERROR LOADER ERROR");
10150-
});
10151-
10152-
it("should handle submit error responses", async () => {
10153-
let { queryRoute } = createStaticHandler(SSR_ROUTES);
10154-
let data;
10155-
10156-
data = await queryRoute(createSubmitRequest("/parent/error"), "error");
10157-
expect(data).toBe("ERROR ACTION ERROR");
10158-
});
10159-
1016010417
it("should handle aborted load requests", async () => {
1016110418
let dfd = createDeferred();
1016210419
let controller = new AbortController();

‎packages/router/router.ts‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1783,10 +1783,27 @@ export function unstable_createStaticHandler(
17831783
return result;
17841784
}
17851785

1786+
let error = result.errors ? Object.values(result.errors)[0] : undefined;
1787+
if (error !== undefined) {
1788+
// While we always re-throw Responses returned from loaders/actions
1789+
// directly for route requests and prevent the unwrapping into an
1790+
// ErrorResponse, we still need this for error cases _prior_ the
1791+
// execution of the loader/action, such as a 404/405 error.
1792+
if (isRouteErrorResponse(error)) {
1793+
return new Response(error.data, {
1794+
status: error.status,
1795+
statusText: error.statusText,
1796+
});
1797+
}
1798+
// If we got back result.errors, that means the loader/action threw
1799+
// _something_ that wasn't a Response, but it's not guaranteed/required
1800+
// to be an `instanceof Error` either, so we have to use throw here to
1801+
// preserve the "error" state outside of queryImpl.
1802+
throw error;
1803+
}
1804+
17861805
// Pick off the right state value to return
1787-
let routeData = [result.errors, result.actionData, result.loaderData].find(
1788-
(v) => v
1789-
);
1806+
let routeData = [result.actionData, result.loaderData].find((v) => v);
17901807
let value = Object.values(routeData || {})[0];
17911808

17921809
if (isRouteErrorResponse(value)) {
@@ -2529,7 +2546,11 @@ function processRouteLoaderData(
25292546
loaderData[id] = result.data;
25302547
// Error status codes always override success status codes, but if all
25312548
// loaders are successful we take the deepest status code.
2532-
if (result.statusCode !== 200 && !foundError) {
2549+
if (
2550+
result.statusCode != null &&
2551+
result.statusCode !== 200 &&
2552+
!foundError
2553+
) {
25332554
statusCode = result.statusCode;
25342555
}
25352556
if (result.headers) {

0 commit comments

Comments
 (0)