Skip to content

Commit 68bb820

Browse files
authored
fix(replay): Add BODY_PARSE_ERROR warning & time out fetch response load (#9622)
This is a tricky one 😬 Basically, it is possible that fetch returns a readable stream that is ongoing. So if we do `await response.text()` this will be pending forever, if a stream is ongoing. I haven't found a way to really check that is the case and avoid parsing this at all 🤔 So the best I could come up with was to instead add a timeout of 500ms when we stop waiting for the fetch body. This should at least unblock waiting on this, but it will still mean that the response continues to be parsed client-side - I don't think there is a way to abort this 🤔 Additionally, this also refactors this a bit so we add a new `BODY_PARSE_ERROR` meta warning if the parsing of the body fails, for whatever reason. we may also use this in the replay UI cc @ryan953 somehow? "fixes" #9616
1 parent 392110c commit 68bb820

File tree

6 files changed

+277
-27
lines changed

6 files changed

+277
-27
lines changed

packages/replay/src/coreHandlers/util/fetchUtils.ts

+93-13
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { logger } from '@sentry/utils';
33

44
import type {
55
FetchHint,
6+
NetworkMetaWarning,
67
ReplayContainer,
78
ReplayNetworkOptions,
89
ReplayNetworkRequestData,
@@ -16,6 +17,7 @@ import {
1617
getBodySize,
1718
getBodyString,
1819
makeNetworkReplayBreadcrumb,
20+
mergeWarning,
1921
parseContentLengthHeader,
2022
urlMatches,
2123
} from './networkUtils';
@@ -118,17 +120,24 @@ function _getRequestInfo(
118120

119121
// We only want to transmit string or string-like bodies
120122
const requestBody = _getFetchRequestArgBody(input);
121-
const bodyStr = getBodyString(requestBody);
122-
return buildNetworkRequestOrResponse(headers, requestBodySize, bodyStr);
123+
const [bodyStr, warning] = getBodyString(requestBody);
124+
const data = buildNetworkRequestOrResponse(headers, requestBodySize, bodyStr);
125+
126+
if (warning) {
127+
return mergeWarning(data, warning);
128+
}
129+
130+
return data;
123131
}
124132

125-
async function _getResponseInfo(
133+
/** Exported only for tests. */
134+
export async function _getResponseInfo(
126135
captureDetails: boolean,
127136
{
128137
networkCaptureBodies,
129138
textEncoder,
130139
networkResponseHeaders,
131-
}: ReplayNetworkOptions & {
140+
}: Pick<ReplayNetworkOptions, 'networkCaptureBodies' | 'networkResponseHeaders'> & {
132141
textEncoder: TextEncoderInternal;
133142
},
134143
response: Response | undefined,
@@ -144,12 +153,39 @@ async function _getResponseInfo(
144153
return buildNetworkRequestOrResponse(headers, responseBodySize, undefined);
145154
}
146155

147-
// Only clone the response if we need to
148-
try {
149-
// We have to clone this, as the body can only be read once
150-
const res = response.clone();
151-
const bodyText = await _parseFetchBody(res);
156+
const [bodyText, warning] = await _parseFetchResponseBody(response);
157+
const result = getResponseData(bodyText, {
158+
networkCaptureBodies,
159+
textEncoder,
160+
responseBodySize,
161+
captureDetails,
162+
headers,
163+
});
164+
165+
if (warning) {
166+
return mergeWarning(result, warning);
167+
}
152168

169+
return result;
170+
}
171+
172+
function getResponseData(
173+
bodyText: string | undefined,
174+
{
175+
networkCaptureBodies,
176+
textEncoder,
177+
responseBodySize,
178+
captureDetails,
179+
headers,
180+
}: {
181+
captureDetails: boolean;
182+
networkCaptureBodies: boolean;
183+
responseBodySize: number | undefined;
184+
headers: Record<string, string>;
185+
textEncoder: TextEncoderInternal;
186+
},
187+
): ReplayNetworkRequestOrResponse | undefined {
188+
try {
153189
const size =
154190
bodyText && bodyText.length && responseBodySize === undefined
155191
? getBodySize(bodyText, textEncoder)
@@ -171,11 +207,19 @@ async function _getResponseInfo(
171207
}
172208
}
173209

174-
async function _parseFetchBody(response: Response): Promise<string | undefined> {
210+
async function _parseFetchResponseBody(response: Response): Promise<[string | undefined, NetworkMetaWarning?]> {
211+
const res = _tryCloneResponse(response);
212+
213+
if (!res) {
214+
return [undefined, 'BODY_PARSE_ERROR'];
215+
}
216+
175217
try {
176-
return await response.text();
177-
} catch {
178-
return undefined;
218+
const text = await _tryGetResponseText(res);
219+
return [text];
220+
} catch (error) {
221+
__DEBUG_BUILD__ && logger.warn('[Replay] Failed to get text body from response', error);
222+
return [undefined, 'BODY_PARSE_ERROR'];
179223
}
180224
}
181225

@@ -237,3 +281,39 @@ function getHeadersFromOptions(
237281

238282
return getAllowedHeaders(headers, allowedHeaders);
239283
}
284+
285+
function _tryCloneResponse(response: Response): Response | void {
286+
try {
287+
// We have to clone this, as the body can only be read once
288+
return response.clone();
289+
} catch (error) {
290+
// this can throw if the response was already consumed before
291+
__DEBUG_BUILD__ && logger.warn('[Replay] Failed to clone response body', error);
292+
}
293+
}
294+
295+
/**
296+
* Get the response body of a fetch request, or timeout after 500ms.
297+
* Fetch can return a streaming body, that may not resolve (or not for a long time).
298+
* If that happens, we rather abort after a short time than keep waiting for this.
299+
*/
300+
function _tryGetResponseText(response: Response): Promise<string | undefined> {
301+
return new Promise((resolve, reject) => {
302+
const timeout = setTimeout(() => reject(new Error('Timeout while trying to read response body')), 500);
303+
304+
_getResponseText(response)
305+
.then(
306+
txt => resolve(txt),
307+
reason => reject(reason),
308+
)
309+
.finally(() => clearTimeout(timeout));
310+
});
311+
312+
return _getResponseText(response);
313+
}
314+
315+
async function _getResponseText(response: Response): Promise<string> {
316+
// Force this to be a promise, just to be safe
317+
// eslint-disable-next-line no-return-await
318+
return await response.text();
319+
}

packages/replay/src/coreHandlers/util/networkUtils.ts

+29-5
Original file line numberDiff line numberDiff line change
@@ -61,26 +61,50 @@ export function parseContentLengthHeader(header: string | null | undefined): num
6161
}
6262

6363
/** Get the string representation of a body. */
64-
export function getBodyString(body: unknown): string | undefined {
64+
export function getBodyString(body: unknown): [string | undefined, NetworkMetaWarning?] {
6565
try {
6666
if (typeof body === 'string') {
67-
return body;
67+
return [body];
6868
}
6969

7070
if (body instanceof URLSearchParams) {
71-
return body.toString();
71+
return [body.toString()];
7272
}
7373

7474
if (body instanceof FormData) {
75-
return _serializeFormData(body);
75+
return [_serializeFormData(body)];
7676
}
7777
} catch {
7878
__DEBUG_BUILD__ && logger.warn('[Replay] Failed to serialize body', body);
79+
return [undefined, 'BODY_PARSE_ERROR'];
7980
}
8081

8182
__DEBUG_BUILD__ && logger.info('[Replay] Skipping network body because of body type', body);
8283

83-
return undefined;
84+
return [undefined];
85+
}
86+
87+
/** Merge a warning into an existing network request/response. */
88+
export function mergeWarning(
89+
info: ReplayNetworkRequestOrResponse | undefined,
90+
warning: NetworkMetaWarning,
91+
): ReplayNetworkRequestOrResponse {
92+
if (!info) {
93+
return {
94+
headers: {},
95+
size: undefined,
96+
_meta: {
97+
warnings: [warning],
98+
},
99+
};
100+
}
101+
102+
const newMeta = { ...info._meta };
103+
const existingWarnings = newMeta.warnings || [];
104+
newMeta.warnings = [...existingWarnings, warning];
105+
106+
info._meta = newMeta;
107+
return info;
84108
}
85109

86110
/** Convert ReplayNetworkRequestData to a PerformanceEntry. */

packages/replay/src/coreHandlers/util/xhrUtils.ts

+15-8
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
import type { Breadcrumb, TextEncoderInternal, XhrBreadcrumbData } from '@sentry/types';
22
import { logger, SENTRY_XHR_DATA_KEY } from '@sentry/utils';
33

4-
import type { ReplayContainer, ReplayNetworkOptions, ReplayNetworkRequestData, XhrHint } from '../../types';
4+
import type {
5+
NetworkMetaWarning,
6+
ReplayContainer,
7+
ReplayNetworkOptions,
8+
ReplayNetworkRequestData,
9+
XhrHint,
10+
} from '../../types';
511
import { addNetworkBreadcrumb } from './addNetworkBreadcrumb';
612
import {
713
buildNetworkRequestOrResponse,
@@ -10,6 +16,7 @@ import {
1016
getBodySize,
1117
getBodyString,
1218
makeNetworkReplayBreadcrumb,
19+
mergeWarning,
1320
parseContentLengthHeader,
1421
urlMatches,
1522
} from './networkUtils';
@@ -103,8 +110,8 @@ function _prepareXhrData(
103110
: {};
104111
const networkResponseHeaders = getAllowedHeaders(getResponseHeaders(xhr), options.networkResponseHeaders);
105112

106-
const requestBody = options.networkCaptureBodies ? getBodyString(input) : undefined;
107-
const responseBody = options.networkCaptureBodies ? _getXhrResponseBody(xhr) : undefined;
113+
const [requestBody, requestWarning] = options.networkCaptureBodies ? getBodyString(input) : [undefined];
114+
const [responseBody, responseWarning] = options.networkCaptureBodies ? _getXhrResponseBody(xhr) : [undefined];
108115

109116
const request = buildNetworkRequestOrResponse(networkRequestHeaders, requestBodySize, requestBody);
110117
const response = buildNetworkRequestOrResponse(networkResponseHeaders, responseBodySize, responseBody);
@@ -115,8 +122,8 @@ function _prepareXhrData(
115122
url,
116123
method,
117124
statusCode,
118-
request,
119-
response,
125+
request: requestWarning ? mergeWarning(request, requestWarning) : request,
126+
response: responseWarning ? mergeWarning(response, responseWarning) : response,
120127
};
121128
}
122129

@@ -134,12 +141,12 @@ function getResponseHeaders(xhr: XMLHttpRequest): Record<string, string> {
134141
}, {});
135142
}
136143

137-
function _getXhrResponseBody(xhr: XMLHttpRequest): string | undefined {
144+
function _getXhrResponseBody(xhr: XMLHttpRequest): [string | undefined, NetworkMetaWarning?] {
138145
// We collect errors that happen, but only log them if we can't get any response body
139146
const errors: unknown[] = [];
140147

141148
try {
142-
return xhr.responseText;
149+
return [xhr.responseText];
143150
} catch (e) {
144151
errors.push(e);
145152
}
@@ -154,5 +161,5 @@ function _getXhrResponseBody(xhr: XMLHttpRequest): string | undefined {
154161

155162
__DEBUG_BUILD__ && logger.warn('[Replay] Failed to get xhr response body', ...errors);
156163

157-
return undefined;
164+
return [undefined];
158165
}

packages/replay/src/types/request.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ type JsonArray = unknown[];
33

44
export type NetworkBody = JsonObject | JsonArray | string;
55

6-
export type NetworkMetaWarning = 'MAYBE_JSON_TRUNCATED' | 'TEXT_TRUNCATED' | 'URL_SKIPPED';
6+
export type NetworkMetaWarning = 'MAYBE_JSON_TRUNCATED' | 'TEXT_TRUNCATED' | 'URL_SKIPPED' | 'BODY_PARSE_ERROR';
77

88
interface NetworkMeta {
99
warnings?: NetworkMetaWarning[];

packages/replay/test/unit/coreHandlers/handleNetworkBreadcrumbs.test.ts

+2
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ async function waitForReplayEventBuffer() {
2323
await Promise.resolve();
2424
await Promise.resolve();
2525
await Promise.resolve();
26+
await Promise.resolve();
27+
await Promise.resolve();
2628
}
2729

2830
const LARGE_BODY = 'a'.repeat(NETWORK_BODY_MAX_SIZE + 1);

0 commit comments

Comments
 (0)