Skip to content

Commit

Permalink
fix(fetcher): onNotOkResponse to still attach res.body
Browse files Browse the repository at this point in the history
Before:
if error - fetcherResponse.body is undefined

After:
fetcherResponse.body is provided, together with res.err and !res.ok
  • Loading branch information
kirillgroshkov committed Sep 10, 2024
1 parent 6512fb3 commit 40ad04e
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 14 deletions.
23 changes: 21 additions & 2 deletions src/error/assert.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
import type { ErrorData, ErrorObject } from '..'
import { _deepEquals, _isErrorObject, _stringify, AssertionError, Class } from '..'
import {
_deepEquals,
_isBackendErrorResponseObject,
_isErrorObject,
_stringify,
AssertionError,
BackendErrorResponseObject,
Class,
ErrorData,
ErrorObject,
} from '..'

/**
* Evaluates the `condition` (casts it to Boolean).
Expand Down Expand Up @@ -113,6 +122,16 @@ export function _assertIsErrorObject<DATA_TYPE extends ErrorData = ErrorData>(
}
}

export function _assertIsBackendErrorResponseObject<DATA_TYPE extends ErrorData = ErrorData>(
obj: any,
): asserts obj is BackendErrorResponseObject<DATA_TYPE> {
if (!_isBackendErrorResponseObject(obj)) {
throw new AssertionError(
`Expected to be BackendErrorResponseObject, actual typeof: ${typeof obj}`,
)
}
}

export function _assertIsString(v: any, message?: string): asserts v is string {
_assertTypeOf<string>(v, 'string', message)
}
Expand Down
2 changes: 1 addition & 1 deletion src/http/fetcher.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ test('redirect: manual', async () => {
`)
expect(r.fetchResponse!.status).toBe(301)
expect(r.fetchResponse!.headers.get('location')).toBe('https://naturalcycles.com/')
expect(r.body).toBeUndefined()
expect(r.body).toBe('')
})

test('timeout', async () => {
Expand Down
11 changes: 10 additions & 1 deletion src/http/fetcher.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { expectTypeOf } from 'expect-type'
import {
_assertIsBackendErrorResponseObject,
AppError,
ErrorObject,
HttpRequestError,
Expand Down Expand Up @@ -137,7 +138,15 @@ test('mocking fetch', async () => {
)
})

const { err } = await fetcher.doFetch({ url: 'some' })
const { err, body } = await fetcher.doFetch({ url: 'some' })
expect(body).toBeDefined()
_assertIsBackendErrorResponseObject(body)
expect(_stringify(body.error, { includeErrorData: true })).toMatchInlineSnapshot(`
"AppError: aya-baya
{
"some": "key"
}"
`)

_assertIsError(err, HttpRequestError)

Expand Down
27 changes: 17 additions & 10 deletions src/http/fetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -416,24 +416,31 @@ export class Fetcher {
private async onNotOkResponse(res: FetcherResponse): Promise<void> {
let cause: ErrorObject

// Try to fetch body and attach to res.body
// (but don't fail if it doesn't work)
if (!res.body && res.fetchResponse) {
try {
res.body = _jsonParseIfPossible(await res.fetchResponse.text())
} catch {
// ignore body fetching/parsing errors at this point
}
}

if (res.err) {
// This is only possible on JSON.parse error, or CORS error,
// or `unexpected redirect`
// This check should go first, to avoid calling .text() twice (which will fail)
cause = _errorLikeToErrorObject(res.err)
} else if (res.fetchResponse) {
const body = _jsonParseIfPossible(await res.fetchResponse.text())
if (body) {
cause = _anyToErrorObject(body)
} else if (res.body) {
cause = _anyToErrorObject(res.body)
} else {
cause = {
name: 'Error',
message: 'Fetch failed',
data: {},
}
}

cause ||= {
name: 'Error',
message: 'Fetch failed',
data: {},
}

let responseStatusCode = res.fetchResponse?.status || 0
if (res.statusFamily === 2) {
// important to reset responseStatusCode to 0 in this case, as status 2xx can be misleading
Expand Down

0 comments on commit 40ad04e

Please sign in to comment.