Skip to content
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

fix: support throwing Response.error() #563

Merged
merged 1 commit into from
Apr 21, 2024
Merged
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
32 changes: 18 additions & 14 deletions src/interceptors/ClientRequest/NodeClientRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,13 @@ import { createRequest } from './utils/createRequest'
import { toInteractiveRequest } from '../../utils/toInteractiveRequest'
import { emitAsync } from '../../utils/emitAsync'
import { getRawFetchHeaders } from '../../utils/getRawFetchHeaders'
import { isPropertyAccessible } from '../../utils/isPropertyAccessible'
import { isNodeLikeError } from '../../utils/isNodeLikeError'
import { INTERNAL_REQUEST_ID_HEADER_NAME } from '../../Interceptor'
import { createRequestId } from '../../createRequestId'
import { createServerErrorResponse } from '../../utils/responseUtils'
import {
createServerErrorResponse,
isResponseError,
} from '../../utils/responseUtils'

export type Protocol = 'http' | 'https'

Expand Down Expand Up @@ -267,9 +269,20 @@ export class NodeClientRequest extends ClientRequest {
resolverResult.error
)

// Treat thrown Responses as mocked responses.
// Handle thrown Response instances.
if (resolverResult.error instanceof Response) {
this.respondWith(resolverResult.error)
// Treat thrown Response.error() as a request error.
if (isResponseError(resolverResult.error)) {
this.logger.info(
'received network error response, erroring request...'
)

this.errorWith(new TypeError('Network error'))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only difference with the RequestError. Response.error supports no error message and so it's translated to a generic Network error error.

} else {
// Handle a thrown Response as a mocked response.
this.respondWith(resolverResult.error)
}

return
}

