diff --git a/packages/next-drupal/src/next-drupal.ts b/packages/next-drupal/src/next-drupal.ts index 904f5703..564e25eb 100644 --- a/packages/next-drupal/src/next-drupal.ts +++ b/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() } diff --git a/packages/next-drupal/tests/NextDrupal/resource-methods.test.ts b/packages/next-drupal/tests/NextDrupal/resource-methods.test.ts index e790aa25..8e7e23fa 100644 --- a/packages/next-drupal/tests/NextDrupal/resource-methods.test.ts +++ b/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( + "/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("/path-do-not-exist") - ).rejects.toThrow("Unable to resolve path /path-do-not-exist.") + drupal.getResourceByPath("/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("/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("/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( "/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) diff --git a/packages/next-drupal/tests/NextDrupalBase/basic-methods.test.ts b/packages/next-drupal/tests/NextDrupalBase/basic-methods.test.ts index 699d146c..97420879 100644 --- a/packages/next-drupal/tests/NextDrupalBase/basic-methods.test.ts +++ b/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", }) diff --git a/packages/next-drupal/tests/NextDrupalPages/pages-router-methods.test.ts b/packages/next-drupal/tests/NextDrupalPages/pages-router-methods.test.ts index 456ee032..242ca6db 100644 --- a/packages/next-drupal/tests/NextDrupalPages/pages-router-methods.test.ts +++ b/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"], diff --git a/packages/next-drupal/tests/utils/mocks/data.ts b/packages/next-drupal/tests/utils/mocks/data.ts index c6be6fa6..944e62f5 100644 --- a/packages/next-drupal/tests/utils/mocks/data.ts +++ b/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": [""], @@ -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: { diff --git a/packages/next-drupal/tests/utils/mocks/fetch.ts b/packages/next-drupal/tests/utils/mocks/fetch.ts index 819deceb..c58e8658 100644 --- a/packages/next-drupal/tests/utils/mocks/fetch.ts +++ b/packages/next-drupal/tests/utils/mocks/fetch.ts @@ -6,6 +6,7 @@ interface SpyOnFetchParams { responseBody?: any throwErrorMessage?: string status?: number + statusText?: string headers?: Record } @@ -13,6 +14,7 @@ export function spyOnFetch({ responseBody = null, throwErrorMessage = null, status = 200, + statusText = "", headers = {}, }: SpyOnFetchParams = {}) { return jest.spyOn(global, "fetch").mockImplementation( @@ -20,6 +22,7 @@ export function spyOnFetch({ responseBody, throwErrorMessage, status, + statusText, headers, }) ) @@ -29,6 +32,7 @@ export function spyOnFetchOnce({ responseBody = null, throwErrorMessage = null, status = 200, + statusText = "", headers = {}, }: SpyOnFetchParams) { return jest.spyOn(global, "fetch").mockImplementationOnce( @@ -36,6 +40,7 @@ export function spyOnFetchOnce({ responseBody, throwErrorMessage, status, + statusText, headers, }) ) @@ -47,6 +52,7 @@ export function spyOnDrupalFetch( responseBody = null, throwErrorMessage = null, status = 200, + statusText = "", headers = {}, }: SpyOnFetchParams = {} ) { @@ -55,6 +61,7 @@ export function spyOnDrupalFetch( responseBody, throwErrorMessage, status, + statusText, headers, }) ) @@ -64,6 +71,7 @@ function fetchMockImplementation({ responseBody = null, throwErrorMessage = null, status = 200, + statusText = "", headers = {}, }: SpyOnFetchParams) { if (throwErrorMessage) { @@ -72,14 +80,34 @@ function fetchMockImplementation({ } } + const hasTextResponse = typeof responseBody === "string" + const mockedHeaders = new Headers(headers) if (!mockedHeaders.has("content-type")) { - mockedHeaders.set("content-type", "application/vnd.api+json") + let contentType: string + if (hasTextResponse) { + contentType = "text/plain" + } else if ( + responseBody?.data || + responseBody?.errors || + responseBody?.meta || + responseBody?.jsonapi + ) { + contentType = "application/vnd.api+json" + } else { + contentType = "application/json" + } + + mockedHeaders.set("Content-type", contentType) } return async () => - new Response(JSON.stringify(responseBody || {}), { - status, - headers: mockedHeaders, - }) + new Response( + hasTextResponse ? responseBody : JSON.stringify(responseBody || {}), + { + status, + statusText, + headers: mockedHeaders, + } + ) }