Skip to content

Don't skip afterFetch in cases where there's no response body. #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 29 additions & 18 deletions src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,17 +118,20 @@ export class Request {
response: Response,
requestOptions: RequestInit
) {
let wasDelete =
requestOptions.method === "DELETE" &&
[204, 200].indexOf(response.status) > -1
if (wasDelete) return

let json
try {
json = await response.clone().json()
} catch (e) {
if (response.status === 202) return
throw new ResponseError(response, "invalid json", e)
const possiblyEmptyResponseStatuses = [202, 204]
const possiblyEmpty =
possiblyEmptyResponseStatuses.indexOf(response.status) > -1

let json = null

if (204 !== response.status) {
try {
json = await response.clone().json()
} catch (e) {
if (!possiblyEmpty) {
throw new ResponseError(response, "invalid json", e)
}
}
}

try {
Expand All @@ -144,13 +147,21 @@ export class Request {

if (response.status >= 500) {
throw new ResponseError(response, "Server Error")
// Allow 422 since we specially handle validation errors
} else if (response.status !== 422 && json.data === undefined) {
if (response.status === 404) {
throw new ResponseError(response, "record not found")
} else {
// Bad JSON, for instance an errors payload
throw new ResponseError(response, "invalid json")
}

// Allow 422 since we specially handle validation errors
if (response.status !== 422) {
if (
(json === null ||
(json.data === undefined && json.meta === undefined)) &&
!possiblyEmpty
) {
if (response.status === 404) {
throw new ResponseError(response, "record not found")
} else {
// Bad JSON, for instance an errors payload
throw new ResponseError(response, "invalid json")
}
}
}

Expand Down
55 changes: 55 additions & 0 deletions test/integration/fetch-middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,15 @@ const mock500 = () => {
})
}

const mock204 = () => {
fetchMock.restore()

fetchMock.mock({
matcher: "*",
response: { status: 204 }
})
}

const mockSuccess = () => {
fetchMock.restore()

Expand Down Expand Up @@ -477,4 +486,50 @@ describe("fetch middleware", () => {
})
})
})

describe("destroys", () => {
describe("on successful response", () => {
it("correctly resolves the promise", async () => {
const author = new Author()
await author.save()

const success = await author.destroy()
expect(success).to.eq(true)
})

it("runs beforeEach hooks", async () => {
const author = new Author()
await author.save()
before = {}
expect(before.url).to.eq(undefined)

const success = await author.destroy()
expect(success).to.eq(true)
expect(before.url).to.eq("http://example.com/api/v1/authors")
expect(before.options).to.deep.eq({
credentials: "same-origin",
headers: {
Accept: "application/vnd.api+json",
"CUSTOM-HEADER": "whatever",
"Content-Type": "application/vnd.api+json"
},
method: "DELETE"
})
})

it("runs afterEach hooks", async () => {
const author = new Author()
await author.save()
after = {}
expect(after.response).to.eq(undefined)

mock204()
const success = await author.destroy()
expect(success).to.eq(true)

expect(after.response).to.not.be.undefined
expect(after.response.status).to.eq(204)
})
})
})
})
2 changes: 1 addition & 1 deletion test/integration/persistence.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ describe("Model persistence", () => {

fetchMock.mock({
matcher: "http://example.com/api/v1/people/1",
response: new Response({ status: 204 } as any)
response: { status: 204 }
})
})

Expand Down