throw non-404 errors from /router/translate-path responses

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

@@ -372,23 +372,33 @@ export class NextDrupal extends NextDrupalBase {
       withAuth: options.withAuth,
     })
 
+    const errorMessagePrefix = "Error while fetching resource by path:"
+
+    if (response.status !== 207) {
+      const errors = await this.getErrorsFromResponse(response)
+      throw new JsonApiErrors(errors, response.status, errorMessagePrefix)
+    }
+
     const json = await response.json()
 
     if (!json?.["resolvedResource#uri{0}"]?.body) {
-      if (json?.router?.body) {
-        const error = JSON.parse(json.router.body)
-        if (error?.message) {
-          this.logOrThrowError(new Error(error.message))
-        }
+      const status = json?.router?.headers?.status?.[0]
+      if (status === 404) {
+        return null
       }
-
-      return null
+      const message =
+        (json?.router?.body && JSON.parse(json.router.body)?.message) ||
+        "Unknown error"
+      throw new JsonApiErrors(message, status, errorMessagePrefix)
     }
 
     const data = JSON.parse(json["resolvedResource#uri{0}"]?.body)
 
     if (data.errors) {
-      this.logOrThrowError(new Error(JsonApiErrors.formatMessage(data.errors)))
+      const status = json?.["resolvedResource#uri{0}"]?.headers?.status?.[0]
+      this.logOrThrowError(
+        new JsonApiErrors(data.errors, status, errorMessagePrefix)
+      )
     }
 
     return options.deserialize ? this.deserialize(data) : data
@@ -554,12 +564,14 @@ export class NextDrupal extends NextDrupalBase {
       withAuth: options.withAuth,
     })
 
-    if (!response?.ok) {
+    if (response.status === 404) {
       // Do not throw errors here, otherwise Next.js will catch the error and
       // throw a 500. We want a 404.
       return null
     }
 
+    await this.throwIfJsonErrors(response)
+
     return await response.json()
   }
 

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

@@ -496,15 +496,16 @@ describe("getResourceByPath()", () => {
         }
       )
     ).rejects.toThrow(
-      "404 Not Found\nThe requested version, identified by `id:-11`, could not be found."
+      "Error while fetching resource by path: 404 Not Found\nThe requested version, identified by `id:-11`, could not be found."
     )
   })
 
   test("throws an error if revision access is forbidden", async () => {
     const drupal = new NextDrupal(BASE_URL)
     spyOnFetch({
-      responseBody: mocks.resources.subRequests.forbidden,
       status: 207,
+      statusText: "Multi-Status",
+      responseBody: mocks.resources.subRequests.forbidden,
     })
 
     await expect(
@@ -518,19 +519,87 @@ describe("getResourceByPath()", () => {
         }
       )
     ).rejects.toThrow(
-      "403 Forbidden\nThe current user is not allowed to GET the selected resource. The user does not have access to the requested version."
+      "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."
     )
   })
 
   test("returns null for path not found", async () => {
     const drupal = new NextDrupal(BASE_URL)
 
+    spyOnFetch({
+      status: 207,
+      statusText: "Multi-Status",
+      responseBody: mocks.resources.subRequests.pathNotFound,
+    })
+
+    const path = await drupal.getResourceByPath<DrupalNode>(
+      "/path-does-not-exist"
+    )
+
+    expect(path).toBeNull()
+  })
+
+  test("throws an error on server error", async () => {
+    const drupal = new NextDrupal(BASE_URL)
+
+    spyOnFetch({
+      status: 500,
+      statusText: "Internal server error",
+      responseBody: "500 Internal server error",
+    })
+
     await expect(
-      drupal.getResourceByPath<DrupalNode>("/path-do-not-exist")
-    ).rejects.toThrow("Unable to resolve path /path-do-not-exist.")
+      drupal.getResourceByPath<DrupalNode>("/server-error")
+    ).rejects.toThrow(
+      "Error while fetching resource by path: Internal server error"
+    )
   })
 
