From 5f65bbe55dc4c3a60845d559e83fb2f3a833dd8f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 21 Nov 2023 14:31:41 +0100 Subject: [PATCH 1/3] fix(replay): Capture JSON XHR response bodies --- .../xhr/captureResponseBody/test.ts | 87 +++++++++++++++++++ .../replay/src/coreHandlers/util/xhrUtils.ts | 54 +++++++++++- 2 files changed, 138 insertions(+), 3 deletions(-) diff --git a/packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/xhr/captureResponseBody/test.ts b/packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/xhr/captureResponseBody/test.ts index 46e20da391cc..12ef0b2a6068 100644 --- a/packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/xhr/captureResponseBody/test.ts +++ b/packages/browser-integration-tests/suites/replay/extendNetworkBreadcrumbs/xhr/captureResponseBody/test.ts @@ -178,6 +178,93 @@ sentryTest('captures JSON response body', async ({ getLocalTestPath, page, brows ]); }); +sentryTest('captures JSON response body when responseType=json', async ({ getLocalTestPath, page, browserName }) => { + // These are a bit flaky on non-chromium browsers + if (shouldSkipReplayTest() || browserName !== 'chromium') { + sentryTest.skip(); + } + + await page.route('**/foo', route => { + return route.fulfill({ + status: 200, + body: JSON.stringify({ res: 'this' }), + headers: { + 'Content-Length': '', + }, + }); + }); + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const requestPromise = waitForErrorRequest(page); + const replayRequestPromise1 = waitForReplayRequest(page, 0); + + const url = await getLocalTestPath({ testDir: __dirname }); + await page.goto(url); + + void page.evaluate(() => { + /* eslint-disable */ + const xhr = new XMLHttpRequest(); + + xhr.open('POST', 'http://localhost:7654/foo'); + // Setting this to json ensures that xhr.response returns a POJO + xhr.responseType = 'json'; + xhr.send(); + + xhr.addEventListener('readystatechange', function () { + if (xhr.readyState === 4) { + // @ts-expect-error Sentry is a global + setTimeout(() => Sentry.captureException('test error', 0)); + } + }); + /* eslint-enable */ + }); + + const request = await requestPromise; + const eventData = envelopeRequestParser(request); + + expect(eventData.exception?.values).toHaveLength(1); + + expect(eventData?.breadcrumbs?.length).toBe(1); + expect(eventData!.breadcrumbs![0]).toEqual({ + timestamp: expect.any(Number), + category: 'xhr', + type: 'http', + data: { + method: 'POST', + response_body_size: 14, + status_code: 200, + url: 'http://localhost:7654/foo', + }, + }); + + const replayReq1 = await replayRequestPromise1; + const { performanceSpans: performanceSpans1 } = getCustomRecordingEvents(replayReq1); + expect(performanceSpans1.filter(span => span.op === 'resource.xhr')).toEqual([ + { + data: { + method: 'POST', + statusCode: 200, + response: { + size: 14, + headers: {}, + body: { res: 'this' }, + }, + }, + description: 'http://localhost:7654/foo', + endTimestamp: expect.any(Number), + op: 'resource.xhr', + startTimestamp: expect.any(Number), + }, + ]); +}); + sentryTest('captures non-text response body', async ({ getLocalTestPath, page, browserName }) => { // These are a bit flaky on non-chromium browsers if (shouldSkipReplayTest() || browserName !== 'chromium') { diff --git a/packages/replay/src/coreHandlers/util/xhrUtils.ts b/packages/replay/src/coreHandlers/util/xhrUtils.ts index fa2345dc04bd..6300507a1c01 100644 --- a/packages/replay/src/coreHandlers/util/xhrUtils.ts +++ b/packages/replay/src/coreHandlers/util/xhrUtils.ts @@ -61,7 +61,7 @@ export function enrichXhrBreadcrumb( const reqSize = getBodySize(input, options.textEncoder); const resSize = xhr.getResponseHeader('content-length') ? parseContentLengthHeader(xhr.getResponseHeader('content-length')) - : getBodySize(xhr.response, options.textEncoder); + : _getBodySize(xhr.response, xhr.responseType, options.textEncoder); if (reqSize !== undefined) { breadcrumb.data.request_body_size = reqSize; @@ -154,8 +154,7 @@ function _getXhrResponseBody(xhr: XMLHttpRequest): [string | undefined, NetworkM // Try to manually parse the response body, if responseText fails try { - const response = xhr.response; - return getBodyString(response); + return _parseXhrResponse(xhr.response, xhr.responseType); } catch (e) { errors.push(e); } @@ -164,3 +163,52 @@ function _getXhrResponseBody(xhr: XMLHttpRequest): [string | undefined, NetworkM return [undefined]; } + +/** + * Get the string representation of the XHR response. + * Based on MDN, these are the possible types of the response: + * string + * ArrayBuffer + * Blob + * Document + * POJO + */ +export function _parseXhrResponse( + body: XMLHttpRequest['response'], + responseType: XMLHttpRequest['responseType'], +): [string | undefined, NetworkMetaWarning?] { + logger.log(body, responseType, typeof body); + try { + if (typeof body === 'string') { + return [body]; + } + + if (body instanceof Document) { + return [body.body.outerHTML]; + } + + if (responseType === 'json' && body && typeof body === 'object') { + return [JSON.stringify(body)]; + } + } catch { + __DEBUG_BUILD__ && logger.warn('[Replay] Failed to serialize body', body); + return [undefined, 'BODY_PARSE_ERROR']; + } + + __DEBUG_BUILD__ && logger.info('[Replay] Skipping network body because of body type', body); + + return [undefined]; +} + +function _getBodySize( + body: XMLHttpRequest['response'], + responseType: XMLHttpRequest['responseType'], + textEncoder: TextEncoder | TextEncoderInternal, +): number | undefined { + try { + const bodyStr = responseType === 'json' && body && typeof body === 'object' ? JSON.stringify(body) : body; + return getBodySize(bodyStr, textEncoder); + } catch { + return undefined; + } +} From 82f8154883df1347e671451270d2eb61788bf03e Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 22 Nov 2023 10:00:26 +0100 Subject: [PATCH 2/3] add further warning & tests --- .../src/coreHandlers/util/networkUtils.ts | 6 +- .../replay/src/coreHandlers/util/xhrUtils.ts | 9 +- packages/replay/src/types/request.ts | 7 +- .../unit/coreHandlers/util/fetchUtils.test.ts | 226 +++++++++--------- .../coreHandlers/util/networkUtils.test.ts | 36 +++ .../unit/coreHandlers/util/xhrUtils.test.ts | 38 +++ 6 files changed, 206 insertions(+), 116 deletions(-) create mode 100644 packages/replay/test/unit/coreHandlers/util/xhrUtils.test.ts diff --git a/packages/replay/src/coreHandlers/util/networkUtils.ts b/packages/replay/src/coreHandlers/util/networkUtils.ts index 5037d5a6769c..29d9684456ca 100644 --- a/packages/replay/src/coreHandlers/util/networkUtils.ts +++ b/packages/replay/src/coreHandlers/util/networkUtils.ts @@ -75,6 +75,10 @@ export function getBodyString(body: unknown): [string | undefined, NetworkMetaWa if (body instanceof FormData) { return [_serializeFormData(body)]; } + + if (!body) { + return [undefined]; + } } catch { DEBUG_BUILD && logger.warn('[Replay] Failed to serialize body', body); return [undefined, 'BODY_PARSE_ERROR']; @@ -82,7 +86,7 @@ export function getBodyString(body: unknown): [string | undefined, NetworkMetaWa DEBUG_BUILD && logger.info('[Replay] Skipping network body because of body type', body); - return [undefined]; + return [undefined, 'UNPARSEABLE_BODY_TYPE']; } /** Merge a warning into an existing network request/response. */ diff --git a/packages/replay/src/coreHandlers/util/xhrUtils.ts b/packages/replay/src/coreHandlers/util/xhrUtils.ts index 6300507a1c01..8148dca70dbf 100644 --- a/packages/replay/src/coreHandlers/util/xhrUtils.ts +++ b/packages/replay/src/coreHandlers/util/xhrUtils.ts @@ -172,12 +172,13 @@ function _getXhrResponseBody(xhr: XMLHttpRequest): [string | undefined, NetworkM * Blob * Document * POJO + * + * Exported only for tests. */ export function _parseXhrResponse( body: XMLHttpRequest['response'], responseType: XMLHttpRequest['responseType'], ): [string | undefined, NetworkMetaWarning?] { - logger.log(body, responseType, typeof body); try { if (typeof body === 'string') { return [body]; @@ -190,6 +191,10 @@ export function _parseXhrResponse( if (responseType === 'json' && body && typeof body === 'object') { return [JSON.stringify(body)]; } + + if (!body) { + return [undefined]; + } } catch { __DEBUG_BUILD__ && logger.warn('[Replay] Failed to serialize body', body); return [undefined, 'BODY_PARSE_ERROR']; @@ -197,7 +202,7 @@ export function _parseXhrResponse( __DEBUG_BUILD__ && logger.info('[Replay] Skipping network body because of body type', body); - return [undefined]; + return [undefined, 'UNPARSEABLE_BODY_TYPE']; } function _getBodySize( diff --git a/packages/replay/src/types/request.ts b/packages/replay/src/types/request.ts index 03067596fb36..60c25a55ce44 100644 --- a/packages/replay/src/types/request.ts +++ b/packages/replay/src/types/request.ts @@ -3,7 +3,12 @@ type JsonArray = unknown[]; export type NetworkBody = JsonObject | JsonArray | string; -export type NetworkMetaWarning = 'MAYBE_JSON_TRUNCATED' | 'TEXT_TRUNCATED' | 'URL_SKIPPED' | 'BODY_PARSE_ERROR'; +export type NetworkMetaWarning = + | 'MAYBE_JSON_TRUNCATED' + | 'TEXT_TRUNCATED' + | 'URL_SKIPPED' + | 'BODY_PARSE_ERROR' + | 'UNPARSEABLE_BODY_TYPE'; interface NetworkMeta { warnings?: NetworkMetaWarning[]; diff --git a/packages/replay/test/unit/coreHandlers/util/fetchUtils.test.ts b/packages/replay/test/unit/coreHandlers/util/fetchUtils.test.ts index 82a6b66eeb7d..123ec85fe887 100644 --- a/packages/replay/test/unit/coreHandlers/util/fetchUtils.test.ts +++ b/packages/replay/test/unit/coreHandlers/util/fetchUtils.test.ts @@ -2,136 +2,138 @@ import { TextEncoder } from 'util'; import { _getResponseInfo } from '../../../../src/coreHandlers/util/fetchUtils'; -describe('_getResponseInfo', () => { - it('works with captureDetails: false', async () => { - const res = await _getResponseInfo( - false, - { - networkCaptureBodies: true, - textEncoder: new TextEncoder(), - networkResponseHeaders: [], - }, - undefined, - undefined, - ); +describe('Unit | coreHandlers | util | fetchUtils', () => { + describe('_getResponseInfo', () => { + it('works with captureDetails: false', async () => { + const res = await _getResponseInfo( + false, + { + networkCaptureBodies: true, + textEncoder: new TextEncoder(), + networkResponseHeaders: [], + }, + undefined, + undefined, + ); - expect(res).toEqual(undefined); - }); + expect(res).toEqual(undefined); + }); - it('works with captureDetails: false & responseBodySize', async () => { - const res = await _getResponseInfo( - false, - { - networkCaptureBodies: true, - textEncoder: new TextEncoder(), - networkResponseHeaders: [], - }, - undefined, - 123, - ); + it('works with captureDetails: false & responseBodySize', async () => { + const res = await _getResponseInfo( + false, + { + networkCaptureBodies: true, + textEncoder: new TextEncoder(), + networkResponseHeaders: [], + }, + undefined, + 123, + ); - expect(res).toEqual({ - headers: {}, - size: 123, - _meta: { - warnings: ['URL_SKIPPED'], - }, + expect(res).toEqual({ + headers: {}, + size: 123, + _meta: { + warnings: ['URL_SKIPPED'], + }, + }); }); - }); - it('works with text body', async () => { - const response = { - headers: { - has: () => { - return false; - }, - get: () => { - return undefined; + it('works with text body', async () => { + const response = { + headers: { + has: () => { + return false; + }, + get: () => { + return undefined; + }, }, - }, - clone: () => response, - text: () => Promise.resolve('text body'), - } as unknown as Response; + clone: () => response, + text: () => Promise.resolve('text body'), + } as unknown as Response; - const res = await _getResponseInfo( - true, - { - networkCaptureBodies: true, - textEncoder: new TextEncoder(), - networkResponseHeaders: [], - }, - response, - undefined, - ); + const res = await _getResponseInfo( + true, + { + networkCaptureBodies: true, + textEncoder: new TextEncoder(), + networkResponseHeaders: [], + }, + response, + undefined, + ); - expect(res).toEqual({ - headers: {}, - size: 9, - body: 'text body', + expect(res).toEqual({ + headers: {}, + size: 9, + body: 'text body', + }); }); - }); - it('works with body that fails', async () => { - const response = { - headers: { - has: () => { - return false; + it('works with body that fails', async () => { + const response = { + headers: { + has: () => { + return false; + }, + get: () => { + return undefined; + }, }, - get: () => { - return undefined; - }, - }, - clone: () => response, - text: () => Promise.reject('cannot read'), - } as unknown as Response; + clone: () => response, + text: () => Promise.reject('cannot read'), + } as unknown as Response; - const res = await _getResponseInfo( - true, - { - networkCaptureBodies: true, - textEncoder: new TextEncoder(), - networkResponseHeaders: [], - }, - response, - undefined, - ); + const res = await _getResponseInfo( + true, + { + networkCaptureBodies: true, + textEncoder: new TextEncoder(), + networkResponseHeaders: [], + }, + response, + undefined, + ); - expect(res).toEqual({ - _meta: { warnings: ['BODY_PARSE_ERROR'] }, - headers: {}, - size: undefined, + expect(res).toEqual({ + _meta: { warnings: ['BODY_PARSE_ERROR'] }, + headers: {}, + size: undefined, + }); }); - }); - it('works with body that times out', async () => { - const response = { - headers: { - has: () => { - return false; - }, - get: () => { - return undefined; + it('works with body that times out', async () => { + const response = { + headers: { + has: () => { + return false; + }, + get: () => { + return undefined; + }, }, - }, - clone: () => response, - text: () => new Promise(resolve => setTimeout(() => resolve('text body'), 1000)), - } as unknown as Response; + clone: () => response, + text: () => new Promise(resolve => setTimeout(() => resolve('text body'), 1000)), + } as unknown as Response; - const res = await _getResponseInfo( - true, - { - networkCaptureBodies: true, - textEncoder: new TextEncoder(), - networkResponseHeaders: [], - }, - response, - undefined, - ); + const res = await _getResponseInfo( + true, + { + networkCaptureBodies: true, + textEncoder: new TextEncoder(), + networkResponseHeaders: [], + }, + response, + undefined, + ); - expect(res).toEqual({ - _meta: { warnings: ['BODY_PARSE_ERROR'] }, - headers: {}, - size: undefined, + expect(res).toEqual({ + _meta: { warnings: ['BODY_PARSE_ERROR'] }, + headers: {}, + size: undefined, + }); }); }); }); diff --git a/packages/replay/test/unit/coreHandlers/util/networkUtils.test.ts b/packages/replay/test/unit/coreHandlers/util/networkUtils.test.ts index f00acad6384d..fccbf24a0a23 100644 --- a/packages/replay/test/unit/coreHandlers/util/networkUtils.test.ts +++ b/packages/replay/test/unit/coreHandlers/util/networkUtils.test.ts @@ -4,6 +4,7 @@ import { NETWORK_BODY_MAX_SIZE } from '../../../../src/constants'; import { buildNetworkRequestOrResponse, getBodySize, + getBodyString, getFullUrl, parseContentLengthHeader, } from '../../../../src/coreHandlers/util/networkUtils'; @@ -248,4 +249,39 @@ describe('Unit | coreHandlers | util | networkUtils', () => { expect(actual).toBe(expected); }); }); + + describe('getBodyString', () => { + it('works with a string', () => { + const actual = getBodyString('abc'); + expect(actual).toEqual(['abc']); + }); + + it('works with URLSearchParams', () => { + const body = new URLSearchParams(); + body.append('name', 'Anne'); + body.append('age', '32'); + const actual = getBodyString(body); + expect(actual).toEqual(['name=Anne&age=32']); + }); + + it('works with FormData', () => { + const body = new FormData(); + body.append('name', 'Anne'); + body.append('age', '32'); + const actual = getBodyString(body); + expect(actual).toEqual(['name=Anne&age=32']); + }); + + it('works with empty data', () => { + const body = undefined; + const actual = getBodyString(body); + expect(actual).toEqual([undefined]); + }); + + it('works with other type of data', () => { + const body = {}; + const actual = getBodyString(body); + expect(actual).toEqual([undefined, 'UNPARSEABLE_BODY_TYPE']); + }); + }); }); diff --git a/packages/replay/test/unit/coreHandlers/util/xhrUtils.test.ts b/packages/replay/test/unit/coreHandlers/util/xhrUtils.test.ts new file mode 100644 index 000000000000..d39b473f317f --- /dev/null +++ b/packages/replay/test/unit/coreHandlers/util/xhrUtils.test.ts @@ -0,0 +1,38 @@ +import { _parseXhrResponse } from '../../../../src/coreHandlers/util/xhrUtils'; + +describe('Unit | coreHandlers | util | xhrUtils', () => { + describe('_parseXhrResponse', () => { + it('works with a string', () => { + const actual = _parseXhrResponse('abc', ''); + expect(actual).toEqual(['abc']); + }); + + it('works with a Document', () => { + const body = document.implementation.createHTMLDocument(); + const bodyEl = document.createElement('body'); + bodyEl.innerHTML = '
abc
'; + body.body = bodyEl; + + const actual = _parseXhrResponse(body, ''); + expect(actual).toEqual(['
abc
']); + }); + + it('works with empty data', () => { + const body = undefined; + const actual = _parseXhrResponse(body, ''); + expect(actual).toEqual([undefined]); + }); + + it('works with other type of data', () => { + const body = {}; + const actual = _parseXhrResponse(body, ''); + expect(actual).toEqual([undefined, 'UNPARSEABLE_BODY_TYPE']); + }); + + it('works with JSON data', () => { + const body = {}; + const actual = _parseXhrResponse(body, 'json'); + expect(actual).toEqual(['{}']); + }); + }); +}); From 70a61c40b7efd53870f691833d7dcaffa3fe2b84 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 4 Dec 2023 15:14:59 +0100 Subject: [PATCH 3/3] fix PR --- packages/replay/src/coreHandlers/util/xhrUtils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/replay/src/coreHandlers/util/xhrUtils.ts b/packages/replay/src/coreHandlers/util/xhrUtils.ts index 8148dca70dbf..b422d3bd1e81 100644 --- a/packages/replay/src/coreHandlers/util/xhrUtils.ts +++ b/packages/replay/src/coreHandlers/util/xhrUtils.ts @@ -196,11 +196,11 @@ export function _parseXhrResponse( return [undefined]; } } catch { - __DEBUG_BUILD__ && logger.warn('[Replay] Failed to serialize body', body); + DEBUG_BUILD && logger.warn('[Replay] Failed to serialize body', body); return [undefined, 'BODY_PARSE_ERROR']; } - __DEBUG_BUILD__ && logger.info('[Replay] Skipping network body because of body type', body); + DEBUG_BUILD && logger.info('[Replay] Skipping network body because of body type', body); return [undefined, 'UNPARSEABLE_BODY_TYPE']; }