Skip to content

Commit 5ed427e

Browse files
JohnAlbinJohnAlbin
authored
fix(next-drupal): throw non-404 errors from /router/translate-path responses
Co-authored-by: Marc Orcau <marc.orcau@digitalist.com> Fixes #687
1 parent 499d023 commit 5ed427e

File tree

6 files changed

+208
-50
lines changed

6 files changed

+208
-50
lines changed

‎packages/next-drupal/src/next-drupal.ts‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -372,23 +372,33 @@ export class NextDrupal extends NextDrupalBase {
372372
withAuth: options.withAuth,
373373
})
374374

375+
const errorMessagePrefix = "Error while fetching resource by path:"
376+
377+
if (response.status !== 207) {
378+
const errors = await this.getErrorsFromResponse(response)
379+
throw new JsonApiErrors(errors, response.status, errorMessagePrefix)
380+
}
381+
375382
const json = await response.json()
376383

377384
if (!json?.["resolvedResource#uri{0}"]?.body) {
378-
if (json?.router?.body) {
379-
const error = JSON.parse(json.router.body)
380-
if (error?.message) {
381-
this.logOrThrowError(new Error(error.message))
382-
}
385+
const status = json?.router?.headers?.status?.[0]
386+
if (status === 404) {
387+
return null
383388
}
384-
385-
return null
389+
const message =
390+
(json?.router?.body && JSON.parse(json.router.body)?.message) ||
391+
"Unknown error"
392+
throw new JsonApiErrors(message, status, errorMessagePrefix)
386393
}
387394

388395
const data = JSON.parse(json["resolvedResource#uri{0}"]?.body)
389396

390397
if (data.errors) {
391-
this.logOrThrowError(new Error(JsonApiErrors.formatMessage(data.errors)))
398+
const status = json?.["resolvedResource#uri{0}"]?.headers?.status?.[0]
399+
this.logOrThrowError(
400+
new JsonApiErrors(data.errors, status, errorMessagePrefix)
401+
)
392402
}
393403

394404
return options.deserialize ? this.deserialize(data) : data
@@ -554,12 +564,14 @@ export class NextDrupal extends NextDrupalBase {
554564
withAuth: options.withAuth,
555565
})
556566

557-
if (!response?.ok) {
567+
if (response.status === 404) {
558568
// Do not throw errors here, otherwise Next.js will catch the error and
559569
// throw a 500. We want a 404.
560570
return null
561571
}
562572

573+
await this.throwIfJsonErrors(response)
574+
563575
return await response.json()
564576
}
565577

‎packages/next-drupal/tests/NextDrupal/resource-methods.test.ts‎

Lines changed: 105 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -496,15 +496,16 @@ describe("getResourceByPath()", () => {
496496
}
497497
)
498498
).rejects.toThrow(
499-
"404 Not Found\nThe requested version, identified by `id:-11`, could not be found."
499+
"Error while fetching resource by path: 404 Not Found\nThe requested version, identified by `id:-11`, could not be found."
500500
)
501501
})
502502

503503
test("throws an error if revision access is forbidden", async () => {
504504
const drupal = new NextDrupal(BASE_URL)
505505
spyOnFetch({
506-
responseBody: mocks.resources.subRequests.forbidden,
507506
status: 207,
507+
statusText: "Multi-Status",
508+
responseBody: mocks.resources.subRequests.forbidden,
508509
})
509510

510511
await expect(
@@ -518,19 +519,87 @@ describe("getResourceByPath()", () => {
518519
}
519520
)
520521
).rejects.toThrow(
521-
"403 Forbidden\nThe current user is not allowed to GET the selected resource. The user does not have access to the requested version."
522+
"Error while fetching resource by path: 403 Forbidden\nThe current user is not allowed to GET the selected resource. The user does not have access to the requested version."
522523
)
523524
})
524525

525526
test("returns null for path not found", async () => {
526527
const drupal = new NextDrupal(BASE_URL)
527528

529+
spyOnFetch({
530+
status: 207,
531+
statusText: "Multi-Status",
532+
responseBody: mocks.resources.subRequests.pathNotFound,
533+
})
534+
535+
const path = await drupal.getResourceByPath<DrupalNode>(
536+
"/path-does-not-exist"
537+
)
538+
539+
expect(path).toBeNull()
540+
})
541+
542+
test("throws an error on server error", async () => {
543+
const drupal = new NextDrupal(BASE_URL)
544+
545+
spyOnFetch({
546+
status: 500,
547+
statusText: "Internal server error",
548+
responseBody: "500 Internal server error",
549+
})
550+
528551
await expect(
529-
drupal.getResourceByPath<DrupalNode>("/path-do-not-exist")
530-
).rejects.toThrow("Unable to resolve path /path-do-not-exist.")
552+
drupal.getResourceByPath<DrupalNode>("/server-error")
553+
).rejects.toThrow(
554+
"Error while fetching resource by path: Internal server error"
555+
)
531556
})
532557