-  test("throws an error for invalid params", async () => {
+  test("throws an error on router error", async () => {
+    const drupal = new NextDrupal(BASE_URL)
+
+    spyOnFetch({
+      status: 207,
+      statusText: "Multi-Status",
+      responseBody: {
+        router: {
+          body: JSON.stringify({ message: "Forbidden" }),
+          headers: {
+            ...mocks.resources.subRequests.pathNotFound.router.headers,
+            status: [403],
+          },
+        },
+      },
+    })
+
+    await expect(
+      drupal.getResourceByPath<DrupalNode>("/router-error")
+    ).rejects.toThrow("Error while fetching resource by path: Forbidden")
+  })
+
+  test("throws an error on unknown router error", async () => {
+    const drupal = new NextDrupal(BASE_URL)
+
+    spyOnFetch({
+      status: 207,
+      statusText: "Multi-Status",
+      responseBody: {
+        router: {
+          body: JSON.stringify({}),
+          headers: {
+            ...mocks.resources.subRequests.pathNotFound.router.headers,
+            status: [403],
+          },
+        },
+      },
+    })
+
+    await expect(
+      drupal.getResourceByPath<DrupalNode>("/router-error")
+    ).rejects.toThrow("Error while fetching resource by path: Unknown error")
+  })
+
+  test("throws an error on resource error", async () => {
     const drupal = new NextDrupal(BASE_URL)
 
     await expect(
@@ -543,18 +612,23 @@ describe("getResourceByPath()", () => {
         }
       )
     ).rejects.toThrow(
-      "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."
+      "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."
     )
   })
 
   test("makes un-authenticated requests by default", async () => {
     const drupal = new NextDrupal(BASE_URL)
-    const drupalFetchSpy = spyOnDrupalFetch(drupal)
+    const drupalFetchSpy = spyOnDrupalFetch(drupal, {
+      status: 207,
+      statusText: "Multi-Status",
+      responseBody: mocks.resources.subRequests.ok,
+    })
     const getAccessTokenSpy = jest.spyOn(drupal, "getAccessToken")
 
     await drupal.getResourceByPath<DrupalNode>(
       "/recipes/deep-mediterranean-quiche"
     )
+
     expect(drupalFetchSpy).toHaveBeenCalledWith(
       expect.anything(),
       expect.not.objectContaining({
@@ -573,8 +647,9 @@ describe("getResourceByPath()", () => {
       logger: mockLogger(),
     })
     const fetchSpy = spyOnFetch({
-      responseBody: { "resolvedResource#uri{0}": { body: "{}" } },
       status: 207,
+      statusText: "Multi-Status",
+      responseBody: { "resolvedResource#uri{0}": { body: "{}" } },
     })
     const getAccessTokenSpy = jest
       .spyOn(drupal, "getAccessToken")
@@ -1012,11 +1087,31 @@ describe("translatePath()", () => {
   test("returns null for path not found", async () => {
     const drupal = new NextDrupal(BASE_URL)
 
-    const path = await drupal.translatePath("/path-not-found")
+    spyOnFetch({
+      status: 404,
+      statusText: "Not found",
+      responseBody: mocks.resources.translatePath.notFound,
+    })
+
+    const path = await drupal.translatePath("/path-does-not-exist")
 
     expect(path).toBeNull()
   })
 
+  test("throws an error on server error", async () => {
+    const drupal = new NextDrupal(BASE_URL)
+
+    spyOnFetch({
+      status: 500,
+      statusText: "Internal server error",
+      responseBody: "500 Internal server error",
+    })
+
+    await expect(drupal.translatePath("/server-error")).rejects.toThrowError(
+      "Internal server error"
+    )
+  })
+
   test("makes un-authenticated requests by default", async () => {
     const drupal = new NextDrupal(BASE_URL)
     const drupalFetchSpy = spyOnDrupalFetch(drupal)

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

@@ -274,6 +274,18 @@ describe("getErrorsFromResponse()", () => {
     expect(await drupal.getErrorsFromResponse(response)).toBe(message)
   })
 
+  test("returns the response status text if the application/json errors cannot be found", async () => {
+    const response = new Response(JSON.stringify({}), {
+      status: 403,
+      statusText: "Forbidden",
+      headers: {
+        "content-type": "application/json",
+      },
+    })
+
+    expect(await drupal.getErrorsFromResponse(response)).toBe("Forbidden")
+  })
+
   test("returns application/vnd.api+json errors", async () => {
     const payload = {
       errors: [
@@ -317,7 +329,7 @@ describe("getErrorsFromResponse()", () => {
   })
 
   test("returns the response status text if no errors can be found", async () => {
-    const response = new Response(JSON.stringify({}), {
+    const response = new Response("403 Forbidden", {
       status: 403,
       statusText: "Forbidden",
     })

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

@@ -891,7 +891,10 @@ describe("getResourceFromContext()", () => {
 
   test("makes un-authenticated requests by default", async () => {
     const drupal = new NextDrupalPages(BASE_URL)
-    const drupalFetchSpy = spyOnDrupalFetch(drupal)
+    const drupalFetchSpy = spyOnDrupalFetch(drupal, {
+      responseBody: mocks.resources.subRequests.ok,
+      status: 207,
+    })
     const context: GetStaticPropsContext = {
       params: {
         slug: ["recipes", "deep-mediterranean-quiche"],

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

@@ -461,25 +461,32 @@ const resources = {
     },
   },
   translatePath: {
-    jsonapi: {
-      basePath: "/en/jsonapi",
-      entryPoint: "https://example.com/en/jsonapi",
-      individual:
-        "https://example.com/en/jsonapi/node/recipe/71e04ead-4cc7-416c-b9ca-60b635fdc50f",
-      resourceName: "node--recipe",
+    ok: {
+      jsonapi: {
+        basePath: "/en/jsonapi",
+        entryPoint: "https://example.com/en/jsonapi",
+        individual:
+          "https://example.com/en/jsonapi/node/recipe/71e04ead-4cc7-416c-b9ca-60b635fdc50f",
+        resourceName: "node--recipe",
+      },
+      entity: {
+        bundle: "recipe",
+        canonical: "https://example.com/en/recipes/deep-mediterranean-quiche",
+        id: "1",
+        langcode: "en",
+        path: "/en/recipes/deep-mediterranean-quiche",
+        type: "node",
+        uuid: "71e04ead-4cc7-416c-b9ca-60b635fdc50f",
+      },
+      isHomePath: false,
+      label: "Deep mediterranean quiche - edited",
+      resolved: "https://example.com/en/recipes/deep-mediterranean-quiche",
     },
-    entity: {
-      bundle: "recipe",
-      canonical: "https://example.com/en/recipes/deep-mediterranean-quiche",
-      id: "1",
-      langcode: "en",
-      path: "/en/recipes/deep-mediterranean-quiche",
-      type: "node",
-      uuid: "71e04ead-4cc7-416c-b9ca-60b635fdc50f",
+    notFound: {
+      message: "Unable to resolve path /path-does-not-exist.",
+      details:
+        "None of the available methods were able to find a match for this path.",
     },
-    isHomePath: false,
-    label: "Deep mediterranean quiche - edited",
-    resolved: "https://example.com/en/recipes/deep-mediterranean-quiche",
   },
   subRequests: {
     ok: {
@@ -532,11 +539,7 @@ const resources = {
     },
     pathNotFound: {
       router: {
-        body: JSON.stringify({
-          message: "Unable to resolve path /path-does-not-exist.",
-          details:
-            "None of the available methods were able to find a match for this path.",
-        }),
+        body: "SEE REPLACEMENT BELOW",
         headers: {
           "cache-control": ["no-cache, private"],
           "content-id": ["<router>"],
@@ -550,15 +553,20 @@ const resources = {
   },
 }
 // JSON-encode the subrequest body fields.
-resources.subRequests.ok.router.body = JSON.stringify(resources.translatePath)
+resources.subRequests.ok.router.body = JSON.stringify(
+  resources.translatePath.ok
+)
 resources.subRequests.ok["resolvedResource#uri{0}"].body = JSON.stringify(
   resources.node.ok
 )
 resources.subRequests.forbidden.router.body = JSON.stringify(
-  resources.translatePath
+  resources.translatePath.ok
 )
 resources.subRequests.forbidden["resolvedResource#uri{0}"].body =
   JSON.stringify(resources.node.forbidden)
+resources.subRequests.pathNotFound.router.body = JSON.stringify(
+  resources.translatePath.notFound
+)
 
 const menus = {
   menuItems: {