From 3bbc5647f3d046376635ca491497744dbf6cba2d Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 9 Dec 2022 13:19:17 +0000 Subject: [PATCH 1/9] feat(browser): HTTPClient Integration --- packages/browser/src/exports.ts | 2 +- .../browser/src/integrations/httpclient.ts | 423 ++++++++++++++++++ .../browser/src/integrations/httpcontext.ts | 2 +- packages/browser/src/integrations/index.ts | 1 + packages/browser/src/sdk.ts | 3 +- packages/integration-tests/package.json | 2 +- .../integrations/httpclient/fetch/init.js | 14 + .../integrations/httpclient/fetch/subject.js | 9 + .../integrations/httpclient/fetch/test.ts | 67 +++ packages/types/src/context.ts | 9 + 10 files changed, 528 insertions(+), 4 deletions(-) create mode 100644 packages/browser/src/integrations/httpclient.ts create mode 100644 packages/integration-tests/suites/integrations/httpclient/fetch/init.js create mode 100644 packages/integration-tests/suites/integrations/httpclient/fetch/subject.js create mode 100644 packages/integration-tests/suites/integrations/httpclient/fetch/test.ts diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index ad96e21c4ffe..d198d921d6ae 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -58,4 +58,4 @@ export { winjsStackLineParser, } from './stack-parsers'; export { defaultIntegrations, forceLoad, init, lastEventId, onLoad, showReportDialog, flush, close, wrap } from './sdk'; -export { GlobalHandlers, TryCatch, Breadcrumbs, LinkedErrors, HttpContext, Dedupe } from './integrations'; +export { GlobalHandlers, TryCatch, Breadcrumbs, LinkedErrors, HttpContext, HttpClient, Dedupe } from './integrations'; diff --git a/packages/browser/src/integrations/httpclient.ts b/packages/browser/src/integrations/httpclient.ts new file mode 100644 index 000000000000..d056a4840b9d --- /dev/null +++ b/packages/browser/src/integrations/httpclient.ts @@ -0,0 +1,423 @@ +import { captureEvent, getCurrentHub } from '@sentry/core'; +import { Event as SentryEvent, Integration } from '@sentry/types'; +import { addExceptionMechanism, fill, GLOBAL_OBJ, logger } from '@sentry/utils'; + +import { eventFromUnknownInput } from '../eventbuilder'; + +export type HttpStatusCodeRange = [number, number] | number; +export type HttpRequestTarget = string | RegExp; + +interface HttpClientOptions { + captureFailedRequests?: boolean; + failedRequestStatusCodes?: HttpStatusCodeRange[]; + failedRequestTargets?: HttpRequestTarget[]; +} + +/** HTTPClient integration creates events for failed client side HTTP requests. */ +export class HttpClient implements Integration { + /** + * @inheritDoc + */ + public static id: string = 'HttpClient'; + + /** + * @inheritDoc + */ + public name: string = HttpClient.id; + + private readonly _options: HttpClientOptions; + + /** + * @inheritDoc + * + * @param options + */ + public constructor(options?: Partial) { + this._options = { + captureFailedRequests: false, + failedRequestStatusCodes: [[500, 599]], + failedRequestTargets: [/.*/], + ...options, + }; + } + + /** + * @inheritDoc + * + * @param options + */ + public setupOnce(): void { + if (!this._options.captureFailedRequests) { + return; + } + + this._wrapFetch(); + this._wrapXHR(); + } + + /** + * Interceptor function for fetch requests + * + * @param requestInfo The Fetch API request info + * @param response The Fetch API response + * @param requestInit The request init object + */ + private _fetchResponseHandler(requestInfo: RequestInfo, response: Response, requestInit?: RequestInit): void { + if (this._shouldCaptureResponse(response.status, response.url)) { + const request = new Request(requestInfo, requestInit); + const url = new URL(request.url); + + let requestHeaders, responseHeaders, requestCookies, responseCookies; + + if (getCurrentHub().shouldSendDefaultPii()) { + [{ headers: requestHeaders, cookies: requestCookies }, { headers: responseHeaders, cookies: responseCookies }] = + [ + { cookieHeader: 'Cookie', obj: request }, + { cookieHeader: 'Set-Cookie', obj: response }, + ].map(({ cookieHeader, obj }) => { + const headers = this._extractFetchHeaders(obj.headers); + let cookies; + + try { + const cookieString = headers[cookieHeader] || headers[cookieHeader.toLowerCase()] || undefined; + + if (cookieString) { + cookies = this._parseCookieString(cookieString); + } + } catch (e) { + __DEBUG_BUILD__ && logger.log(`Could not extract cookies from header ${cookieHeader}`); + } + + return { + headers, + cookies, + }; + }); + } + + const event = this._createEvent({ + url: url, + method: request.method, + status: response.status, + requestHeaders, + responseHeaders, + requestCookies, + responseCookies, + }); + + captureEvent(event, { + data: { + OK_HTTP_REQUEST: request, + OK_HTTP_RESPONSE: response, + }, + }); + } + } + + /** + * Interceptor function for XHR requests + * + * @param xhr The XHR request + * @param method The HTTP method + * @param headers The HTTP headers + */ + private _xhrResponseHandler(xhr: XMLHttpRequest, method: string, headers: Record): void { + if (this._shouldCaptureResponse(xhr.status, xhr.responseURL)) { + const url = new URL(xhr.responseURL); + + let requestHeaders, responseCookies, responseHeaders; + + if (getCurrentHub().shouldSendDefaultPii()) { + try { + const cookieString = xhr.getResponseHeader('Set-Cookie') || xhr.getResponseHeader('set-cookie') || undefined; + + if (cookieString) { + responseCookies = this._parseCookieString(cookieString); + } + } catch (e) { + __DEBUG_BUILD__ && logger.log('Could not extract cookies from response headers'); + } + + try { + responseHeaders = this._getXHRResponseHeaders(xhr); + } catch (e) { + __DEBUG_BUILD__ && logger.log('Could not extract headers from response'); + } + + requestHeaders = headers; + } + + const event = this._createEvent({ + url: url, + method: method, + status: xhr.status, + requestHeaders, + // Can't access request cookies from XHR + responseHeaders, + responseCookies, + }); + + // Note: Not adding request / response objects as hints for XHR + captureEvent(event); + } + } + + /** + * Extracts response size from `Content-Length` header when possible + * + * @param headers + * @returns The response size in bytes or undefined + */ + private _getResponseSizeFromHeaders(headers?: Record): number | undefined { + if (headers) { + const contentLength = headers['Content-Length'] || headers['content-length']; + + if (contentLength) { + return parseInt(contentLength, 10); + } + } + + return undefined; + } + + /** + * Creates an object containing cookies from the given cookie string + * + * @param cookieString The cookie string to parse + * @returns The parsed cookies + */ + private _parseCookieString(cookieString: string): Record { + return cookieString.split('; ').reduce((acc: Record, cookie: string) => { + const [key, value] = cookie.split('='); + acc[key] = value; + return acc; + }, {}); + } + + /** + * Extracts the headers as an object from the given Fetch API request or response object + * + * @param headers The headers to extract + * @returns The extracted headers as an object + */ + private _extractFetchHeaders(headers: Headers): Record { + const result: Record = {}; + + headers.forEach((value, key) => { + result[key] = value; + }); + + return result; + } + + /** + * Extracts the response headers as an object from the given XHR object + * + * @param xhr The XHR object to extract the response headers from + * @returns The response headers as an object + */ + private _getXHRResponseHeaders(xhr: XMLHttpRequest): Record { + const headers = xhr.getAllResponseHeaders(); + + if (!headers) { + return {}; + } + + return headers.split('\r\n').reduce((acc: Record, line: string) => { + const [key, value] = line.split(': '); + acc[key] = value; + return acc; + }, {}); + } + + /** + * Checks if the given target url is in the given list of targets + * + * @param target The target url to check + * @returns true if the target url is in the given list of targets, false otherwise + */ + private _isInGivenRequestTargets(target: string): boolean { + if (!this._options.failedRequestTargets) { + return false; + } + + return this._options.failedRequestTargets.some((givenRequestTarget: HttpRequestTarget) => { + if (typeof givenRequestTarget === 'string') { + return target === givenRequestTarget; + } + + return givenRequestTarget.test(target); + }); + } + + /** + * Checks if the given status code is in the given range + * + * @param status The status code to check + * @returns true if the status code is in the given range, false otherwise + */ + private _isInGivenStatusRanges(status: number): boolean { + if (!this._options.failedRequestStatusCodes) { + return false; + } + + return this._options.failedRequestStatusCodes.some((range: HttpStatusCodeRange) => { + if (typeof range === 'number') { + return range === status; + } + + return status >= range[0] && status <= range[1]; + }); + } + + /** + * + */ + private _wrapFetch(): void { + if (!('fetch' in GLOBAL_OBJ)) { + return; + } + + // eslint-disable-next-line @typescript-eslint/no-this-alias + const self = this; + + fill( + GLOBAL_OBJ, + 'fetch', + function (originalFetch: (requestInfo: RequestInfo, requestInit?: RequestInit) => Promise) { + return function (this: Window, requestInfo: RequestInfo, requestInit?: RequestInit): Promise { + const responsePromise: Promise = originalFetch.apply(this, [requestInfo, requestInit]); + + responsePromise + .then((response: Response) => { + self._fetchResponseHandler(requestInfo, response, requestInit); + return response; + }) + .catch((error: Error) => { + // TODO: + throw error; + }); + + return responsePromise; + }; + }, + ); + } + + /** + * Wraps XMLHttpRequest to capture request and response data + */ + private _wrapXHR(): void { + if (!('XMLHttpRequest' in GLOBAL_OBJ)) { + return; + } + + // eslint-disable-next-line @typescript-eslint/no-this-alias + const self = this; + + fill(XMLHttpRequest.prototype, 'open', function (originalOpen: (method: string) => void): () => void { + return function (this: XMLHttpRequest, ...openArgs: any[]): void { + // eslint-disable-next-line @typescript-eslint/no-this-alias + const xhr = this; + const method = openArgs[0]; + const headers: Record = {}; + + // Intercepting `setRequestHeader` to access the request headers of XHR instance. + // This will only work for user/library defined headers, not for the default/browser-assigned headers. + // Request cookies are also unavailable for XHR, as `Cookie` header can't be defined by `setRequestHeader`. + fill( + xhr, + 'setRequestHeader', + // eslint-disable-next-line @typescript-eslint/ban-types + function (originalSetRequestHeader: (header: string, value: string) => void): Function { + return function (header: string, value: string): void { + headers[header] = value; + + return originalSetRequestHeader.apply(xhr, [header, value]); + }; + }, + ); + + // eslint-disable-next-line @typescript-eslint/ban-types + fill(xhr, 'onloadend', function (original?: (progressEvent: ProgressEvent) => void): Function { + return function (progressEvent: ProgressEvent): void { + try { + self._xhrResponseHandler(xhr, method, headers); + } catch (e) { + __DEBUG_BUILD__ && logger.warn('Error while extracting response event form XHR response', e); + } + + return original?.apply(xhr, progressEvent); + }; + }); + + return originalOpen.apply(this, openArgs); + }; + }); + } + + /** + * Checks whether given url points to Sentry server + * + * @param url url to verify + */ + private _isSentryRequest(url: string): boolean { + const dsn = getCurrentHub().getClient()?.getDsn(); + return dsn ? url.includes(dsn.host) : false; + } + + /** + * Checks whether to capture given response as an event + * + * @param status response status code + * @param url response url + */ + private _shouldCaptureResponse(status: number, url: string): boolean { + return this._isInGivenStatusRanges(status) && this._isInGivenRequestTargets(url) && !this._isSentryRequest(url); + } + + /** + * Creates a synthetic Sentry event from given response data + * + * @param data response data + * @returns event + */ + private _createEvent(data: { + url: URL; + method: string; + status: number; + responseHeaders?: Record; + responseCookies?: Record; + requestHeaders?: Record; + requestCookies?: Record; + }): SentryEvent { + const event = eventFromUnknownInput(() => [], `HTTP Client Error with status code: ${data.status}`); + + event.request = { + url: data.url.href, + query_string: data.url.search, + // TODO: should we add `data: request.body` too? + // https://develop.sentry.dev/sdk/event-payloads/request/ + method: data.method, + headers: data.requestHeaders, + cookies: data.requestCookies, + }; + + event.contexts = { + ...event.contexts, + response: { + type: 'response', + status_code: data.status, + headers: data.responseHeaders, + cookies: data.responseCookies, + body_size: this._getResponseSizeFromHeaders(data.responseHeaders), + }, + }; + + addExceptionMechanism(event, { + type: 'http.client', + }); + + return event; + } +} diff --git a/packages/browser/src/integrations/httpcontext.ts b/packages/browser/src/integrations/httpcontext.ts index b99eeeb7540b..88f2873882d4 100644 --- a/packages/browser/src/integrations/httpcontext.ts +++ b/packages/browser/src/integrations/httpcontext.ts @@ -36,7 +36,7 @@ export class HttpContext implements Integration { ...(referrer && { Referer: referrer }), ...(userAgent && { 'User-Agent': userAgent }), }; - const request = { ...(url && { url }), headers }; + const request = { ...event.request, ...(url && { url }), headers }; return { ...event, request }; } diff --git a/packages/browser/src/integrations/index.ts b/packages/browser/src/integrations/index.ts index e029422f363c..f581d82c4fe5 100644 --- a/packages/browser/src/integrations/index.ts +++ b/packages/browser/src/integrations/index.ts @@ -4,3 +4,4 @@ export { Breadcrumbs } from './breadcrumbs'; export { LinkedErrors } from './linkederrors'; export { HttpContext } from './httpcontext'; export { Dedupe } from './dedupe'; +export { HttpClient } from './httpclient'; diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index f4c05482b4a9..8246ec2dbff5 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -16,13 +16,14 @@ import { import { BrowserClient, BrowserClientOptions, BrowserOptions } from './client'; import { ReportDialogOptions, WINDOW, wrap as internalWrap } from './helpers'; -import { Breadcrumbs, Dedupe, GlobalHandlers, HttpContext, LinkedErrors, TryCatch } from './integrations'; +import { Breadcrumbs, Dedupe, GlobalHandlers, HttpClient, HttpContext, LinkedErrors, TryCatch } from './integrations'; import { defaultStackParser } from './stack-parsers'; import { makeFetchTransport, makeXHRTransport } from './transports'; export const defaultIntegrations = [ new CoreIntegrations.InboundFilters(), new CoreIntegrations.FunctionToString(), + new HttpClient(), new TryCatch(), new Breadcrumbs(), new GlobalHandlers(), diff --git a/packages/integration-tests/package.json b/packages/integration-tests/package.json index 6ab725c711ff..017f8809d1ee 100644 --- a/packages/integration-tests/package.json +++ b/packages/integration-tests/package.json @@ -14,7 +14,7 @@ "lint:eslint": "eslint . --cache --cache-location '../../eslintcache/' --format stylish", "lint:prettier": "prettier --check \"{suites,utils}/**/*.ts\"", "type-check": "tsc", - "pretest": "yarn clean && yarn type-check", + "pretest": "yarn clean", "test": "playwright test ./suites", "test:bundle:es5": "PW_BUNDLE=bundle_es5 yarn test", "test:bundle:es5:min": "PW_BUNDLE=bundle_es5_min yarn test", diff --git a/packages/integration-tests/suites/integrations/httpclient/fetch/init.js b/packages/integration-tests/suites/integrations/httpclient/fetch/init.js new file mode 100644 index 000000000000..61e1fa5f8351 --- /dev/null +++ b/packages/integration-tests/suites/integrations/httpclient/fetch/init.js @@ -0,0 +1,14 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [ + new Sentry.Integrations.HttpClient({ + captureFailedRequests: true, + }), + ], + tracesSampleRate: 1, + sendDefaultPii: true, +}); diff --git a/packages/integration-tests/suites/integrations/httpclient/fetch/subject.js b/packages/integration-tests/suites/integrations/httpclient/fetch/subject.js new file mode 100644 index 000000000000..94ab60d92ed9 --- /dev/null +++ b/packages/integration-tests/suites/integrations/httpclient/fetch/subject.js @@ -0,0 +1,9 @@ +fetch('http://localhost:7654/foo', { + method: 'GET', + credentials: 'include', + headers: { + Accept: 'application/json', + 'Content-Type': 'application/json', + Cache: 'no-cache', + }, +}); diff --git a/packages/integration-tests/suites/integrations/httpclient/fetch/test.ts b/packages/integration-tests/suites/integrations/httpclient/fetch/test.ts new file mode 100644 index 000000000000..a440b1ab5ee2 --- /dev/null +++ b/packages/integration-tests/suites/integrations/httpclient/fetch/test.ts @@ -0,0 +1,67 @@ +import { expect } from '@playwright/test'; +import { Event } from '@sentry/types'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers'; + +sentryTest( + 'should assign request and response context from a failed 500 request', + async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + await page.route('**/foo', route => { + return route.fulfill({ + status: 500, + body: JSON.stringify({ + error: { + message: 'Internal Server Error', + }, + }), + headers: { + 'Content-Type': 'text/html', + }, + }); + }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.exception?.values).toHaveLength(1); + + // Not able to get the cookies from the request/response because of Playwright bug + // https://github.com/microsoft/playwright/issues/11035 + expect(eventData).toMatchObject({ + message: 'HTTP Client Error with status code: 500', + exception: { + values: [ + { + type: 'Error', + value: 'HTTP Client Error with status code: 500', + mechanism: { + type: 'http.client', + handled: true, + }, + }, + ], + }, + request: { + url: 'http://localhost:7654/foo', + method: 'GET', + headers: { + accept: 'application/json', + cache: 'no-cache', + 'content-type': 'application/json', + }, + }, + contexts: { + response: { + status_code: 500, + body_size: 45, + headers: { + 'content-type': 'text/html', + 'content-length': '45', + }, + }, + }, + }); + }, +); diff --git a/packages/types/src/context.ts b/packages/types/src/context.ts index 4b6a08585273..e725b68ea0c3 100644 --- a/packages/types/src/context.ts +++ b/packages/types/src/context.ts @@ -5,6 +5,7 @@ export interface Contexts extends Record { device?: DeviceContext; os?: OsContext; culture?: CultureContext; + response?: ResponseContext; } export interface AppContext extends Record { @@ -70,3 +71,11 @@ export interface CultureContext extends Record { is_24_hour_format?: boolean; timezone?: string; } + +export interface ResponseContext extends Record { + type?: string; + cookies?: string[][] | Record; + headers?: Record; // TODO: should be only Record? + status_code?: number; + body_size?: number; // in bytes +} From 92c506d672530d709373fb9e707165195da0e303 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 13 Dec 2022 14:10:03 +0000 Subject: [PATCH 2/9] Add integration test for XHR interception. --- .../integrations/httpclient/fetch/test.ts | 2 +- .../httpclient/{fetch => }/init.js | 0 .../integrations/httpclient/xhr/subject.js | 8 +++ .../integrations/httpclient/xhr/test.ts | 67 +++++++++++++++++++ 4 files changed, 76 insertions(+), 1 deletion(-) rename packages/integration-tests/suites/integrations/httpclient/{fetch => }/init.js (100%) create mode 100644 packages/integration-tests/suites/integrations/httpclient/xhr/subject.js create mode 100644 packages/integration-tests/suites/integrations/httpclient/xhr/test.ts diff --git a/packages/integration-tests/suites/integrations/httpclient/fetch/test.ts b/packages/integration-tests/suites/integrations/httpclient/fetch/test.ts index a440b1ab5ee2..d399dd209a84 100644 --- a/packages/integration-tests/suites/integrations/httpclient/fetch/test.ts +++ b/packages/integration-tests/suites/integrations/httpclient/fetch/test.ts @@ -5,7 +5,7 @@ import { sentryTest } from '../../../../utils/fixtures'; import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers'; sentryTest( - 'should assign request and response context from a failed 500 request', + 'should assign request and response context from a failed 500 fetch request', async ({ getLocalTestPath, page }) => { const url = await getLocalTestPath({ testDir: __dirname }); diff --git a/packages/integration-tests/suites/integrations/httpclient/fetch/init.js b/packages/integration-tests/suites/integrations/httpclient/init.js similarity index 100% rename from packages/integration-tests/suites/integrations/httpclient/fetch/init.js rename to packages/integration-tests/suites/integrations/httpclient/init.js diff --git a/packages/integration-tests/suites/integrations/httpclient/xhr/subject.js b/packages/integration-tests/suites/integrations/httpclient/xhr/subject.js new file mode 100644 index 000000000000..7a2e3cdd28c0 --- /dev/null +++ b/packages/integration-tests/suites/integrations/httpclient/xhr/subject.js @@ -0,0 +1,8 @@ +const xhr = new XMLHttpRequest(); + +xhr.open('GET', 'http://localhost:7654/foo', true); +xhr.withCredentials = true; +xhr.setRequestHeader('Accept', 'application/json'); +xhr.setRequestHeader('Content-Type', 'application/json'); +xhr.setRequestHeader('Cache', 'no-cache'); +xhr.send(); diff --git a/packages/integration-tests/suites/integrations/httpclient/xhr/test.ts b/packages/integration-tests/suites/integrations/httpclient/xhr/test.ts new file mode 100644 index 000000000000..72167f73ca2b --- /dev/null +++ b/packages/integration-tests/suites/integrations/httpclient/xhr/test.ts @@ -0,0 +1,67 @@ +import { expect } from '@playwright/test'; +import { Event } from '@sentry/types'; + +import { sentryTest } from '../../../../utils/fixtures'; +import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers'; + +sentryTest( + 'should assign request and response context from a failed 500 XHR request', + async ({ getLocalTestPath, page }) => { + const url = await getLocalTestPath({ testDir: __dirname }); + + await page.route('**/foo', route => { + return route.fulfill({ + status: 500, + body: JSON.stringify({ + error: { + message: 'Internal Server Error', + }, + }), + headers: { + 'Content-Type': 'text/html', + }, + }); + }); + + const eventData = await getFirstSentryEnvelopeRequest(page, url); + + expect(eventData.exception?.values).toHaveLength(1); + + // Not able to get the cookies from the request/response because of Playwright bug + // https://github.com/microsoft/playwright/issues/11035 + expect(eventData).toMatchObject({ + message: 'HTTP Client Error with status code: 500', + exception: { + values: [ + { + type: 'Error', + value: 'HTTP Client Error with status code: 500', + mechanism: { + type: 'http.client', + handled: true, + }, + }, + ], + }, + request: { + url: 'http://localhost:7654/foo', + method: 'GET', + headers: { + Accept: 'application/json', + Cache: 'no-cache', + 'Content-Type': 'application/json', + }, + }, + contexts: { + response: { + status_code: 500, + body_size: 45, + headers: { + 'content-type': 'text/html', + 'content-length': '45', + }, + }, + }, + }); + }, +); From 1c68a68b2b286a29d83645e9a5eaea39e3f999ad Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 21 Dec 2022 14:22:52 +0000 Subject: [PATCH 3/9] Address review comments. --- .../browser/src/integrations/httpclient.ts | 106 ++++++++++-------- packages/browser/src/sdk.ts | 3 +- packages/types/src/context.ts | 2 +- 3 files changed, 59 insertions(+), 52 deletions(-) diff --git a/packages/browser/src/integrations/httpclient.ts b/packages/browser/src/integrations/httpclient.ts index d056a4840b9d..0a0deb10a64f 100644 --- a/packages/browser/src/integrations/httpclient.ts +++ b/packages/browser/src/integrations/httpclient.ts @@ -1,15 +1,36 @@ import { captureEvent, getCurrentHub } from '@sentry/core'; import { Event as SentryEvent, Integration } from '@sentry/types'; -import { addExceptionMechanism, fill, GLOBAL_OBJ, logger } from '@sentry/utils'; +import { addExceptionMechanism, fill, GLOBAL_OBJ, logger, supportsNativeFetch } from '@sentry/utils'; import { eventFromUnknownInput } from '../eventbuilder'; export type HttpStatusCodeRange = [number, number] | number; export type HttpRequestTarget = string | RegExp; - interface HttpClientOptions { + /** + * Controls whether failed requests should be captured. + * + * Default: false + */ captureFailedRequests?: boolean; + + /** + * HTTP status codes that should be considered failed. + * This array can contain tuples of `[begin, end]` (both inclusive), + * single status codes, or a combinations of both + * + * Example: [[500, 505], 507] + * Default: [[500, 599]] + */ failedRequestStatusCodes?: HttpStatusCodeRange[]; + + /** + * Targets to track for failed requests. + * This array can contain strings or regular expressions. + * + * Example: ['http://localhost', /api\/.*\/] + * Default: [/.*\/] + */ failedRequestTargets?: HttpRequestTarget[]; } @@ -65,7 +86,6 @@ export class HttpClient implements Integration { private _fetchResponseHandler(requestInfo: RequestInfo, response: Response, requestInit?: RequestInit): void { if (this._shouldCaptureResponse(response.status, response.url)) { const request = new Request(requestInfo, requestInit); - const url = new URL(request.url); let requestHeaders, responseHeaders, requestCookies, responseCookies; @@ -96,7 +116,7 @@ export class HttpClient implements Integration { } const event = this._createEvent({ - url: url, + url: request.url, method: request.method, status: response.status, requestHeaders, @@ -105,12 +125,7 @@ export class HttpClient implements Integration { responseCookies, }); - captureEvent(event, { - data: { - OK_HTTP_REQUEST: request, - OK_HTTP_RESPONSE: response, - }, - }); + captureEvent(event); } } @@ -123,8 +138,6 @@ export class HttpClient implements Integration { */ private _xhrResponseHandler(xhr: XMLHttpRequest, method: string, headers: Record): void { if (this._shouldCaptureResponse(xhr.status, xhr.responseURL)) { - const url = new URL(xhr.responseURL); - let requestHeaders, responseCookies, responseHeaders; if (getCurrentHub().shouldSendDefaultPii()) { @@ -148,7 +161,7 @@ export class HttpClient implements Integration { } const event = this._createEvent({ - url: url, + url: xhr.responseURL, method: method, status: xhr.status, requestHeaders, @@ -274,34 +287,30 @@ export class HttpClient implements Integration { * */ private _wrapFetch(): void { - if (!('fetch' in GLOBAL_OBJ)) { + if (!supportsNativeFetch()) { return; } // eslint-disable-next-line @typescript-eslint/no-this-alias const self = this; - fill( - GLOBAL_OBJ, - 'fetch', - function (originalFetch: (requestInfo: RequestInfo, requestInit?: RequestInit) => Promise) { - return function (this: Window, requestInfo: RequestInfo, requestInit?: RequestInit): Promise { - const responsePromise: Promise = originalFetch.apply(this, [requestInfo, requestInit]); - - responsePromise - .then((response: Response) => { - self._fetchResponseHandler(requestInfo, response, requestInit); - return response; - }) - .catch((error: Error) => { - // TODO: - throw error; - }); - - return responsePromise; - }; - }, - ); + fill(GLOBAL_OBJ, 'fetch', function (originalFetch: (...args: unknown[]) => Promise) { + return function (this: Window, ...args: unknown[]): Promise { + const [requestInfo, requestInit] = args as [RequestInfo, RequestInit | undefined]; + const responsePromise: Promise = originalFetch.apply(this, args); + + responsePromise + .then((response: Response) => { + self._fetchResponseHandler(requestInfo, response, requestInit); + return response; + }) + .catch((error: Error) => { + throw error; + }); + + return responsePromise; + }; + }); } /** @@ -315,11 +324,11 @@ export class HttpClient implements Integration { // eslint-disable-next-line @typescript-eslint/no-this-alias const self = this; - fill(XMLHttpRequest.prototype, 'open', function (originalOpen: (method: string) => void): () => void { - return function (this: XMLHttpRequest, ...openArgs: any[]): void { + fill(XMLHttpRequest.prototype, 'open', function (originalOpen: (...openArgs: unknown[]) => void): () => void { + return function (this: XMLHttpRequest, ...openArgs: unknown[]): void { // eslint-disable-next-line @typescript-eslint/no-this-alias const xhr = this; - const method = openArgs[0]; + const method = openArgs[0] as string; const headers: Record = {}; // Intercepting `setRequestHeader` to access the request headers of XHR instance. @@ -329,25 +338,27 @@ export class HttpClient implements Integration { xhr, 'setRequestHeader', // eslint-disable-next-line @typescript-eslint/ban-types - function (originalSetRequestHeader: (header: string, value: string) => void): Function { - return function (header: string, value: string): void { + function (originalSetRequestHeader: (...setRequestHeaderArgs: unknown[]) => void): Function { + return function (...setRequestHeaderArgs: unknown[]): void { + const [header, value] = setRequestHeaderArgs as [string, string]; + headers[header] = value; - return originalSetRequestHeader.apply(xhr, [header, value]); + return originalSetRequestHeader.apply(xhr, setRequestHeaderArgs); }; }, ); // eslint-disable-next-line @typescript-eslint/ban-types - fill(xhr, 'onloadend', function (original?: (progressEvent: ProgressEvent) => void): Function { - return function (progressEvent: ProgressEvent): void { + fill(xhr, 'onloadend', function (original?: (...onloadendArgs: unknown[]) => void): Function { + return function (...onloadendArgs: unknown[]): void { try { self._xhrResponseHandler(xhr, method, headers); } catch (e) { __DEBUG_BUILD__ && logger.warn('Error while extracting response event form XHR response', e); } - return original?.apply(xhr, progressEvent); + return original?.apply(xhr, onloadendArgs); }; }); @@ -383,7 +394,7 @@ export class HttpClient implements Integration { * @returns event */ private _createEvent(data: { - url: URL; + url: string; method: string; status: number; responseHeaders?: Record; @@ -394,10 +405,7 @@ export class HttpClient implements Integration { const event = eventFromUnknownInput(() => [], `HTTP Client Error with status code: ${data.status}`); event.request = { - url: data.url.href, - query_string: data.url.search, - // TODO: should we add `data: request.body` too? - // https://develop.sentry.dev/sdk/event-payloads/request/ + url: data.url, method: data.method, headers: data.requestHeaders, cookies: data.requestCookies, diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index 8246ec2dbff5..f4c05482b4a9 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -16,14 +16,13 @@ import { import { BrowserClient, BrowserClientOptions, BrowserOptions } from './client'; import { ReportDialogOptions, WINDOW, wrap as internalWrap } from './helpers'; -import { Breadcrumbs, Dedupe, GlobalHandlers, HttpClient, HttpContext, LinkedErrors, TryCatch } from './integrations'; +import { Breadcrumbs, Dedupe, GlobalHandlers, HttpContext, LinkedErrors, TryCatch } from './integrations'; import { defaultStackParser } from './stack-parsers'; import { makeFetchTransport, makeXHRTransport } from './transports'; export const defaultIntegrations = [ new CoreIntegrations.InboundFilters(), new CoreIntegrations.FunctionToString(), - new HttpClient(), new TryCatch(), new Breadcrumbs(), new GlobalHandlers(), diff --git a/packages/types/src/context.ts b/packages/types/src/context.ts index e725b68ea0c3..3a342373c3fe 100644 --- a/packages/types/src/context.ts +++ b/packages/types/src/context.ts @@ -75,7 +75,7 @@ export interface CultureContext extends Record { export interface ResponseContext extends Record { type?: string; cookies?: string[][] | Record; - headers?: Record; // TODO: should be only Record? + headers?: Record; status_code?: number; body_size?: number; // in bytes } From 67136adaac317cad299138365ff5d30257f21030 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 23 Dec 2022 15:10:33 +0000 Subject: [PATCH 4/9] Use partial match while checking request targest. --- packages/browser/src/integrations/httpclient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/browser/src/integrations/httpclient.ts b/packages/browser/src/integrations/httpclient.ts index 0a0deb10a64f..79982bec1e56 100644 --- a/packages/browser/src/integrations/httpclient.ts +++ b/packages/browser/src/integrations/httpclient.ts @@ -256,7 +256,7 @@ export class HttpClient implements Integration { return this._options.failedRequestTargets.some((givenRequestTarget: HttpRequestTarget) => { if (typeof givenRequestTarget === 'string') { - return target === givenRequestTarget; + return target.includes(givenRequestTarget); } return givenRequestTarget.test(target); From 6a2b90c3e06f75f328c173692b12694d36041df0 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 23 Dec 2022 15:12:10 +0000 Subject: [PATCH 5/9] Remove extra `type` property from response context. --- packages/browser/src/integrations/httpclient.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/browser/src/integrations/httpclient.ts b/packages/browser/src/integrations/httpclient.ts index 79982bec1e56..5d2b0fbd516c 100644 --- a/packages/browser/src/integrations/httpclient.ts +++ b/packages/browser/src/integrations/httpclient.ts @@ -414,7 +414,6 @@ export class HttpClient implements Integration { event.contexts = { ...event.contexts, response: { - type: 'response', status_code: data.status, headers: data.responseHeaders, cookies: data.responseCookies, From f346e2a3cf3ebbf5c85e706799264e43f8211167 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 3 Jan 2023 15:47:42 +0000 Subject: [PATCH 6/9] Address review comments. --- .../browser/src/integrations/httpclient.ts | 29 ++++++++----------- .../suites/integrations/httpclient/init.js | 6 +--- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/packages/browser/src/integrations/httpclient.ts b/packages/browser/src/integrations/httpclient.ts index 5d2b0fbd516c..046c974fa802 100644 --- a/packages/browser/src/integrations/httpclient.ts +++ b/packages/browser/src/integrations/httpclient.ts @@ -7,13 +7,6 @@ import { eventFromUnknownInput } from '../eventbuilder'; export type HttpStatusCodeRange = [number, number] | number; export type HttpRequestTarget = string | RegExp; interface HttpClientOptions { - /** - * Controls whether failed requests should be captured. - * - * Default: false - */ - captureFailedRequests?: boolean; - /** * HTTP status codes that should be considered failed. * This array can contain tuples of `[begin, end]` (both inclusive), @@ -55,7 +48,6 @@ export class HttpClient implements Integration { */ public constructor(options?: Partial) { this._options = { - captureFailedRequests: false, failedRequestStatusCodes: [[500, 599]], failedRequestTargets: [/.*/], ...options, @@ -68,10 +60,6 @@ export class HttpClient implements Integration { * @param options */ public setupOnce(): void { - if (!this._options.captureFailedRequests) { - return; - } - this._wrapFetch(); this._wrapXHR(); } @@ -170,7 +158,6 @@ export class HttpClient implements Integration { responseCookies, }); - // Note: Not adding request / response objects as hints for XHR captureEvent(event); } } @@ -284,7 +271,7 @@ export class HttpClient implements Integration { } /** - * + * Wraps `fetch` function to capture request and response data */ private _wrapFetch(): void { if (!supportsNativeFetch()) { @@ -314,7 +301,7 @@ export class HttpClient implements Integration { } /** - * Wraps XMLHttpRequest to capture request and response data + * Wraps XMLHttpRequest to capture request and response data */ private _wrapXHR(): void { if (!('XMLHttpRequest' in GLOBAL_OBJ)) { @@ -358,7 +345,9 @@ export class HttpClient implements Integration { __DEBUG_BUILD__ && logger.warn('Error while extracting response event form XHR response', e); } - return original?.apply(xhr, onloadendArgs); + if (original) { + return original.apply(xhr, onloadendArgs); + } }; }); @@ -373,7 +362,13 @@ export class HttpClient implements Integration { * @param url url to verify */ private _isSentryRequest(url: string): boolean { - const dsn = getCurrentHub().getClient()?.getDsn(); + const client = getCurrentHub().getClient(); + + if (!client) { + return false; + } + + const dsn = client.getDsn(); return dsn ? url.includes(dsn.host) : false; } diff --git a/packages/integration-tests/suites/integrations/httpclient/init.js b/packages/integration-tests/suites/integrations/httpclient/init.js index 61e1fa5f8351..5dbc124bb83b 100644 --- a/packages/integration-tests/suites/integrations/httpclient/init.js +++ b/packages/integration-tests/suites/integrations/httpclient/init.js @@ -4,11 +4,7 @@ window.Sentry = Sentry; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', - integrations: [ - new Sentry.Integrations.HttpClient({ - captureFailedRequests: true, - }), - ], + integrations: [new Sentry.Integrations.HttpClient()], tracesSampleRate: 1, sendDefaultPii: true, }); From e8fb9cd2dae8d7cbc5061bac58f429c460935254 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 4 Jan 2023 17:18:20 +0000 Subject: [PATCH 7/9] Move HTTPClient integration to `@sentry/integrations`. --- packages/browser/src/exports.ts | 2 +- packages/browser/src/integrations/index.ts | 1 - .../suites/integrations/httpclient/init.js | 3 +- .../src}/httpclient.ts | 70 +++++++++++-------- packages/integrations/src/index.ts | 1 + packages/types/src/hub.ts | 6 ++ 6 files changed, 52 insertions(+), 31 deletions(-) rename packages/{browser/src/integrations => integrations/src}/httpclient.ts (88%) diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index d198d921d6ae..ad96e21c4ffe 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -58,4 +58,4 @@ export { winjsStackLineParser, } from './stack-parsers'; export { defaultIntegrations, forceLoad, init, lastEventId, onLoad, showReportDialog, flush, close, wrap } from './sdk'; -export { GlobalHandlers, TryCatch, Breadcrumbs, LinkedErrors, HttpContext, HttpClient, Dedupe } from './integrations'; +export { GlobalHandlers, TryCatch, Breadcrumbs, LinkedErrors, HttpContext, Dedupe } from './integrations'; diff --git a/packages/browser/src/integrations/index.ts b/packages/browser/src/integrations/index.ts index f581d82c4fe5..e029422f363c 100644 --- a/packages/browser/src/integrations/index.ts +++ b/packages/browser/src/integrations/index.ts @@ -4,4 +4,3 @@ export { Breadcrumbs } from './breadcrumbs'; export { LinkedErrors } from './linkederrors'; export { HttpContext } from './httpcontext'; export { Dedupe } from './dedupe'; -export { HttpClient } from './httpclient'; diff --git a/packages/integration-tests/suites/integrations/httpclient/init.js b/packages/integration-tests/suites/integrations/httpclient/init.js index 5dbc124bb83b..5d43b49e75fb 100644 --- a/packages/integration-tests/suites/integrations/httpclient/init.js +++ b/packages/integration-tests/suites/integrations/httpclient/init.js @@ -1,10 +1,11 @@ import * as Sentry from '@sentry/browser'; +import { HttpClient } from '@sentry/integrations'; window.Sentry = Sentry; Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', - integrations: [new Sentry.Integrations.HttpClient()], + integrations: [new HttpClient()], tracesSampleRate: 1, sendDefaultPii: true, }); diff --git a/packages/browser/src/integrations/httpclient.ts b/packages/integrations/src/httpclient.ts similarity index 88% rename from packages/browser/src/integrations/httpclient.ts rename to packages/integrations/src/httpclient.ts index 046c974fa802..8792ca5901fc 100644 --- a/packages/browser/src/integrations/httpclient.ts +++ b/packages/integrations/src/httpclient.ts @@ -1,9 +1,6 @@ -import { captureEvent, getCurrentHub } from '@sentry/core'; -import { Event as SentryEvent, Integration } from '@sentry/types'; +import { Event as SentryEvent, EventProcessor, Hub, Integration } from '@sentry/types'; import { addExceptionMechanism, fill, GLOBAL_OBJ, logger, supportsNativeFetch } from '@sentry/utils'; -import { eventFromUnknownInput } from '../eventbuilder'; - export type HttpStatusCodeRange = [number, number] | number; export type HttpRequestTarget = string | RegExp; interface HttpClientOptions { @@ -41,6 +38,11 @@ export class HttpClient implements Integration { private readonly _options: HttpClientOptions; + /** + * Returns current hub. + */ + private _getCurrentHub?: () => Hub; + /** * @inheritDoc * @@ -59,7 +61,8 @@ export class HttpClient implements Integration { * * @param options */ - public setupOnce(): void { + public setupOnce(_: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { + this._getCurrentHub = getCurrentHub; this._wrapFetch(); this._wrapXHR(); } @@ -72,12 +75,13 @@ export class HttpClient implements Integration { * @param requestInit The request init object */ private _fetchResponseHandler(requestInfo: RequestInfo, response: Response, requestInit?: RequestInit): void { - if (this._shouldCaptureResponse(response.status, response.url)) { + if (this._getCurrentHub && this._shouldCaptureResponse(response.status, response.url)) { const request = new Request(requestInfo, requestInit); + const hub = this._getCurrentHub(); let requestHeaders, responseHeaders, requestCookies, responseCookies; - if (getCurrentHub().shouldSendDefaultPii()) { + if (hub.shouldSendDefaultPii()) { [{ headers: requestHeaders, cookies: requestCookies }, { headers: responseHeaders, cookies: responseCookies }] = [ { cookieHeader: 'Cookie', obj: request }, @@ -113,7 +117,7 @@ export class HttpClient implements Integration { responseCookies, }); - captureEvent(event); + hub.captureEvent(event); } } @@ -125,10 +129,11 @@ export class HttpClient implements Integration { * @param headers The HTTP headers */ private _xhrResponseHandler(xhr: XMLHttpRequest, method: string, headers: Record): void { - if (this._shouldCaptureResponse(xhr.status, xhr.responseURL)) { + if (this._getCurrentHub && this._shouldCaptureResponse(xhr.status, xhr.responseURL)) { let requestHeaders, responseCookies, responseHeaders; + const hub = this._getCurrentHub(); - if (getCurrentHub().shouldSendDefaultPii()) { + if (hub.shouldSendDefaultPii()) { try { const cookieString = xhr.getResponseHeader('Set-Cookie') || xhr.getResponseHeader('set-cookie') || undefined; @@ -158,7 +163,7 @@ export class HttpClient implements Integration { responseCookies, }); - captureEvent(event); + hub.captureEvent(event); } } @@ -362,7 +367,7 @@ export class HttpClient implements Integration { * @param url url to verify */ private _isSentryRequest(url: string): boolean { - const client = getCurrentHub().getClient(); + const client = this._getCurrentHub && this._getCurrentHub().getClient(); if (!client) { return false; @@ -397,22 +402,31 @@ export class HttpClient implements Integration { requestHeaders?: Record; requestCookies?: Record; }): SentryEvent { - const event = eventFromUnknownInput(() => [], `HTTP Client Error with status code: ${data.status}`); - - event.request = { - url: data.url, - method: data.method, - headers: data.requestHeaders, - cookies: data.requestCookies, - }; - - event.contexts = { - ...event.contexts, - response: { - status_code: data.status, - headers: data.responseHeaders, - cookies: data.responseCookies, - body_size: this._getResponseSizeFromHeaders(data.responseHeaders), + const message = `HTTP Client Error with status code: ${data.status}`; + + const event: SentryEvent = { + message, + exception: { + values: [ + { + type: 'Error', + value: message, + }, + ], + }, + request: { + url: data.url, + method: data.method, + headers: data.requestHeaders, + cookies: data.requestCookies, + }, + contexts: { + response: { + status_code: data.status, + headers: data.responseHeaders, + cookies: data.responseCookies, + body_size: this._getResponseSizeFromHeaders(data.responseHeaders), + }, }, }; diff --git a/packages/integrations/src/index.ts b/packages/integrations/src/index.ts index 9a2573ee5a44..2f3708075ac1 100644 --- a/packages/integrations/src/index.ts +++ b/packages/integrations/src/index.ts @@ -7,3 +7,4 @@ export { ReportingObserver } from './reportingobserver'; export { RewriteFrames } from './rewriteframes'; export { SessionTiming } from './sessiontiming'; export { Transaction } from './transaction'; +export { HttpClient } from './httpclient'; diff --git a/packages/types/src/hub.ts b/packages/types/src/hub.ts index a34d4372e08d..555da1ef94ab 100644 --- a/packages/types/src/hub.ts +++ b/packages/types/src/hub.ts @@ -227,4 +227,10 @@ export interface Hub { * @param endSession If set the session will be marked as exited and removed from the scope */ captureSession(endSession?: boolean): void; + + /** + * Returns if default PII should be sent to Sentry and propagated in ourgoing requests + * when Tracing is used. + */ + shouldSendDefaultPii(): boolean; } From c7d13d1b9a59d1ac9862d6b64d3ded32d88344ee Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 4 Jan 2023 22:46:22 +0000 Subject: [PATCH 8/9] Skip `HttpClient` integration tests when in bundle mode. --- .../suites/integrations/httpclient/fetch/test.ts | 6 ++++++ .../suites/integrations/httpclient/xhr/test.ts | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/packages/integration-tests/suites/integrations/httpclient/fetch/test.ts b/packages/integration-tests/suites/integrations/httpclient/fetch/test.ts index d399dd209a84..1d31c868d784 100644 --- a/packages/integration-tests/suites/integrations/httpclient/fetch/test.ts +++ b/packages/integration-tests/suites/integrations/httpclient/fetch/test.ts @@ -7,6 +7,12 @@ import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers'; sentryTest( 'should assign request and response context from a failed 500 fetch request', async ({ getLocalTestPath, page }) => { + // Skipping this test when running in bundle mode, because `@sentry/integrations` bundle + // is not injected to the page with the current test setup. + if (process.env.PW_BUNDLE?.includes('bundle')) { + sentryTest.skip(); + } + const url = await getLocalTestPath({ testDir: __dirname }); await page.route('**/foo', route => { diff --git a/packages/integration-tests/suites/integrations/httpclient/xhr/test.ts b/packages/integration-tests/suites/integrations/httpclient/xhr/test.ts index 72167f73ca2b..7d19da9854f4 100644 --- a/packages/integration-tests/suites/integrations/httpclient/xhr/test.ts +++ b/packages/integration-tests/suites/integrations/httpclient/xhr/test.ts @@ -7,6 +7,12 @@ import { getFirstSentryEnvelopeRequest } from '../../../../utils/helpers'; sentryTest( 'should assign request and response context from a failed 500 XHR request', async ({ getLocalTestPath, page }) => { + // Skipping this test when running in bundle mode, because `@sentry/integrations` bundle + // is not injected to the page with the current test setup. + if (process.env.PW_BUNDLE?.includes('bundle')) { + sentryTest.skip(); + } + const url = await getLocalTestPath({ testDir: __dirname }); await page.route('**/foo', route => { From 84b43888649b34eeeb0ab0a7476196dc1810967a Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 5 Jan 2023 11:37:19 +0000 Subject: [PATCH 9/9] Add `type-check` back to `pretest`. --- packages/integration-tests/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/integration-tests/package.json b/packages/integration-tests/package.json index 017f8809d1ee..6ab725c711ff 100644 --- a/packages/integration-tests/package.json +++ b/packages/integration-tests/package.json @@ -14,7 +14,7 @@ "lint:eslint": "eslint . --cache --cache-location '../../eslintcache/' --format stylish", "lint:prettier": "prettier --check \"{suites,utils}/**/*.ts\"", "type-check": "tsc", - "pretest": "yarn clean", + "pretest": "yarn clean && yarn type-check", "test": "playwright test ./suites", "test:bundle:es5": "PW_BUNDLE=bundle_es5 yarn test", "test:bundle:es5:min": "PW_BUNDLE=bundle_es5_min yarn test",