Skip to content

Commit 67e2971

Browse files
Zen-cronicZen-cronic
committed
fix(browser): Change parsing the fetch req payload to allow more types other than string
- Moved internal `getFetchRequestArgBody` to `browser-utils`. - Added unit tests. Signed-off-by: Kaung Zin Hein <[email protected]>
1 parent 4d06a2f commit 67e2971

File tree

9 files changed

+86
-105
lines changed

9 files changed

+86
-105
lines changed

‎packages/browser-utils/src/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,6 @@ export { fetch, setTimeout, clearCachedImplementation, getNativeImplementation }
2727

2828
export { addXhrInstrumentationHandler, SENTRY_XHR_DATA_KEY } from './instrument/xhr';
2929

30-
export { getBodyString } from './networkUtils';
30+
export { getBodyString, getFetchRequestArgBody } from './networkUtils';
3131

3232
export type { FetchHint, NetworkMetaWarning, XhrHint } from './types';

‎packages/browser-utils/src/networkUtils.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,3 +70,15 @@ export function getBodyString(
7070

7171
return [undefined, 'UNPARSEABLE_BODY_TYPE'];
7272
}
73+
74+
/**
75+
* Parses the fetch arguments to extract the request payload.
76+
* We only support getting the body from the fetch options.
77+
*/
78+
export function getFetchRequestArgBody(fetchArgs: unknown[] = []): RequestInit['body'] | undefined {
79+
if (fetchArgs.length !== 2 || typeof fetchArgs[1] !== 'object') {
80+
return undefined;
81+
}
82+
83+
return (fetchArgs[1] as RequestInit).body;
84+
}

‎packages/browser-utils/test/networkUtils.test.ts‎

Lines changed: 67 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,39 +3,98 @@
33
*/
44

55
import { describe, expect, it } from 'vitest';
6-
import { getBodyString } from '../src/networkUtils';
6+
import { getBodyString, getFetchRequestArgBody } from '../src/networkUtils';
77

