Skip to content

feat(core): Only check for native fetch in browser #22893

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mydea wants to merge 1 commit into from
+11 −9
Open

feat(core): Only check for native fetch in browser #22893

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/browser/src/integrations/httpclient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ function _wrapFetch(client: Client, options: HttpClientOptions): void {
}

_fetchResponseHandler(options, requestInfo, response as Response, requestInit, error || virtualError);
}, false);
});
}

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/cloudflare/src/integrations/fetch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ const _fetchIntegration = ((options: Partial<Options> = {}) => {
if (breadcrumbs) {
createBreadcrumb(handlerData);
}
}, true);
});
},
setup(client) {
HAS_CLIENT_MAP.set(client, true);
Expand Down
16 changes: 9 additions & 7 deletions packages/core/src/instrument/fetch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { getClient } from '../currentScopes';
import type { HandlerDataFetch } from '../types/instrument';
import type { WebFetchHeaders } from '../types/webfetchapi';
import { isError, isObjectLike, isRequest } from '../utils/is';
import { isBrowser } from '../utils/isBrowser';
import { addNonEnumerableProperty, fill } from '../utils/object';
import { supportsNativeFetch } from '../utils/supports';
import { timestampInSeconds } from '../utils/time';
Expand All @@ -20,13 +21,10 @@ type FetchResource = string | { toString(): string } | { url: string };
* Use at your own risk, this might break without changelog notice, only used internally.
* @hidden
*/
export function addFetchInstrumentationHandler(
handler: (data: HandlerDataFetch) => void,
skipNativeFetchCheck?: boolean,
): () => void {
export function addFetchInstrumentationHandler(handler: (data: HandlerDataFetch) => void): () => void {
const type = 'fetch';
const removeHandler = addHandler(type, handler);
maybeInstrument(type, () => instrumentFetch(undefined, skipNativeFetchCheck));
maybeInstrument(type, () => instrumentFetch());
return removeHandler;
}

Expand All @@ -46,8 +44,12 @@ export function addFetchEndInstrumentationHandler(handler: (data: HandlerDataFet
return removeHandler;
}

function instrumentFetch(onFetchResolved?: (response: Response) => void, skipNativeFetchCheck: boolean = false): void {
if (skipNativeFetchCheck && !supportsNativeFetch()) {
function instrumentFetch(onFetchResolved?: (response: Response) => void): void {
// The native-fetch check is only meaningful in the browser: it probes for `[native code]` and
// falls back to an iframe DOM check to detect a polyfilled/wrapped `fetch` (which we don't want to
// double-instrument alongside XHR). Outside the browser there is no DOM, and `fetch` may be
// legitimately wrapped by the host (e.g. Next.js on Bun), so we always patch the global there.
if (isBrowser() && !supportsNativeFetch()) {

@cursor cursor Bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feat PR lacks required tests

Medium Severity

This is a feat PR, but the diff has no integration or E2E test covering the new isBrowser()-gated native-fetch check in instrumentFetch. That violates the testing convention that feat PRs include at least one integration or E2E test. Without that, browser skip vs non-browser always-patch behavior can regress unnoticed.

Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot

Reviewed by Cursor Bugbot for commit b9bc3ac. Configure here.

@mydea mydea Jul 31, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is tested in various places already.

@isaacs isaacs Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguably this is a ref(core), I guess, since it's really not adding any user-facing feature, I guess?

return;
}

Expand Down
Loading