Skip to content

Commit 23639a5

Browse files
mydeamydea
committed
wrap fetch better
1 parent d8651a6 commit 23639a5

2 files changed

Lines changed: 132 additions & 95 deletions

File tree

‎packages/core/src/instrument/fetch.ts‎

Lines changed: 97 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -53,108 +53,112 @@ function instrumentFetch(onFetchResolved?: (response: Response) => void): void {
5353
return;
5454
}
5555

56-
fill(GLOBAL_OBJ, 'fetch', function (originalFetch: () => void): () => void {
57-
return function (...args: any[]): void {
58-
// We capture the error right here and not in the Promise error callback because Safari (and probably other
59-
// browsers too) will wipe the stack trace up to this point, only leaving us with this file which is useless.
60-
61-
// NOTE: If you are a Sentry user, and you are seeing this stack frame,
62-
// it means the error, that was caused by your fetch call did not
63-
// have a stack trace, so the SDK backfilled the stack trace so
64-
// you can see which fetch call failed.
65-
const virtualError = new Error();
66-
67-
const { method, url } = parseFetchArgs(args);
68-
const handlerData: HandlerDataFetch = {
69-
args,
70-
fetchData: {
71-
method,
72-
url,
73-
},
74-
startTimestamp: timestampInSeconds() * 1000,
75-
// // Adding the error to be able to fingerprint the failed fetch event in HttpClient instrumentation
76-
virtualError,
77-
headers: getHeadersFromFetchArgs(args),
78-
};
79-
80-
// if there is no callback, fetch is instrumented directly
81-
if (!onFetchResolved) {
82-
triggerHandlers('fetch', {
83-
...handlerData,
84-
});
85-
}
56+
// We wrap `fetch` in a `Proxy` rather than a plain closure so that non-standard own properties some
57+
// runtimes hang off the global `fetch` (e.g. Bun's `fetch.preconnect`) keep working - property access
58+
// and `toString()` transparently forward to the native implementation.
59+
fill(GLOBAL_OBJ, 'fetch', function (originalFetch: (...args: any[]) => Promise<Response>): unknown {
60+
return new Proxy(originalFetch, {
61+
apply(target, _thisArg, args: any[]): Promise<Response> {
62+
// We capture the error right here and not in the Promise error callback because Safari (and probably other
63+
// browsers too) will wipe the stack trace up to this point, only leaving us with this file which is useless.
64+
65+
// NOTE: If you are a Sentry user, and you are seeing this stack frame,
66+
// it means the error, that was caused by your fetch call did not
67+
// have a stack trace, so the SDK backfilled the stack trace so
68+
// you can see which fetch call failed.
69+
const virtualError = new Error();
70+
71+
const { method, url } = parseFetchArgs(args);
72+
const handlerData: HandlerDataFetch = {
73+
args,
74+
fetchData: {
75+
method,
76+
url,
77+
},
78+
startTimestamp: timestampInSeconds() * 1000,
79+
// // Adding the error to be able to fingerprint the failed fetch event in HttpClient instrumentation
80+
virtualError,
81+
headers: getHeadersFromFetchArgs(args),
82+
};
83+
84+
// if there is no callback, fetch is instrumented directly
85+
if (!onFetchResolved) {
86+
triggerHandlers('fetch', {
87+
...handlerData,
88+
});
89+
}
90+
91+
return Reflect.apply(target, GLOBAL_OBJ, args).then(
92+
async (response: Response) => {
93+
if (onFetchResolved) {
94+
onFetchResolved(response);
95+
} else {
96+
triggerHandlers('fetch', {
97+
...handlerData,
98+
endTimestamp: timestampInSeconds() * 1000,
99+
response,
100+
});
101+
}
86102

87-
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
88-
return originalFetch.apply(GLOBAL_OBJ, args).then(
89-
async (response: Response) => {
90-
if (onFetchResolved) {
91-
onFetchResolved(response);
92-
} else {
103+
return response;
104+
},
105+
(error: Error) => {
93106
triggerHandlers('fetch', {
94107
...handlerData,
95108
endTimestamp: timestampInSeconds() * 1000,
96-
response,
109+
error,
97110
});
98-
}
99111

100-
return response;
101-
},
102-
(error: Error) => {
103-
triggerHandlers('fetch', {
104-
...handlerData,
105-
endTimestamp: timestampInSeconds() * 1000,
106-
error,
107-
});
112+
if (isError(error) && error.stack === undefined) {
113+
// NOTE: If you are a Sentry user, and you are seeing this stack frame,
114+
// it means the error, that was caused by your fetch call did not
115+
// have a stack trace, so the SDK backfilled the stack trace so
116+
// you can see which fetch call failed.
117+
error.stack = virtualError.stack;
118+
addNonEnumerableProperty(error, 'framesToPop', 1);
119+
}
108120

109-
if (isError(error) && error.stack === undefined) {
110-
// NOTE: If you are a Sentry user, and you are seeing this stack frame,
111-
// it means the error, that was caused by your fetch call did not
112-
// have a stack trace, so the SDK backfilled the stack trace so
113-
// you can see which fetch call failed.
114-
error.stack = virtualError.stack;
115-
addNonEnumerableProperty(error, 'framesToPop', 1);
116-
}
117-
118-
// We enhance fetch error messages with hostname information based on the configuration.
119-
// Possible messages we handle here:
120-
// * "Failed to fetch" (chromium)
121-
// * "Load failed" (webkit)
122-
// * "NetworkError when attempting to fetch resource." (firefox)
123-
const client = getClient();
124-
const enhanceOption = client?.getOptions().enhanceFetchErrorMessages ?? 'always';
125-
const shouldEnhance = enhanceOption !== false;
126-
127-
if (
128-
shouldEnhance &&
129-
error instanceof TypeError &&
130-
(error.message === 'Failed to fetch' ||
131-
error.message === 'Load failed' ||
132-
error.message === 'NetworkError when attempting to fetch resource.')
133-
) {
134-
try {
135-
const url = new URL(handlerData.fetchData.url);
136-
const hostname = url.host;
137-
138-
if (enhanceOption === 'always') {
139-
// Modify the error message directly
140-
error.message = `${error.message} (${hostname})`;
141-
} else {
142-
// Store hostname as non-enumerable property for Sentry-only enhancement
143-
// This preserves the original error message for third-party packages
144-
addNonEnumerableProperty(error, '__sentry_fetch_url_host__', hostname);
121+
// We enhance fetch error messages with hostname information based on the configuration.
122+
// Possible messages we handle here:
123+
// * "Failed to fetch" (chromium)
124+
// * "Load failed" (webkit)
125+
// * "NetworkError when attempting to fetch resource." (firefox)
126+
const client = getClient();
127+
const enhanceOption = client?.getOptions().enhanceFetchErrorMessages ?? 'always';
128+
const shouldEnhance = enhanceOption !== false;
129+
130+
if (
131+
shouldEnhance &&
132+
error instanceof TypeError &&
133+
(error.message === 'Failed to fetch' ||
134+
error.message === 'Load failed' ||
135+
error.message === 'NetworkError when attempting to fetch resource.')
136+
) {
137+
try {
138+
const url = new URL(handlerData.fetchData.url);
139+
const hostname = url.host;
140+
141+
if (enhanceOption === 'always') {
142+
// Modify the error message directly
143+
error.message = `${error.message} (${hostname})`;
144+
} else {
145+
// Store hostname as non-enumerable property for Sentry-only enhancement
146+
// This preserves the original error message for third-party packages
147+
addNonEnumerableProperty(error, '__sentry_fetch_url_host__', hostname);
148+
}
149+
} catch {
150+
// ignore it if errors happen here
145151
}
146-
} catch {
147-
// ignore it if errors happen here
148152
}
149-
}
150-
151-
// NOTE: If you are a Sentry user, and you are seeing this stack frame,
152-
// it means the sentry.javascript SDK caught an error invoking your application code.
153-
// This is expected behavior and NOT indicative of a bug with sentry.javascript.
154-
throw error;
155-
},
156-
);
157-
};
153+
154+
// NOTE: If you are a Sentry user, and you are seeing this stack frame,
155+
// it means the sentry.javascript SDK caught an error invoking your application code.
156+
// This is expected behavior and NOT indicative of a bug with sentry.javascript.
157+
throw error;
158+
},
159+
);
160+
},
161+
});
158162
});
159163
}
160164

‎packages/core/test/lib/instrument/fetch.test.ts‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
import { describe, expect, it } from 'vitest';
2-
import { parseFetchArgs } from '../../../src/instrument/fetch';
1+
import { afterEach, describe, expect, it, vi } from 'vitest';
2+
import { addFetchInstrumentationHandler, parseFetchArgs } from '../../../src/instrument/fetch';
3+
import { resetInstrumentationHandlers } from '../../../src/instrument/handlers';
4+
import * as isBrowserModule from '../../../src/utils/isBrowser';
5+
import { GLOBAL_OBJ } from '../../../src/utils/worldwide';
36

47
describe('instrument > parseFetchArgs', () => {
58
it.each([
@@ -53,3 +56,33 @@ describe('instrument > parseFetchArgs', () => {
5356
});
5457
});
5558
});
59+
60+
describe('instrument > addFetchInstrumentationHandler', () => {
61+
const globalWithFetch = GLOBAL_OBJ as typeof GLOBAL_OBJ & { fetch?: (...args: unknown[]) => unknown };
62+
63+
afterEach(() => {
64+
resetInstrumentationHandlers();
65+
vi.restoreAllMocks();
66+
});
67+
68+
it('preserves non-standard own properties on the global fetch (e.g. Bun `fetch.preconnect`)', () => {
69+
// Non-browser runtime so we skip the native-fetch check and always patch
70+
vi.spyOn(isBrowserModule, 'isBrowser').mockReturnValue(false);
71+
72+
const preconnect = vi.fn();
73+
const originalFetch = vi.fn(() => Promise.resolve(new Response()));
74+
(originalFetch as unknown as { preconnect: unknown }).preconnect = preconnect;
75+
globalWithFetch.fetch = originalFetch as unknown as typeof globalWithFetch.fetch;
76+
77+
try {
78+
addFetchInstrumentationHandler(() => {});
79+
80+
// fetch was actually wrapped ...
81+
expect(globalWithFetch.fetch).not.toBe(originalFetch);
82+
// ... and the non-standard own property was carried over onto the wrapper
83+
expect((globalWithFetch.fetch as unknown as { preconnect: unknown }).preconnect).toBe(preconnect);
84+
} finally {
85+
globalWithFetch.fetch = originalFetch as unknown as typeof globalWithFetch.fetch;
86+
}
87+
});
88+
});

0 commit comments

Comments
 (0)