Don't swallow fetch TypeError as CORS in non-browser environments

packages/client/src/client/auth.ts

@@ -826,18 +826,48 @@ 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> {
+    // CORS only exists in browsers. In Node.js/Workers/etc., a TypeError from fetch is always a
+    // real network or configuration error, never CORS.
+    const corsIsPossible = (globalThis as { document?: unknown }).document !== 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) || !corsIsPossible) {
+            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;
     }
 }
 

packages/client/test/client/auth.test.ts

@@ -33,10 +33,27 @@ vi.mock('pkce-challenge', () => ({
 const mockFetch = vi.fn();
 globalThis.fetch = mockFetch;
 
+/**
+ * fetchWithCorsRetry gates its CORS-swallowing heuristic on running in a browser (where `document`
+ * exists). CORS doesn't apply in Node.js, so there a fetch TypeError is always a real network error
+ * and is thrown instead of swallowed. Tests that specifically exercise the CORS retry logic call
+ * this helper to simulate a browser environment. Cleanup is done by `restoreBrowserLikeEnvironment`
+ * in an `afterEach` hook so a failed assertion can't leak the `document` stub into later tests.
+ */
+function withBrowserLikeEnvironment(): void {
+    (globalThis as { document?: unknown }).document = {};
+}
+function restoreBrowserLikeEnvironment(): void {
+    delete (globalThis as { document?: unknown }).document;
+}
+
 describe('OAuth Authorization', () => {
     beforeEach(() => {
         mockFetch.mockReset();
     });
+    afterEach(() => {
+        restoreBrowserLikeEnvironment();
+    });
 
     describe('extractWWWAuthenticateParams', () => {
         it('returns resource metadata url when present', async () => {
@@ -131,7 +148,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 +177,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 +196,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 +379,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 +592,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 +624,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 +653,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 +672,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 +863,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 +920,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 +932,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', () => {