Don't swallow fetch TypeError as CORS in non-browser environments (#1595)

bhosmer-ant · felixweinberger · web-flow · commit 13a0d345c0b8 · 2026-03-25T10:45:59.000Z
Co-authored-by: Felix Weinberger &lt;3823880+felixweinberger@users.noreply.github.com&gt;
Co-authored-by: Felix Weinberger &lt;fweinberger@anthropic.com&gt;
diff --git a/.changeset/tame-camels-greet.md b/.changeset/tame-camels-greet.md
@@ -0,0 +1,9 @@
+---
+'@modelcontextprotocol/client': patch
+---
+
+Don't swallow fetch `TypeError` as CORS in non-browser environments. Network errors
+(DNS resolution failure, connection refused, invalid URL) in Node.js and Cloudflare
+Workers now propagate from OAuth discovery instead of being silently misattributed
+to CORS and returning `undefined`. This surfaces the real error to callers rather
+than masking it as "metadata not found."
diff --git a/packages/client/package.json b/packages/client/package.json
@@ -30,8 +30,8 @@
                 "import": "./dist/shimsWorkerd.mjs"
             },
             "browser": {
-                "types": "./dist/shimsWorkerd.d.mts",
-                "import": "./dist/shimsWorkerd.mjs"
+                "types": "./dist/shimsBrowser.d.mts",
+                "import": "./dist/shimsBrowser.mjs"
             },
             "node": {
                 "types": "./dist/shimsNode.d.mts",
diff --git a/packages/client/src/client/auth.ts b/packages/client/src/client/auth.ts
@@ -1,3 +1,4 @@
+import { CORS_IS_POSSIBLE } from '@modelcontextprotocol/client/_shims';
 import type {
     AuthorizationServerMetadata,
     FetchLike,
@@ -512,7 +513,12 @@ async function authInternal(
                     { resourceMetadataUrl: effectiveResourceMetadataUrl },
                     fetchFn
                 );
-            } catch {
+            } catch (error) {
+                // Network failures (DNS, connection refused) surface as TypeError — propagate
+                // those rather than masking a transient reachability problem.
+                if (error instanceof TypeError) {
+                    throw error;
+                }
                 // RFC 9728 not available — selectResourceURL will handle undefined
             }
         }