88
describe('getBodyString', () => {
9-
it('works with a string', () => {
9+
it('should work with a string', () => {
1010
const actual = getBodyString('abc');
1111
expect(actual).toEqual(['abc']);
1212
});
1313

14-
it('works with URLSearchParams', () => {
14+
it('should work with URLSearchParams', () => {
1515
const body = new URLSearchParams();
1616
body.append('name', 'Anne');
1717
body.append('age', '32');
1818
const actual = getBodyString(body);
1919
expect(actual).toEqual(['name=Anne&age=32']);
2020
});
2121

22-
it('works with FormData', () => {
22+
it('should work with FormData', () => {
2323
const body = new FormData();
24-
body.append('name', 'Anne');
24+
body.append('name', 'Bob');
2525
body.append('age', '32');
2626
const actual = getBodyString(body);
27-
expect(actual).toEqual(['name=Anne&age=32']);
27+
expect(actual).toEqual(['name=Bob&age=32']);
2828
});
2929

30-
it('works with empty data', () => {
30+
it('should work with empty data', () => {
3131
const body = undefined;
3232
const actual = getBodyString(body);
3333
expect(actual).toEqual([undefined]);
3434
});
3535

36-
it('works with other type of data', () => {
36+
it('should return unparsable with other types of data', () => {
3737
const body = {};
3838
const actual = getBodyString(body);
3939
expect(actual).toEqual([undefined, 'UNPARSEABLE_BODY_TYPE']);
4040
});
4141
});
42+
43+
describe('getFetchRequestArgBody', () => {
44+
describe('valid types of body', () => {
45+
it('should work with json string', () => {
46+
const body = { data: [1, 2, 3] };
47+
const jsonBody = JSON.stringify(body);
48+
49+
const actual = getFetchRequestArgBody(['http://example.com', { method: 'POST', body: jsonBody }]);
50+
expect(actual).toEqual(jsonBody);
51+
});
52+
53+
it('should work with URLSearchParams', () => {
54+
const body = new URLSearchParams();
55+
body.append('name', 'Anne');
56+
body.append('age', '32');
57+
58+
const actual = getFetchRequestArgBody(['http://example.com', { method: 'POST', body }]);
59+
expect(actual).toEqual(body);
60+
});
61+
62+
it('should work with FormData', () => {
63+
const body = new FormData();
64+
body.append('name', 'Bob');
65+
body.append('age', '32');
66+
67+
const actual = getFetchRequestArgBody(['http://example.com', { method: 'POST', body }]);
68+
expect(actual).toEqual(body);
69+
});
70+
71+
it('should work with Blob', () => {
72+
const body = new Blob(['example'], { type: 'text/plain' });
73+
const actual = getFetchRequestArgBody(['http://example.com', { method: 'POST', body }]);
74+
expect(actual).toEqual(body);
75+
});
76+
77+
it('should work with BufferSource (ArrayBufferView | ArrayBuffer)', () => {
78+
const body = new Uint8Array([1, 2, 3]);
79+
const actual = getFetchRequestArgBody(['http://example.com', { method: 'POST', body }]);
80+
expect(actual).toEqual(body);
81+
});
82+
});
83+
84+
describe('does not work without body passed as the second option', () => {
85+
it.each([
86+
['string URL only', ['http://example.com'], undefined],
87+
['URL object only', [new URL('http://example.com')], undefined],
88+
['Request URL only', [{ url: 'http://example.com' }], undefined],
89+
[
90+
'body in first arg',
91+
[{ url: 'http://example.com', method: 'POST', body: JSON.stringify({ data: [1, 2, 3] }) }],
92+
undefined,
93+
],
94+
])('%s', (_name, args, expected) => {
95+
const actual = getFetchRequestArgBody(args);
96+
97+
expect(actual).toEqual(expected);
98+
});
99+
});
100+
});

‎packages/browser/src/integrations/graphqlClient.ts‎

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
1-
import { SENTRY_XHR_DATA_KEY, getBodyString } from '@sentry-internal/browser-utils';
1+
import { SENTRY_XHR_DATA_KEY, getBodyString, getFetchRequestArgBody } from '@sentry-internal/browser-utils';
22
import type { FetchHint, XhrHint } from '@sentry-internal/browser-utils';
33
import {
44
SEMANTIC_ATTRIBUTE_HTTP_REQUEST_METHOD,
55
SEMANTIC_ATTRIBUTE_SENTRY_OP,
66
SEMANTIC_ATTRIBUTE_URL_FULL,
77
defineIntegration,
8-
hasProp,
98
isString,
109
spanToJSON,
1110
stringMatchesSomePattern,
@@ -131,27 +130,13 @@ export function getRequestPayloadXhrOrFetch(hint: XhrHint | FetchHint): string |
131130
const sentryXhrData = hint.xhr[SENTRY_XHR_DATA_KEY];
132131
body = sentryXhrData && getBodyString(sentryXhrData.body)[0];
133132
} else {
134-
const sentryFetchData = parseFetchPayload(hint.input);
133+
const sentryFetchData = getFetchRequestArgBody(hint.input);
135134
body = getBodyString(sentryFetchData)[0];
136135
}
137136

138137
return body;
139138
}
140139

141-
/**
142-
* Parses the fetch arguments to extract the request payload.
143-
* Exported for tests only.
144-
*/
145-
export function parseFetchPayload(fetchArgs: unknown[]): string | undefined {
146-
if (fetchArgs.length === 2) {
147-
const options = fetchArgs[1];
148-
return hasProp(options, 'body') ? String(options.body) : undefined;
149-
}
150-
151-
const arg = fetchArgs[0];
152-
return hasProp(arg, 'body') ? String(arg.body) : undefined;
153-
}
154-
155140
/**
156141
* Extract the name and type of the operation from the GraphQL query.
157142
* Exported for tests only.

‎packages/browser/test/integrations/graphqlClient.test.ts‎

Lines changed: 1 addition & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -5,46 +5,14 @@
55
import { describe, expect, test } from 'vitest';
66

77
import { SENTRY_XHR_DATA_KEY } from '@sentry-internal/browser-utils';
8-
import type { FetchHint, XhrHint } from '@sentry-internal/replay';
8+
import type { FetchHint, XhrHint } from '@sentry-internal/browser-utils';
99
import {
1010
getGraphQLRequestPayload,
1111
getRequestPayloadXhrOrFetch,
12-
parseFetchPayload,
1312
parseGraphQLQuery,
1413
} from '../../src/integrations/graphqlClient';
1514

1615
describe('GraphqlClient', () => {
17-
describe('parseFetchPayload', () => {
18-
const data = [1, 2, 3];
19-
const jsonData = '{"data":[1,2,3]}';
20-
21-
test.each([
22-
['string URL only', ['http://example.com'], undefined],
23-
['URL object only', [new URL('http://example.com')], undefined],
24-
['Request URL only', [{ url: 'http://example.com' }], undefined],
25-
[
26-
'Request URL & method only',
27-
[{ url: 'http://example.com', method: 'post', body: JSON.stringify({ data }) }],
28-
jsonData,
29-
],
30-
['string URL & options', ['http://example.com', { method: 'post', body: JSON.stringify({ data }) }], jsonData],
31-
[
32-
'URL object & options',
33-
[new URL('http://example.com'), { method: 'post', body: JSON.stringify({ data }) }],
34-
jsonData,
35-
],
36-
[
37-
'Request URL & options',
38-
[{ url: 'http://example.com' }, { method: 'post', body: JSON.stringify({ data }) }],
39-
jsonData,
40-
],
41-
])('%s', (_name, args, expected) => {
42-
const actual = parseFetchPayload(args as unknown[]);
43-
44-
expect(actual).toEqual(expected);
45-
});
46-
});
47-
4816
describe('parseGraphQLQuery', () => {
4917
const queryOne = `query Test {
5018
items {

‎packages/core/src/index.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ export {
9696
extractQueryParamsFromUrl,
9797
headersToDict,
9898
} from './utils/request';
99-
export { hasProp } from './utils/hasProp';
10099
export { DEFAULT_ENVIRONMENT } from './constants';
101100
export { addBreadcrumb } from './breadcrumbs';
102101
export { functionToStringIntegration } from './integrations/functiontostring';

‎packages/core/src/utils/hasProp.ts‎

Lines changed: 0 additions & 6 deletions
This file was deleted.

‎packages/core/test/lib/utils/hasProp.test.ts‎

Lines changed: 0 additions & 27 deletions
This file was deleted.

‎packages/replay-internal/src/coreHandlers/util/fetchUtils.ts‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { getBodyString, setTimeout } from '@sentry-internal/browser-utils';
1+
import { getBodyString, getFetchRequestArgBody, setTimeout } from '@sentry-internal/browser-utils';
22
import type { FetchHint, NetworkMetaWarning } from '@sentry-internal/browser-utils';
33
import type { Breadcrumb, FetchBreadcrumbData } from '@sentry/core';
44

@@ -55,7 +55,7 @@ export function enrichFetchBreadcrumb(
5555
): void {
5656
const { input, response } = hint;
5757

58-
const body = input ? _getFetchRequestArgBody(input) : undefined;
58+
const body = input ? getFetchRequestArgBody(input) : undefined;
5959
const reqSize = getBodySize(body);
6060

6161
const resSize = response ? parseContentLengthHeader(response.headers.get('content-length')) : undefined;
@@ -115,7 +115,7 @@ function _getRequestInfo(
115115
}
116116

117117
// We only want to transmit string or string-like bodies
118-
const requestBody = _getFetchRequestArgBody(input);
118+
const requestBody = getFetchRequestArgBody(input);
119119
const [bodyStr, warning] = getBodyString(requestBody, logger);
120120
const data = buildNetworkRequestOrResponse(headers, requestBodySize, bodyStr);
121121

@@ -216,15 +216,6 @@ async function _parseFetchResponseBody(response: Response): Promise<[string | un
216216
}
217217
}
218218

219-
function _getFetchRequestArgBody(fetchArgs: unknown[] = []): RequestInit['body'] | undefined {
220-
// We only support getting the body from the fetch options
221-
if (fetchArgs.length !== 2 || typeof fetchArgs[1] !== 'object') {
222-
return undefined;
223-
}
224-
225-
return (fetchArgs[1] as RequestInit).body;
226-
}
227-
228219
function getAllHeaders(headers: Headers, allowedHeaders: string[]): Record<string, string> {
229220
const allHeaders: Record<string, string> = {};
230221

0 commit comments

Comments
 (0)