533-
test("throws an error for invalid params", async () => {
558+
test("throws an error on router error", async () => {
559+
const drupal = new NextDrupal(BASE_URL)
560+
561+
spyOnFetch({
562+
status: 207,
563+
statusText: "Multi-Status",
564+
responseBody: {
565+
router: {
566+
body: JSON.stringify({ message: "Forbidden" }),
567+
headers: {
568+
...mocks.resources.subRequests.pathNotFound.router.headers,
569+
status: [403],
570+
},
571+
},
572+
},
573+
})
574+
575+
await expect(
576+
drupal.getResourceByPath<DrupalNode>("/router-error")
577+
).rejects.toThrow("Error while fetching resource by path: Forbidden")
578+
})
579+
580+
test("throws an error on unknown router error", async () => {
581+
const drupal = new NextDrupal(BASE_URL)
582+
583+
spyOnFetch({
584+
status: 207,
585+
statusText: "Multi-Status",
586+
responseBody: {
587+
router: {
588+
body: JSON.stringify({}),
589+
headers: {
590+
...mocks.resources.subRequests.pathNotFound.router.headers,
591+
status: [403],
592+
},
593+
},
594+
},
595+
})
596+
597+
await expect(
598+
drupal.getResourceByPath<DrupalNode>("/router-error")
599+
).rejects.toThrow("Error while fetching resource by path: Unknown error")
600+
})
601+
602+
test("throws an error on resource error", async () => {
534603
const drupal = new NextDrupal(BASE_URL)
535604

536605
await expect(
@@ -543,18 +612,23 @@ describe("getResourceByPath()", () => {
543612
}
544613
)
545614
).rejects.toThrow(
546-
"400 Bad Request\n`invalid_relationship` is not a valid relationship field name. Possible values: node_type, revision_uid, uid, menu_link, field_media_image, field_recipe_category, field_tags."
615+
"Error while fetching resource by path: 400 Bad Request\n`invalid_relationship` is not a valid relationship field name. Possible values: node_type, revision_uid, uid, menu_link, field_media_image, field_recipe_category, field_tags."
547616
)
548617
})
549618

550619
test("makes un-authenticated requests by default", async () => {
551620
const drupal = new NextDrupal(BASE_URL)
552-
const drupalFetchSpy = spyOnDrupalFetch(drupal)
621+
const drupalFetchSpy = spyOnDrupalFetch(drupal, {
622+
status: 207,
623+
statusText: "Multi-Status",
624+
responseBody: mocks.resources.subRequests.ok,
625+
})
553626
const getAccessTokenSpy = jest.spyOn(drupal, "getAccessToken")
554627

555628
await drupal.getResourceByPath<DrupalNode>(
556629
"/recipes/deep-mediterranean-quiche"
557630
)
631+
558632
expect(drupalFetchSpy).toHaveBeenCalledWith(
559633
expect.anything(),
560634
expect.not.objectContaining({
@@ -573,8 +647,9 @@ describe("getResourceByPath()", () => {
573647
logger: mockLogger(),
574648
})
575649
const fetchSpy = spyOnFetch({
576-
responseBody: { "resolvedResource#uri{0}": { body: "{}" } },
577650
status: 207,
651+
statusText: "Multi-Status",
652+
responseBody: { "resolvedResource#uri{0}": { body: "{}" } },
578653
})
579654
const getAccessTokenSpy = jest
580655
.spyOn(drupal, "getAccessToken")
@@ -1012,11 +1087,31 @@ describe("translatePath()", () => {
10121087
test("returns null for path not found", async () => {
10131088
const drupal = new NextDrupal(BASE_URL)
10141089

1015-
const path = await drupal.translatePath("/path-not-found")
1090+
spyOnFetch({
1091+
status: 404,
1092+
statusText: "Not found",
1093+
responseBody: mocks.resources.translatePath.notFound,
1094+
})
1095+
1096+
const path = await drupal.translatePath("/path-does-not-exist")
10161097

10171098
expect(path).toBeNull()
10181099
})
10191100

1101+
test("throws an error on server error", async () => {
1102+
const drupal = new NextDrupal(BASE_URL)
1103+
1104+
spyOnFetch({
1105+
status: 500,
1106+
statusText: "Internal server error",
1107+
responseBody: "500 Internal server error",
1108+
})
1109+
1110+
await expect(drupal.translatePath("/server-error")).rejects.toThrowError(
1111+
"Internal server error"
1112+
)
1113+
})
1114+
10201115
test("makes un-authenticated requests by default", async () => {
10211116
const drupal = new NextDrupal(BASE_URL)
10221117
const drupalFetchSpy = spyOnDrupalFetch(drupal)

‎packages/next-drupal/tests/NextDrupalBase/basic-methods.test.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,18 @@ describe("getErrorsFromResponse()", () => {
274274
expect(await drupal.getErrorsFromResponse(response)).toBe(message)
275275
})
276276

277+
test("returns the response status text if the application/json errors cannot be found", async () => {
278+
const response = new Response(JSON.stringify({}), {
279+
status: 403,
280+
statusText: "Forbidden",
281+
headers: {
282+
"content-type": "application/json",
283+
},
284+
})
285+
286+
expect(await drupal.getErrorsFromResponse(response)).toBe("Forbidden")
287+
})
288+
277289
test("returns application/vnd.api+json errors", async () => {
278290
const payload = {
279291
errors: [
@@ -317,7 +329,7 @@ describe("getErrorsFromResponse()", () => {
317329
})
318330

319331
test("returns the response status text if no errors can be found", async () => {
320-
const response = new Response(JSON.stringify({}), {
332+
const response = new Response("403 Forbidden", {
321333
status: 403,
322334
statusText: "Forbidden",
323335
})

‎packages/next-drupal/tests/NextDrupalPages/pages-router-methods.test.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -891,7 +891,10 @@ describe("getResourceFromContext()", () => {
891891

892892
test("makes un-authenticated requests by default", async () => {
893893
const drupal = new NextDrupalPages(BASE_URL)
894-
const drupalFetchSpy = spyOnDrupalFetch(drupal)
894+
const drupalFetchSpy = spyOnDrupalFetch(drupal, {
895+
responseBody: mocks.resources.subRequests.ok,
896+
status: 207,
897+
})
895898
const context: GetStaticPropsContext = {
896899
params: {
897900
slug: ["recipes", "deep-mediterranean-quiche"],

‎packages/next-drupal/tests/utils/mocks/data.ts‎

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -461,25 +461,32 @@ const resources = {
461461
},
462462
},
463463
translatePath: {
464-
jsonapi: {
465-
basePath: "/en/jsonapi",
466-
entryPoint: "https://example.com/en/jsonapi",
467-
individual:
468-
"https://example.com/en/jsonapi/node/recipe/71e04ead-4cc7-416c-b9ca-60b635fdc50f",
469-
resourceName: "node--recipe",
464+
ok: {
465+
jsonapi: {
466+
basePath: "/en/jsonapi",
467+
entryPoint: "https://example.com/en/jsonapi",
468+
individual:
469+
"https://example.com/en/jsonapi/node/recipe/71e04ead-4cc7-416c-b9ca-60b635fdc50f",
470+
resourceName: "node--recipe",
471+
},
472+
entity: {
473+
bundle: "recipe",
474+
canonical: "https://example.com/en/recipes/deep-mediterranean-quiche",
475+
id: "1",
476+
langcode: "en",
477+
path: "/en/recipes/deep-mediterranean-quiche",
478+
type: "node",
479+
uuid: "71e04ead-4cc7-416c-b9ca-60b635fdc50f",
480+
},
481+
isHomePath: false,
482+
label: "Deep mediterranean quiche - edited",
483+
resolved: "https://example.com/en/recipes/deep-mediterranean-quiche",
470484
},
471-
entity: {
472-
bundle: "recipe",
473-
canonical: "https://example.com/en/recipes/deep-mediterranean-quiche",
474-
id: "1",
475-
langcode: "en",
476-
path: "/en/recipes/deep-mediterranean-quiche",
477-
type: "node",
478-
uuid: "71e04ead-4cc7-416c-b9ca-60b635fdc50f",
485+
notFound: {
486+
message: "Unable to resolve path /path-does-not-exist.",
487+
details:
488+
"None of the available methods were able to find a match for this path.",
479489
},
480-
isHomePath: false,
481-
label: "Deep mediterranean quiche - edited",
482-
resolved: "https://example.com/en/recipes/deep-mediterranean-quiche",
483490
},
484491
subRequests: {
485492
ok: {
@@ -532,11 +539,7 @@ const resources = {
532539
},
533540
pathNotFound: {
534541
router: {
535-
body: JSON.stringify({
536-
message: "Unable to resolve path /path-does-not-exist.",
537-
details:
538-
"None of the available methods were able to find a match for this path.",
539-
}),
542+
body: "SEE REPLACEMENT BELOW",
540543
headers: {
541544
"cache-control": ["no-cache, private"],
542545
"content-id": ["<router>"],
@@ -550,15 +553,20 @@ const resources = {
550553
},
551554
}
552555
// JSON-encode the subrequest body fields.
553-
resources.subRequests.ok.router.body = JSON.stringify(resources.translatePath)
556+
resources.subRequests.ok.router.body = JSON.stringify(
557+
resources.translatePath.ok
558+
)
554559
resources.subRequests.ok["resolvedResource#uri{0}"].body = JSON.stringify(
555560
resources.node.ok
556561
)
557562
resources.subRequests.forbidden.router.body = JSON.stringify(
558-
resources.translatePath
563+
resources.translatePath.ok
559564
)
560565
resources.subRequests.forbidden["resolvedResource#uri{0}"].body =
561566
JSON.stringify(resources.node.forbidden)
567+
resources.subRequests.pathNotFound.router.body = JSON.stringify(
568+
resources.translatePath.notFound
569+
)
562570

563571
const menus = {
564572
menuItems: {

0 commit comments

Comments
 (0)