@@ -826,18 +832,45 @@ export async function discoverOAuthProtectedResourceMetadata(
 }
 
 /**
- * Helper function to handle fetch with CORS retry logic
+ * Fetch with a retry heuristic for CORS errors caused by custom headers.
+ *
+ * In browsers, adding a custom header (e.g. `MCP-Protocol-Version`) triggers a CORS preflight.
+ * If the server doesn't allow that header, the browser throws a `TypeError` before any response
+ * is received. Retrying without custom headers often succeeds because the request becomes
+ * "simple" (no preflight). If the server sends no CORS headers at all, the retry also fails
+ * with `TypeError` and we return `undefined` so callers can fall through to an alternate URL.
+ *
+ * However, `fetch()` also throws `TypeError` for non-CORS failures (DNS resolution, connection
+ * refused, invalid URL). Swallowing those and returning `undefined` masks real errors and can
+ * cause callers to silently fall through to a different discovery URL. CORS is a browser-only
+ * concept, so in non-browser runtimes (Node.js, Workers) a `TypeError` from `fetch` is never a
+ * CORS error — there we propagate the error instead of swallowing it.
+ *
+ * In browsers, we cannot reliably distinguish CORS `TypeError` from network `TypeError` from the
+ * error object alone, so the swallow-and-fallthrough heuristic is preserved there.
  */
 async function fetchWithCorsRetry(url: URL, headers?: Record<string, string>, fetchFn: FetchLike = fetch): Promise<Response | undefined> {
     try {
         return await fetchFn(url, { headers });
     } catch (error) {
-        if (error instanceof TypeError) {
-            // CORS errors come back as TypeError, retry without headers
-            // We're getting CORS errors on retry too, return undefined
-            return headers ? fetchWithCorsRetry(url, undefined, fetchFn) : undefined;
+        if (!(error instanceof TypeError) || !CORS_IS_POSSIBLE) {
+            throw error;
         }
-        throw error;
+        if (headers) {
+            // Could be a CORS preflight rejection caused by our custom header. Retry as a simple
+            // request: if that succeeds, we've sidestepped the preflight.
+            try {
+                return await fetchFn(url, {});
+            } catch (retryError) {
+                if (!(retryError instanceof TypeError)) {
+                    throw retryError;
+                }
+                // Retry also got CORS-blocked (server sends no CORS headers at all).
+                // Return undefined so the caller tries the next discovery URL.
+                return undefined;
+            }
+        }
+        return undefined;
     }
 }
 
@@ -1146,7 +1179,13 @@ export async function discoverOAuthServerInfo(
         if (resourceMetadata.authorization_servers && resourceMetadata.authorization_servers.length > 0) {
             authorizationServerUrl = resourceMetadata.authorization_servers[0];
         }
-    } catch {
+    } catch (error) {
+        // Network failures (DNS, connection refused) surface as TypeError from fetch. Those are
+        // transient reachability problems, not "server doesn't support PRM" — propagate so the
+        // caller sees the real error instead of silently falling back to a different auth server.
+        if (error instanceof TypeError) {
+            throw error;
+        }
         // RFC 9728 not supported -- fall back to treating the server URL as the authorization server
     }
 
diff --git a/packages/client/src/shimsBrowser.ts b/packages/client/src/shimsBrowser.ts
@@ -0,0 +1,13 @@
+/**
+ * Browser runtime shims for client package
+ *
+ * This file is selected via package.json export conditions when running in a browser.
+ */
+export { CfWorkerJsonSchemaValidator as DefaultJsonSchemaValidator } from '@modelcontextprotocol/core';
+
+/**
+ * Whether `fetch()` may throw `TypeError` due to CORS. Only true in browser contexts
+ * (including Web Workers / Service Workers). In Node.js and Cloudflare Workers, a
+ * `TypeError` from `fetch` is always a real network/configuration error.
+ */
+export const CORS_IS_POSSIBLE = true;
diff --git a/packages/client/src/shimsNode.ts b/packages/client/src/shimsNode.ts
@@ -4,3 +4,10 @@
  * This file is selected via package.json export conditions when running in Node.js.
  */
 export { AjvJsonSchemaValidator as DefaultJsonSchemaValidator } from '@modelcontextprotocol/core';
+
+/**
+ * Whether `fetch()` may throw `TypeError` due to CORS. CORS is a browser-only concept —
+ * in Node.js, a `TypeError` from `fetch` is always a real network/configuration error
+ * (DNS resolution, connection refused, invalid URL), never a CORS error.
+ */
+export const CORS_IS_POSSIBLE = false;
diff --git a/packages/client/src/shimsWorkerd.ts b/packages/client/src/shimsWorkerd.ts
@@ -4,3 +4,10 @@
  * This file is selected via package.json export conditions when running in workerd.
  */
 export { CfWorkerJsonSchemaValidator as DefaultJsonSchemaValidator } from '@modelcontextprotocol/core';
+
+/**
+ * Whether `fetch()` may throw `TypeError` due to CORS. CORS is a browser-only concept —
+ * in Cloudflare Workers, a `TypeError` from `fetch` is always a real network/configuration
+ * error, never a CORS error.
+ */
+export const CORS_IS_POSSIBLE = false;
diff --git a/packages/client/test/client/auth.test.ts b/packages/client/test/client/auth.test.ts
@@ -33,10 +33,34 @@ vi.mock('pkce-challenge', () => ({
 const mockFetch = vi.fn();
 globalThis.fetch = mockFetch;
 
+/**
+ * fetchWithCorsRetry gates its CORS-swallowing heuristic on the `CORS_IS_POSSIBLE` shim constant.
+ * Tests run under the Node shim (`false`), so a fetch TypeError is treated as a real network error
+ * and thrown instead of swallowed. Tests that specifically exercise the browser CORS retry path
+ * call `withBrowserLikeEnvironment()` to flip the mocked constant to `true`. The `afterEach` hook
+ * resets it so a failed assertion can't leak the override into later tests.
+ */
+let mockedCorsIsPossible = false;
+vi.mock('@modelcontextprotocol/client/_shims', async importOriginal => {
+    const actual = await importOriginal<typeof import('@modelcontextprotocol/client/_shims')>();
+    return {
+        ...actual,
+        get CORS_IS_POSSIBLE() {
+            return mockedCorsIsPossible;
+        }
+    };
+});
+function withBrowserLikeEnvironment(): void {
+    mockedCorsIsPossible = true;
+}
+
 describe('OAuth Authorization', () => {
     beforeEach(() => {
         mockFetch.mockReset();
     });
+    afterEach(() => {
+        mockedCorsIsPossible = false;
+    });
 
     describe('extractWWWAuthenticateParams', () => {
         it('returns resource metadata url when present', async () => {
@@ -131,7 +155,8 @@ describe('OAuth Authorization', () => {
             expect(url.toString()).toBe('https://resource.example.com/.well-known/oauth-protected-resource');
         });
 
-        it('returns metadata when first fetch fails but second without MCP header succeeds', async () => {
+        it('returns metadata when first fetch fails but second without MCP header succeeds (browser CORS retry)', async () => {
+            withBrowserLikeEnvironment();
             // Set up a counter to control behavior
             let callCount = 0;
 
@@ -159,7 +184,8 @@ describe('OAuth Authorization', () => {
             expect(mockFetch.mock.calls[0]![1]?.headers).toHaveProperty('MCP-Protocol-Version');
         });
 
-        it('throws an error when all fetch attempts fail', async () => {
+        it('throws an error when all fetch attempts fail (browser, retry throws non-TypeError)', async () => {
+            withBrowserLikeEnvironment();
             // Set up a counter to control behavior
             let callCount = 0;
 
@@ -177,6 +203,18 @@ describe('OAuth Authorization', () => {
             expect(mockFetch).toHaveBeenCalledTimes(2);
         });
 
+        it('throws TypeError immediately in non-browser environments without retrying', async () => {
+            // In Node.js/Workers, CORS doesn't exist — a TypeError from fetch is a real
+            // network/config error (DNS failure, connection refused, invalid URL) and
+            // should propagate rather than being silently swallowed.
+            mockFetch.mockImplementation(() => Promise.reject(new TypeError('getaddrinfo ENOTFOUND resource.example.com')));
+
+            await expect(discoverOAuthProtectedResourceMetadata('https://resource.example.com')).rejects.toThrow(TypeError);
+
+            // Only one call — no CORS retry attempted
+            expect(mockFetch).toHaveBeenCalledTimes(1);
+        });
+
         it('throws on 404 errors', async () => {
             mockFetch.mockResolvedValueOnce({
                 ok: false,
@@ -348,7 +386,8 @@ describe('OAuth Authorization', () => {
             expect(url.toString()).toBe('https://resource.example.com/.well-known/oauth-protected-resource');
         });
 
-        it('falls back when path-aware discovery encounters CORS error', async () => {
+        it('falls back when path-aware discovery encounters CORS error (browser)', async () => {
+            withBrowserLikeEnvironment();
             // First call (path-aware) fails with TypeError (CORS)
             mockFetch.mockImplementationOnce(() => Promise.reject(new TypeError('CORS error')));
 
@@ -560,7 +599,8 @@ describe('OAuth Authorization', () => {
             expect(url.toString()).toBe('https://auth.example.com/.well-known/oauth-authorization-server');
         });
 
-        it('falls back when path-aware discovery encounters CORS error', async () => {
+        it('falls back when path-aware discovery encounters CORS error (browser)', async () => {
+            withBrowserLikeEnvironment();
             // First call (path-aware) fails with TypeError (CORS)
             mockFetch.mockImplementationOnce(() => Promise.reject(new TypeError('CORS error')));
 
@@ -591,7 +631,8 @@ describe('OAuth Authorization', () => {
             });
         });
 
-        it('returns metadata when first fetch fails but second without MCP header succeeds', async () => {
+        it('returns metadata when first fetch fails but second without MCP header succeeds (browser CORS retry)', async () => {
+            withBrowserLikeEnvironment();
             // Set up a counter to control behavior
             let callCount = 0;
 
@@ -619,7 +660,8 @@ describe('OAuth Authorization', () => {
             expect(mockFetch.mock.calls[0]![1]?.headers).toHaveProperty('MCP-Protocol-Version');
         });
 
-        it('throws an error when all fetch attempts fail', async () => {
+        it('throws an error when all fetch attempts fail (browser, retry throws non-TypeError)', async () => {
+            withBrowserLikeEnvironment();
             // Set up a counter to control behavior
             let callCount = 0;
 
@@ -637,7 +679,8 @@ describe('OAuth Authorization', () => {
             expect(mockFetch).toHaveBeenCalledTimes(2);
         });
 
-        it('returns undefined when both CORS requests fail in fetchWithCorsRetry', async () => {
+        it('returns undefined when both CORS requests fail in fetchWithCorsRetry (browser)', async () => {
+            withBrowserLikeEnvironment();
             // fetchWithCorsRetry tries with headers (fails with CORS), then retries without headers (also fails with CORS)
             // simulating a 404 w/o headers set. We want this to return undefined, not throw TypeError
             mockFetch.mockImplementation(() => {
@@ -827,7 +870,8 @@ describe('OAuth Authorization', () => {
             await expect(discoverAuthorizationServerMetadata('https://mcp.example.com')).rejects.toThrow('HTTP 500');
         });
 
-        it('handles CORS errors with retry', async () => {
+        it('handles CORS errors with retry (browser)', async () => {
+            withBrowserLikeEnvironment();
             // First call fails with CORS
             mockFetch.mockImplementationOnce(() => Promise.reject(new TypeError('CORS error')));
 
@@ -883,7 +927,8 @@ describe('OAuth Authorization', () => {
             });
         });
 
-        it('returns undefined when all URLs fail with CORS errors', async () => {
+        it('returns undefined when all URLs fail with CORS errors (browser)', async () => {
+            withBrowserLikeEnvironment();
             // All fetch attempts fail with CORS errors (TypeError)
             mockFetch.mockImplementation(() => Promise.reject(new TypeError('CORS error')));
 
@@ -894,6 +939,18 @@ describe('OAuth Authorization', () => {
             // Verify that all discovery URLs were attempted
             expect(mockFetch).toHaveBeenCalledTimes(6); // 3 URLs × 2 attempts each (with and without headers)
         });
+
+        it('throws TypeError in non-browser environments instead of silently falling through (network failure)', async () => {
+            // In Node.js, a TypeError from fetch is a real error (DNS/connection), not CORS.
+            // Swallowing it and returning undefined would cause the caller to silently fall
+            // through to the next discovery URL, masking the actual network failure.
+            mockFetch.mockImplementation(() => Promise.reject(new TypeError('getaddrinfo ENOTFOUND auth.example.com')));
+
+            await expect(discoverAuthorizationServerMetadata('https://auth.example.com/tenant1')).rejects.toThrow(TypeError);
+
+            // Only one call — no CORS retry attempted in non-browser environments
+            expect(mockFetch).toHaveBeenCalledTimes(1);
+        });
     });
 
     describe('discoverOAuthServerInfo', () => {
@@ -1006,6 +1063,15 @@ describe('OAuth Authorization', () => {
             // Verify the override URL was used instead of the default well-known path
             expect(mockFetch.mock.calls[0]![0].toString()).toBe(overrideUrl.toString());
         });
+
+        it('propagates network failures instead of silently falling back (non-browser)', async () => {
+            // PRM discovery hits a DNS/connection failure. That's a transient reachability problem,
+            // not "server doesn't support RFC 9728" — the caller should see the real error rather
+            // than silently falling back to treating the MCP server URL as the auth server.
+            mockFetch.mockImplementation(() => Promise.reject(new TypeError('getaddrinfo ENOTFOUND resource.example.com')));
+
+            await expect(discoverOAuthServerInfo('https://resource.example.com')).rejects.toThrow(TypeError);
+        });
     });
 
     describe('auth with provider authorization server URL caching', () => {
diff --git a/packages/client/tsdown.config.ts b/packages/client/tsdown.config.ts
@@ -3,7 +3,7 @@ import { defineConfig } from 'tsdown';
 export default defineConfig({
     // 1. Entry Points
     //    Directly matches package.json include/exclude globs
-    entry: ['src/index.ts', 'src/shimsNode.ts', 'src/shimsWorkerd.ts'],
+    entry: ['src/index.ts', 'src/shimsNode.ts', 'src/shimsWorkerd.ts', 'src/shimsBrowser.ts'],
 
     // 2. Output Configuration
     format: ['esm'],