Expand Down Expand Up @@ -304,16 +317,7 @@ export class NodeClientRequest extends ClientRequest {
this.destroyed = false

// Handle mocked "Response.error" network error responses.
if (
/**
* @note Some environments, like Miniflare (Cloudflare) do not
* implement the "Response.type" property and throw on its access.
* Safely check if we can access "type" on "Response" before continuing.
* @see https://github.com/mswjs/msw/issues/1834
*/
isPropertyAccessible(mockedResponse, 'type') &&
mockedResponse.type === 'error'
) {
if (isResponseError(mockedResponse)) {
this.logger.info(
'received network error response, erroring request...'
)
Expand Down
14 changes: 11 additions & 3 deletions src/interceptors/XMLHttpRequest/XMLHttpRequestProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import { XMLHttpRequestEmitter } from '.'
import { toInteractiveRequest } from '../../utils/toInteractiveRequest'
import { emitAsync } from '../../utils/emitAsync'
import { XMLHttpRequestController } from './XMLHttpRequestController'
import { createServerErrorResponse } from '../../utils/responseUtils'
import {
createServerErrorResponse,
isResponseError,
} from '../../utils/responseUtils'

export interface XMLHttpRequestProxyOptions {
emitter: XMLHttpRequestEmitter
Expand Down Expand Up @@ -97,7 +100,12 @@ export function createXMLHttpRequestProxy({

// Treat thrown Responses as mocked responses.
if (resolverResult.error instanceof Response) {
this.respondWith(resolverResult.error)
if (isResponseError(resolverResult.error)) {
xhrRequestController.errorWith(new TypeError('Network error'))
} else {
this.respondWith(resolverResult.error)
}

return
}
// Unhandled exceptions in the request listeners are
Expand All @@ -119,7 +127,7 @@ export function createXMLHttpRequestProxy({
mockedResponse.statusText
)

if (mockedResponse.type === 'error') {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops! Inaccessible way to check for Response.error().

if (isResponseError(mockedResponse)) {
this.logger.info(
'received a network error response, rejecting the request promise...'
)
Expand Down
17 changes: 11 additions & 6 deletions src/interceptors/fetch/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ import { HttpRequestEventMap, IS_PATCHED_MODULE } from '../../glossary'
import { Interceptor } from '../../Interceptor'
import { toInteractiveRequest } from '../../utils/toInteractiveRequest'
import { emitAsync } from '../../utils/emitAsync'
import { isPropertyAccessible } from '../../utils/isPropertyAccessible'
import { canParseUrl } from '../../utils/canParseUrl'
import { createRequestId } from '../../createRequestId'
import { createServerErrorResponse } from '../../utils/responseUtils'
import {
createServerErrorResponse,
isResponseError,
} from '../../utils/responseUtils'

export class FetchInterceptor extends Interceptor<HttpRequestEventMap> {
static symbol = Symbol('fetch')
Expand Down Expand Up @@ -142,6 +144,12 @@ export class FetchInterceptor extends Interceptor<HttpRequestEventMap> {
if (resolverResult.error) {
// Treat thrown Responses as mocked responses.
if (resolverResult.error instanceof Response) {
// Treat thrown Response.error() as a request error.
if (isResponseError(resolverResult.error)) {
return Promise.reject(createNetworkError(resolverResult.error))
}

// Treat the rest of thrown Responses as mocked responses.
return respondWith(resolverResult.error)
}

Expand All @@ -157,10 +165,7 @@ export class FetchInterceptor extends Interceptor<HttpRequestEventMap> {
this.logger.info('received mocked response:', mockedResponse)

// Reject the request Promise on mocked "Response.error" responses.
if (
isPropertyAccessible(mockedResponse, 'type') &&
mockedResponse.type === 'error'
) {
if (isResponseError(mockedResponse)) {
this.logger.info(
'received a network error response, rejecting the request promise...'
)
Expand Down
7 changes: 4 additions & 3 deletions src/utils/isObject.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
import { it, expect } from 'vitest'
import { isObject } from './isObject'

it('resolves given an object', () => {
it('returns true given an object', () => {
expect(isObject({})).toBe(true)
expect(isObject({ a: 1 })).toBe(true)
})

it('rejects given an object-like instance', () => {
it('returns false given an object-like instance', () => {
expect(isObject([1])).toBe(false)
expect(isObject(function () {})).toBe(false)
expect(isObject(new Response())).toBe(false)
})

it('rejects given a non-object instance', () => {
it('returns false given a non-object instance', () => {
expect(isObject(null)).toBe(false)
expect(isObject(undefined)).toBe(false)
expect(isObject(false)).toBe(false)
Expand Down
6 changes: 4 additions & 2 deletions src/utils/isObject.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
/**
* Determines if a given value is an instance of object.
*/
export function isObject<T>(value: any): value is T {
return Object.prototype.toString.call(value) === '[object Object]'
export function isObject<T>(value: any, loose = false): value is T {
return loose
? Object.prototype.toString.call(value).startsWith('[object ')
: Object.prototype.toString.call(value) === '[object Object]'
}
16 changes: 16 additions & 0 deletions src/utils/responseUtils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { isPropertyAccessible } from './isPropertyAccessible'

/**
* Response status codes for responses that cannot have body.
* @see https://fetch.spec.whatwg.org/#statuses
Expand Down Expand Up @@ -37,3 +39,17 @@ export function createServerErrorResponse(body: unknown): Response {
}
)
}

/**
* Checks if the given response is a `Response.error()`.
*
* @note Some environments, like Miniflare (Cloudflare) do not
* implement the "Response.type" property and throw on its access.
* Safely check if we can access "type" on "Response" before continuing.
* @see https://github.com/mswjs/msw/issues/1834
*/
export function isResponseError(
response: Response
): response is Response & { type: 'error' } {
return isPropertyAccessible(response, 'type') && response.type === 'error'
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ it('treats a thrown Response instance as a mocked response', async () => {
expect(request.responseText).toBe('hello world')
})

it('treats Response.error() as a network error', async () => {
it('treats a Response.error() as a network error', async () => {
interceptor.on('request', ({ request }) => {
request.respondWith(Response.error())
})
Expand All @@ -94,3 +94,20 @@ it('treats Response.error() as a network error', async () => {
expect(request.status).toBe(0)
expect(requestErrorListener).toHaveBeenCalledTimes(1)
})

it('treats a thrown Response.error() as a network error', async () => {
interceptor.on('request', () => {
throw Response.error()
})

const requestErrorListener = vi.fn()
const request = await createXMLHttpRequest((request) => {
request.responseType = 'text'
request.open('GET', 'http://localhost/api')
request.addEventListener('error', requestErrorListener)
request.send()
})

expect(request.status).toBe(0)
expect(requestErrorListener).toHaveBeenCalledTimes(1)
})
18 changes: 17 additions & 1 deletion test/modules/fetch/fetch-exception.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ it('treats a thrown Response as a mocked response', async () => {
expect(await response.text()).toBe('hello world')
})

it('treats Response.error() as a network error', async () => {
it('treats a Response.error() as a network error', async () => {
interceptor.on('request', ({ request }) => {
request.respondWith(Response.error())
})
Expand All @@ -61,3 +61,19 @@ it('treats Response.error() as a network error', async () => {
expect(requestError.message).toBe('Failed to fetch')
expect(requestError.cause).toBeInstanceOf(Response)
})

it('treats a thrown Response.error() as a network error', async () => {
interceptor.on('request', () => {
throw Response.error()
})

const requestError = await fetch('http://localhost:3001/resource')
.then(() => {
throw new Error('Must not resolve')
})
.catch<TypeError & { cause?: unknown }>((error) => error)

expect(requestError.name).toBe('TypeError')
expect(requestError.message).toBe('Failed to fetch')
expect(requestError.cause).toBeInstanceOf(Response)
})
61 changes: 46 additions & 15 deletions test/modules/http/response/http-response-error.test.ts
Original file line number Diff line number Diff line change
@@ -1,32 +1,63 @@
import { it, expect, beforeAll, afterAll } from 'vitest'
import http from 'http'
import { vi, it, expect, beforeAll, afterAll, afterEach } from 'vitest'
import http from 'node:http'
import { ClientRequestInterceptor } from '../../../../src/interceptors/ClientRequest'
import { DeferredPromise } from '@open-draft/deferred-promise'

const interceptor = new ClientRequestInterceptor()

interceptor.on('request', ({ request }) => {
request.respondWith(Response.error())
})

beforeAll(() => {
interceptor.apply()
})

afterEach(() => {
interceptor.removeAllListeners()
})

afterAll(() => {
interceptor.dispose()
})

it('treats "Response.error()" as a request error', async () => {
const requestErrorPromise = new DeferredPromise<Error>()
it('treats "Response.error()" as a network error', async () => {
const requestErrorListener = vi.fn()
const responseListener = vi.fn()

interceptor.on('request', ({ request }) => {
request.respondWith(Response.error())
})
interceptor.on('response', responseListener)

const request = http.get('http://localhost:3001/resource')
request.on('error', requestErrorListener)

// Must handle Response.error() as a network error.
await vi.waitFor(() => {
expect(requestErrorListener).toHaveBeenNthCalledWith(
1,
new TypeError('Network error')
)
})

expect(responseListener).not.toHaveBeenCalled()
})

it('treats a thrown Response.error() as a network error', async () => {
const requestErrorListener = vi.fn()
const responseListener = vi.fn()

interceptor.on('request', () => {
throw Response.error()
})
interceptor.on('response', responseListener)

const request = http.get('http://localhost:3001/resource')
request.on('error', requestErrorListener)

// In ClientRequest, network errors are forwarded as
// request errors.
request.on('error', requestErrorPromise.resolve)
const requestError = await requestErrorPromise
// Must handle Response.error() as a request error.
await vi.waitFor(() => {
expect(requestErrorListener).toHaveBeenNthCalledWith(
1,
new TypeError('Network error')
)
})

expect(requestError).toBeInstanceOf(TypeError)
expect(requestError.message).toBe('Network error')
expect(responseListener).not.toHaveBeenCalled()
})
Loading