fix(client): surface underlying network error via Error.cause on probe failures (#2657)

.changeset/brave-donkeys-listen.md

@@ -0,0 +1,7 @@
+---
+'@modelcontextprotocol/core-internal': patch
+'@modelcontextprotocol/client': patch
+'@modelcontextprotocol/server': patch
+---
+
+`SdkError` and `SdkHttpError` accept standard `ErrorOptions` as an optional fourth constructor argument and forward it to `Error`, so a wrapped error is reachable through the standard `Error.cause` chain. Version-negotiation probe failures (`SdkErrorCode.EraNegotiationFailed`) now use it: the underlying `TypeError: fetch failed` and the DNS or socket error beneath it surface via `error.cause`, so pino, Sentry, and `util.inspect` render `ENOTFOUND` / `ECONNREFUSED` / `ETIMEDOUT` instead of stopping at the `SdkError` (#2657). The previous `error.data.cause` slot is still populated for compatibility but is deprecated and slated for removal; read `error.cause` instead.

packages/client/src/client/probeClassifier.ts

@@ -311,9 +311,14 @@ function classifyNetworkError(error: unknown, context: ProbeClassifierContext):
     }
     return {
         kind: 'error',
-        error: new SdkError(SdkErrorCode.EraNegotiationFailed, `Version negotiation probe failed: ${describeError(error)}`, {
-            cause: error
-        })
+        error: new SdkError(
+            SdkErrorCode.EraNegotiationFailed,
+            `Version negotiation probe failed: ${describeError(error)}`,
+            // Keep data.cause for existing consumers while also exposing the
+            // standard Error.cause chain (#2657).
+            { cause: error },
+            { cause: error }
+        )
     };
 }
 

packages/client/test/client/probeAuthSeam.test.ts

@@ -283,6 +283,9 @@ describe('stamped-seam fault injection (identity-preserving auth outcomes, never
         expect(out.settled).toBe('rejected');
         expect(out.error).toBeInstanceOf(SdkError);
         expect((out.error as SdkError).code).toBe(SdkErrorCode.EraNegotiationFailed);
+        // The failure rides the standard cause chain (#2657); the legacy data.cause
+        // slot is kept populated for compatibility until it is removed.
+        expect((out.error as SdkError).cause).toBe(netError);
         expect(((out.error as SdkError).data as { cause?: unknown }).cause).toBe(netError);
     });
 });

packages/client/test/client/probeClassifier.test.ts

@@ -270,6 +270,25 @@ describe('row: network outage → typed connect error (Node)', () => {
         const verdict = classify({ kind: 'network-error', error: new TypeError('fetch failed') }, { environment: 'node' });
         expect(verdict.kind).toBe('error');
     });
+
+    test('the underlying network error is reachable via Error.cause (#2657)', () => {
+        // Node's fetch wraps the socket/DNS failure: `TypeError: fetch failed` with
+        // the error that actually names the failure (ENOTFOUND / ECONNREFUSED /
+        // ETIMEDOUT) on its own `cause`.
+        const dnsError = Object.assign(new Error('getaddrinfo ENOTFOUND unreachable.invalid'), { code: 'ENOTFOUND' });
+        const fetchError = new TypeError('fetch failed', { cause: dnsError });
+        const verdict = classify({ kind: 'network-error', error: fetchError });
+        expect(verdict.kind).toBe('error');
+        if (verdict.kind === 'error') {
+            // Walking `.cause` (what loggers and error reporters do) must reach the
+            // error that names the failure instead of dead-ending on the SdkError.
+            expect(verdict.error.cause).toBe(fetchError);
+            expect((verdict.error.cause as Error).cause).toBe(dnsError);
+            // The legacy data.cause slot stays populated too (kept for compatibility,
+            // slated for removal).
+            expect(((verdict.error as SdkError).data as { cause?: unknown }).cause).toBe(fetchError);
+        }
+    });
 });
 
 describe('row: timeout — transport-aware verdict', () => {

packages/core-internal/src/errors/sdkErrors.ts

@@ -144,12 +144,23 @@ export class SdkError extends Error {
         return brandedHasInstance(this, value);
     }
 
+    /**
+     * @param code - Stable string code identifying the failure ({@linkcode SdkErrorCode}).
+     * @param message - Human-readable description.
+     * @param data - Optional structured payload (for example the HTTP status carried by
+     * {@linkcode SdkHttpError}). Opaque to the SDK: a `cause` key inside `data` is not
+     * promoted to `Error.cause`.
+     * @param options - Standard `ErrorOptions`, forwarded to `Error`. Pass the underlying
+     * failure as `{ cause }` so it is reachable through the `Error.cause` chain that
+     * loggers and error trackers walk.
+     */
     constructor(
         public readonly code: SdkErrorCode,
         message: string,
-        public readonly data?: unknown
+        public readonly data?: unknown,
+        options?: ErrorOptions
     ) {
-        super(message);
+        super(message, options);
         this.name = 'SdkError';
         stampErrorBrands(this, new.target);
     }
@@ -187,8 +198,11 @@ export class SdkHttpError extends SdkError {
 
     declare readonly data: SdkHttpErrorData;
 
-    constructor(code: SdkErrorCode, message: string, data: SdkHttpErrorData) {
-        super(code, message, data);
+    /**
+     * @param options - Standard `ErrorOptions`, forwarded to `Error` (see {@linkcode SdkError}).
+     */
+    constructor(code: SdkErrorCode, message: string, data: SdkHttpErrorData, options?: ErrorOptions) {
+        super(code, message, data, options);
         this.name = 'SdkHttpError';
     }
 

packages/core-internal/test/types/errorSurfacePins.test.ts

@@ -205,6 +205,48 @@ describe('SdkError', () => {
         expect(error.code).toBe('CLIENT_HTTP_FAILED_TO_OPEN_STREAM');
         expect(error.data).toMatchObject({ status: 404 });
     });
+
+    // Cause plumbing (#2657): a wrapped error travels on the standard `Error.cause`
+    // chain via `ErrorOptions`, never through the opaque `data` payload, so pino /
+    // Sentry / `util.inspect` reach the root failure without SDK-specific handling.
+    test('forwards ErrorOptions.cause onto Error.cause without touching data', () => {
+        const root = new TypeError('fetch failed');
+        const error = new SdkError(SdkErrorCode.EraNegotiationFailed, 'Version negotiation probe failed', undefined, {
+            cause: root
+        });
+        expect(error.cause).toBe(root);
+        expect(error.data).toBeUndefined();
+        // Same non-enumerable own property the native Error constructor installs,
+        // so serializers that copy enumerable fields do not emit it twice.
+        expect(Object.getOwnPropertyDescriptor(error, 'cause')?.enumerable).toBe(false);
+    });
+
+    test('does not promote a `cause` key inside data to Error.cause', () => {
+        const root = new Error('boom');
+        const error = new SdkError(SdkErrorCode.RequestTimeout, 'Request timed out', { timeout: 60_000, cause: root });
+        expect(error.cause).toBeUndefined();
+        expect(error.data).toEqual({ timeout: 60_000, cause: root });
+    });
+
+    test('carries data and cause independently when both are passed', () => {
+        const root = new Error('boom');
+        const error = new SdkError(SdkErrorCode.RequestTimeout, 'Request timed out', { timeout: 60_000 }, { cause: root });
+        expect(error.cause).toBe(root);
+        expect(error.data).toEqual({ timeout: 60_000 });
+    });
+
+    test('SdkHttpError forwards ErrorOptions.cause and keeps the HTTP status', () => {
+        const root = new Error('socket hang up');
+        const error = new SdkHttpError(
+            SdkErrorCode.ClientHttpFailedToOpenStream,
+            'Failed to open SSE stream: Bad Gateway',
+            { status: 502, statusText: 'Bad Gateway' },
+            { cause: root }
+        );
+        expect(error.cause).toBe(root);
+        expect(error.status).toBe(502);
+        expect(error.statusText).toBe('Bad Gateway');
+    });
 });
 
 describe('protocol version constants', () => {