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

🐛[request] do not consider opaque response as error #197

Merged
merged 2 commits into from
Dec 10, 2019
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
8 changes: 5 additions & 3 deletions packages/core/src/requestCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export interface RequestDetails {
url: string
status: number
response?: string
responseType?: string
startTime: number
duration: number
traceId?: number
Expand Down Expand Up @@ -100,7 +101,7 @@ export function trackFetch(observable: RequestObservable) {
window.fetch = monitor(function(this: GlobalFetch['fetch'], input: RequestInfo, init?: RequestInit) {
const method = (init && init.method) || (typeof input === 'object' && input.method) || 'GET'
const startTime = performance.now()
const reportFetchError = async (response: Response | Error) => {
const reportFetch = async (response: Response | Error) => {
const duration = performance.now() - startTime
const url = normalizeUrl((typeof input === 'object' && input.url) || (input as string))
if ('stack' in response || response instanceof Error) {
Expand All @@ -122,13 +123,14 @@ export function trackFetch(observable: RequestObservable) {
startTime,
url,
response: text,
responseType: response.type,
status: response.status,
type: RequestType.FETCH,
})
}
}
const responsePromise = originalFetch.call(this, input, init)
responsePromise.then(monitor(reportFetchError), monitor(reportFetchError))
responsePromise.then(monitor(reportFetch), monitor(reportFetch))
return responsePromise
})
}
Expand All @@ -138,7 +140,7 @@ export function normalizeUrl(url: string) {
}

export function isRejected(request: RequestDetails) {
return request.status === 0
return request.status === 0 && request.responseType !== 'opaque'
}

export function isServerError(request: RequestDetails) {
Expand Down
31 changes: 30 additions & 1 deletion packages/core/test/requestCollection.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import { Observable } from '../src/observable'
import { normalizeUrl, RequestDetails, RequestType, trackFetch } from '../src/requestCollection'
import {
isRejected,
isServerError,
normalizeUrl,
RequestDetails,
RequestType,
trackFetch,
} from '../src/requestCollection'
import { FetchStub, FetchStubBuilder, FetchStubPromise, isFirefox, isIE } from '../src/specHelper'

describe('fetch tracker', () => {
Expand Down Expand Up @@ -41,6 +48,8 @@ describe('fetch tracker', () => {
expect(request.url).toEqual(FAKE_URL)
expect(request.status).toEqual(500)
expect(request.response).toEqual('fetch error')
expect(isRejected(request)).toBe(false)
expect(isServerError(request)).toBe(true)
done()
})
})
Expand All @@ -55,6 +64,24 @@ describe('fetch tracker', () => {
expect(request.url).toEqual(FAKE_URL)
expect(request.status).toEqual(0)
expect(request.response).toMatch(/Error: fetch error/)
expect(isRejected(request)).toBe(true)
expect(isServerError(request)).toBe(false)
done()
})
})

it('should track opaque fetch', (done) => {
// https://fetch.spec.whatwg.org/#concept-filtered-response-opaque
fetchStub(FAKE_URL).resolveWith({ status: 0, type: 'opaque' })

fetchStubBuilder.whenAllComplete((requests: RequestDetails[]) => {
const request = requests[0]
expect(request.type).toEqual(RequestType.FETCH)
expect(request.method).toEqual('GET')
expect(request.url).toEqual(FAKE_URL)
expect(request.status).toEqual(0)
expect(isRejected(request)).toBe(false)
expect(isServerError(request)).toBe(false)
done()
})
})
Expand All @@ -69,6 +96,8 @@ describe('fetch tracker', () => {
expect(request.url).toEqual(FAKE_URL)
expect(request.status).toEqual(400)
expect(request.response).toEqual('Not found')
expect(isRejected(request)).toBe(false)
expect(isServerError(request)).toBe(false)
done()
})
})
Expand Down