v2: SdkError tests + codemod (#2137)
This commit is contained in:
parent
f563440cfd
commit
542d5c9586
9 changed files with 101 additions and 23 deletions
5
.changeset/codemod-streamablehttperror-sdkhttperror.md
Normal file
5
.changeset/codemod-streamablehttperror-sdkhttperror.md
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
'@modelcontextprotocol/codemod': patch
|
||||
---
|
||||
|
||||
The v1→v2 codemod now migrates the removed `StreamableHTTPError` to `SdkHttpError` (instead of the base `SdkError`), matching the shipped error type and the migration guide. Diagnostics now point at the typed `error.status` / `error.statusText` accessors and note that unexpected-content-type responses are thrown as the base `SdkError`.
|
||||
|
|
@ -1590,6 +1590,7 @@ describe('SSEClientTransport', () => {
|
|||
expect(error).toBeInstanceOf(SdkHttpError);
|
||||
expect((error as SdkHttpError).code).toBe(SdkErrorCode.ClientHttpAuthentication);
|
||||
expect((error as SdkHttpError).status).toBe(401);
|
||||
expect((error as SdkHttpError).statusText).toBe('Unauthorized');
|
||||
expect(authProvider.onUnauthorized).toHaveBeenCalledTimes(1);
|
||||
expect(postCount).toBe(2);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1875,6 +1875,7 @@ describe('StreamableHTTPClientTransport', () => {
|
|||
expect(error).toBeInstanceOf(SdkHttpError);
|
||||
expect((error as SdkHttpError).code).toBe(SdkErrorCode.ClientHttpAuthentication);
|
||||
expect((error as SdkHttpError).status).toBe(401);
|
||||
expect((error as SdkHttpError).statusText).toBe('Unauthorized');
|
||||
expect(mockAuthProvider.saveTokens).toHaveBeenCalledWith({
|
||||
access_token: 'new-access-token',
|
||||
token_type: 'Bearer',
|
||||
|
|
|
|||
|
|
@ -95,13 +95,26 @@ describe('StreamableHTTPClientTransport with AuthProvider', () => {
|
|||
vi.spyOn(globalThis, 'fetch');
|
||||
|
||||
(globalThis.fetch as Mock)
|
||||
.mockResolvedValueOnce({ ok: false, status: 401, headers: new Headers(), text: async () => 'unauthorized' })
|
||||
.mockResolvedValueOnce({ ok: false, status: 401, headers: new Headers(), text: async () => 'unauthorized' });
|
||||
.mockResolvedValueOnce({
|
||||
ok: false,
|
||||
status: 401,
|
||||
statusText: 'Unauthorized',
|
||||
headers: new Headers(),
|
||||
text: async () => 'unauthorized'
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
ok: false,
|
||||
status: 401,
|
||||
statusText: 'Unauthorized',
|
||||
headers: new Headers(),
|
||||
text: async () => 'unauthorized'
|
||||
});
|
||||
|
||||
const error = await transport.send(message).catch(e => e);
|
||||
expect(error).toBeInstanceOf(SdkHttpError);
|
||||
expect((error as SdkHttpError).code).toBe(SdkErrorCode.ClientHttpAuthentication);
|
||||
expect((error as SdkHttpError).status).toBe(401);
|
||||
expect((error as SdkHttpError).statusText).toBe('Unauthorized');
|
||||
expect(authProvider.onUnauthorized).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ const REEXPORT_WARNINGS: Record<string, string> = {
|
|||
'Re-exported RequestHandlerExtra was renamed to ServerContext/ClientContext in v2. Update this re-export manually.',
|
||||
IsomorphicHeaders: 'Re-exported IsomorphicHeaders was removed in v2 (replaced by standard Headers API). Remove this re-export.',
|
||||
StreamableHTTPError:
|
||||
'Re-exported StreamableHTTPError was renamed to SdkError in v2 with different constructor. Update this re-export manually.'
|
||||
'Re-exported StreamableHTTPError was renamed to SdkHttpError in v2 with a different constructor. Update this re-export manually.'
|
||||
};
|
||||
|
||||
export const importPathsTransform: Transform = {
|
||||
|
|
|
|||
|
|
@ -148,13 +148,14 @@ function handleStreamableHTTPError(sourceFile: SourceFile, diagnostics: Diagnost
|
|||
warning(
|
||||
sourceFile.getFilePath(),
|
||||
node.getStartLineNumber(),
|
||||
'new StreamableHTTPError(statusCode, statusText, body?) → new SdkError(code, message, data?). ' +
|
||||
'Constructor arguments differ — manual review required. Map HTTP status to SdkErrorCode enum value.'
|
||||
'new StreamableHTTPError(statusCode, statusText, body?) → new SdkHttpError(code, message, data). ' +
|
||||
'Constructor arguments differ — manual review required. Map the HTTP status to a SdkErrorCode enum value ' +
|
||||
'and pass the HTTP status via the data argument, e.g. { status, statusText }.'
|
||||
)
|
||||
);
|
||||
}
|
||||
|
||||
renameAllReferences(sourceFile, localName, 'SdkError');
|
||||
renameAllReferences(sourceFile, localName, 'SdkHttpError');
|
||||
changesCount++;
|
||||
|
||||
foundImport.remove();
|
||||
|
|
@ -164,7 +165,7 @@ function handleStreamableHTTPError(sourceFile: SourceFile, diagnostics: Diagnost
|
|||
|
||||
const targetModule = resolveTargetModule(sourceFile, moduleSpec);
|
||||
const insertIndex = sourceFile.getImportDeclarations().length;
|
||||
const importsToAdd = hasConstructorCalls ? ['SdkError', 'SdkErrorCode'] : ['SdkError'];
|
||||
const importsToAdd = hasConstructorCalls ? ['SdkHttpError', 'SdkErrorCode'] : ['SdkHttpError'];
|
||||
addOrMergeImport(sourceFile, targetModule, importsToAdd, false, insertIndex);
|
||||
changesCount++;
|
||||
|
||||
|
|
@ -172,8 +173,10 @@ function handleStreamableHTTPError(sourceFile: SourceFile, diagnostics: Diagnost
|
|||
warning(
|
||||
sourceFile.getFilePath(),
|
||||
line,
|
||||
'StreamableHTTPError replaced with SdkError. Constructor arguments differ — manual review required. ' +
|
||||
'HTTP status is now in error.data?.status.'
|
||||
'StreamableHTTPError replaced with SdkHttpError (a subclass of SdkError). ' +
|
||||
'HTTP status and status text are now available via error.status and error.statusText. ' +
|
||||
'Note: unexpected-content-type responses (HTTP 200 with the wrong content type) are thrown as the ' +
|
||||
'base SdkError, not SdkHttpError, so a catch-all check should use `instanceof SdkError`.'
|
||||
)
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -324,8 +324,8 @@ describe('integration', () => {
|
|||
expect(output).toContain('const h: Headers');
|
||||
expect(output).not.toContain('IsomorphicHeaders');
|
||||
|
||||
// StreamableHTTPError renamed to SdkError
|
||||
expect(output).toContain('instanceof SdkError');
|
||||
// StreamableHTTPError renamed to SdkHttpError
|
||||
expect(output).toContain('instanceof SdkHttpError');
|
||||
expect(output).not.toContain('StreamableHTTPError');
|
||||
|
||||
// schemaToJson removed (import gone)
|
||||
|
|
|
|||
|
|
@ -163,37 +163,37 @@ describe('removed-apis transform', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('StreamableHTTPError → SdkError', () => {
|
||||
it('renames StreamableHTTPError to SdkError in references', () => {
|
||||
describe('StreamableHTTPError → SdkHttpError', () => {
|
||||
it('renames StreamableHTTPError to SdkHttpError in references', () => {
|
||||
const input = [
|
||||
`import { StreamableHTTPError } from '@modelcontextprotocol/client';`,
|
||||
`if (error instanceof StreamableHTTPError) { throw error; }`,
|
||||
''
|
||||
].join('\n');
|
||||
const { text } = applyTransform(input);
|
||||
expect(text).toContain('instanceof SdkError');
|
||||
expect(text).toContain('instanceof SdkHttpError');
|
||||
expect(text).not.toContain('StreamableHTTPError');
|
||||
});
|
||||
|
||||
it('adds SdkError import without SdkErrorCode when no constructor calls', () => {
|
||||
it('adds SdkHttpError import without SdkErrorCode when no constructor calls', () => {
|
||||
const input = [
|
||||
`import { StreamableHTTPError } from '@modelcontextprotocol/client';`,
|
||||
`if (error instanceof StreamableHTTPError) {}`,
|
||||
''
|
||||
].join('\n');
|
||||
const { text } = applyTransform(input);
|
||||
expect(text).toContain('SdkError');
|
||||
expect(text).toContain('SdkHttpError');
|
||||
expect(text).not.toContain('SdkErrorCode');
|
||||
});
|
||||
|
||||
it('adds SdkError and SdkErrorCode imports when constructor calls exist', () => {
|
||||
it('adds SdkHttpError and SdkErrorCode imports when constructor calls exist', () => {
|
||||
const input = [
|
||||
`import { StreamableHTTPError } from '@modelcontextprotocol/client';`,
|
||||
`throw new StreamableHTTPError(404, 'Not Found');`,
|
||||
''
|
||||
].join('\n');
|
||||
const { text } = applyTransform(input);
|
||||
expect(text).toContain('SdkError');
|
||||
expect(text).toContain('SdkHttpError');
|
||||
expect(text).toContain('SdkErrorCode');
|
||||
});
|
||||
|
||||
|
|
@ -208,14 +208,14 @@ describe('removed-apis transform', () => {
|
|||
expect(constructorWarning).toBeDefined();
|
||||
});
|
||||
|
||||
it('emits general migration warning', () => {
|
||||
it('emits general migration warning pointing at the typed status accessor', () => {
|
||||
const input = [
|
||||
`import { StreamableHTTPError } from '@modelcontextprotocol/client';`,
|
||||
`if (error instanceof StreamableHTTPError) {}`,
|
||||
''
|
||||
].join('\n');
|
||||
const { result } = applyTransform(input);
|
||||
const migrationWarning = result.diagnostics.find(d => d.message.includes('error.data?.status'));
|
||||
const migrationWarning = result.diagnostics.find(d => d.message.includes('error.status'));
|
||||
expect(migrationWarning).toBeDefined();
|
||||
});
|
||||
|
||||
|
|
@ -227,7 +227,7 @@ describe('removed-apis transform', () => {
|
|||
].join('\n');
|
||||
const { text } = applyTransform(input);
|
||||
expect(text).not.toContain('import { StreamableHTTPError }');
|
||||
expect(text).toMatch(/import.*SdkError/);
|
||||
expect(text).toMatch(/import.*SdkHttpError/);
|
||||
});
|
||||
|
||||
it('is idempotent', () => {
|
||||
|
|
@ -249,9 +249,9 @@ describe('removed-apis transform', () => {
|
|||
''
|
||||
].join('\n');
|
||||
const { text, result } = applyTransform(input);
|
||||
expect(text).toContain('instanceof SdkError');
|
||||
expect(text).toContain('instanceof SdkHttpError');
|
||||
expect(text).not.toMatch(/\bSHE\b/);
|
||||
expect(text).toMatch(/import.*SdkError/);
|
||||
expect(text).toMatch(/import.*SdkHttpError/);
|
||||
const constructorWarning = result.diagnostics.find(d => d.message.includes('Constructor arguments differ'));
|
||||
expect(constructorWarning).toBeDefined();
|
||||
});
|
||||
|
|
|
|||
55
packages/core/test/errors/sdkHttpError.test.ts
Normal file
55
packages/core/test/errors/sdkHttpError.test.ts
Normal file
|
|
@ -0,0 +1,55 @@
|
|||
import { describe, it, expect } from 'vitest';
|
||||
import { SdkError, SdkErrorCode, SdkHttpError } from '../../src/index.js';
|
||||
|
||||
describe('SdkHttpError', () => {
|
||||
it('exposes status and statusText via getters', () => {
|
||||
const error = new SdkHttpError(SdkErrorCode.ClientHttpAuthentication, 'Unauthorized', {
|
||||
status: 401,
|
||||
statusText: 'Unauthorized'
|
||||
});
|
||||
|
||||
expect(error.status).toBe(401);
|
||||
expect(error.statusText).toBe('Unauthorized');
|
||||
});
|
||||
|
||||
it('returns undefined for statusText when omitted', () => {
|
||||
const error = new SdkHttpError(SdkErrorCode.ClientHttpAuthentication, 'auth failed', {
|
||||
status: 401
|
||||
});
|
||||
|
||||
expect(error.status).toBe(401);
|
||||
expect(error.statusText).toBeUndefined();
|
||||
});
|
||||
|
||||
it('is an instance of SdkError', () => {
|
||||
const error = new SdkHttpError(SdkErrorCode.ClientHttpForbidden, 'Forbidden', {
|
||||
status: 403,
|
||||
statusText: 'Forbidden'
|
||||
});
|
||||
|
||||
expect(error).toBeInstanceOf(SdkError);
|
||||
expect(error).toBeInstanceOf(SdkHttpError);
|
||||
});
|
||||
|
||||
it('preserves code and message from SdkError', () => {
|
||||
const error = new SdkHttpError(SdkErrorCode.ClientHttpNotImplemented, 'Not Implemented', {
|
||||
status: 501,
|
||||
statusText: 'Not Implemented'
|
||||
});
|
||||
|
||||
expect(error.code).toBe(SdkErrorCode.ClientHttpNotImplemented);
|
||||
expect(error.message).toBe('Not Implemented');
|
||||
expect(error.name).toBe('SdkHttpError');
|
||||
});
|
||||
|
||||
it('exposes extra data fields alongside status', () => {
|
||||
const error = new SdkHttpError(SdkErrorCode.ClientHttpAuthentication, 'auth failed', {
|
||||
status: 401,
|
||||
statusText: 'Unauthorized',
|
||||
retryAfter: 30
|
||||
});
|
||||
|
||||
expect(error.data.retryAfter).toBe(30);
|
||||
expect(error.status).toBe(401);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue