Co-authored-by: Konstantin Konstantinov <konstantin@mach5technology.com> Co-authored-by: Konstantin Konstantinov <KKonstantinov@users.noreply.github.com>
This commit is contained in:
parent
dcc01028ff
commit
6fa42279fe
6 changed files with 97 additions and 7 deletions
7
.changeset/brave-donkeys-listen.md
Normal file
7
.changeset/brave-donkeys-listen.md
Normal file
|
|
@ -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.
|
||||
|
|
@ -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 }
|
||||
)
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
|
|
@ -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';
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
Loading…
Reference in a new issue