Skip to content

Commit

Permalink
feat(next-drupal): translatePath, getResourceByPath - throw error whe…
Browse files Browse the repository at this point in the history
…n backend error

Until now, any not-ok response from decoupled router was treated equally, as a 404 Not Found.
Behaviour is changed differentiating between 404 Not Found and server error.
The advantage is frontend holds to previous built page and doesn't update public page content
with a 404 due to backend issues.

BREAKING CHANGE:
Might break sites relying on response being the same in 404 than in 50x.

Fixes #687
  • Loading branch information
marcorcau committed Apr 14, 2024
1 parent 4bce663 commit c5c9e80
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 4 deletions.
32 changes: 28 additions & 4 deletions packages/next-drupal/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,19 @@ export class DrupalClient {
withAuth: options.withAuth,
})

// Server error. But 404 is treated implicitely below.
if (!response?.ok && response.status !== 404) {
let errorMessage: string
try {
const responseJson = await response.json()
errorMessage = `${response.status} ${responseJson?.message}`
} catch (e) {
/* c8 ignore next 2 */
errorMessage = `${response.status} ${response.statusText}`
}
throw new Error(errorMessage)
}

const json = await response.json()

if (!json?.["resolvedResource#uri{0}"]?.body) {
Expand Down Expand Up @@ -890,10 +903,21 @@ export class DrupalClient {
})

if (!response?.ok) {
// Do not throw errors here.
// Otherwise next.js will catch error and throw a 500.
// We want a 404.
return null
// Do not throw errors here when response is 404.
if (response.status === 404) {
return null
}

// Throw error in any other situation as response is not ok.
let errorMessage: string
try {
const responseJson = await response.json()
errorMessage = `${response.status} ${responseJson?.message}`
} catch (e) {
/* c8 ignore next 2 */
errorMessage = `${response.status} ${response.statusText}`
}
throw new Error(errorMessage)
}

const json = await response.json()
Expand Down
26 changes: 26 additions & 0 deletions packages/next-drupal/tests/DrupalClient/resource-methods.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,19 @@ describe("getResourceByPath()", () => {
).rejects.toThrow("Unable to resolve path /path-do-not-exist.")
})

test("throws an error for server errors", async () => {
const client = new DrupalClient(BASE_URL)

spyOnFetch({
responseBody: { message: "mocked internal server error" },
status: 500,
})

await expect(
client.getResourceByPath<DrupalNode>("/server-error")
).rejects.toThrow("500 mocked internal server error")
})

test("throws an error for invalid params", async () => {
const client = new DrupalClient(BASE_URL)

Expand Down Expand Up @@ -836,6 +849,19 @@ describe("translatePath()", () => {
expect(path).toBeNull()
})

test("throws an error for server errors", async () => {
const client = new DrupalClient(BASE_URL)

spyOnFetch({
responseBody: { message: "mocked internal server error" },
status: 500,
})

await expect(client.translatePath("/server-error")).rejects.toThrowError(
"500 mocked internal server error"
)
})

test("makes un-authenticated requests by default", async () => {
const client = new DrupalClient(BASE_URL)
const fetchSpy = jest.spyOn(client, "fetch")
Expand Down

0 comments on commit c5c9e80

Please sign in to comment.