diff --git a/.changeset/mcp-structured-error-classification.md b/.changeset/mcp-structured-error-classification.md new file mode 100644 index 00000000..036c3f3e --- /dev/null +++ b/.changeset/mcp-structured-error-classification.md @@ -0,0 +1,9 @@ +--- +'lens': patch +--- + +Structure-first error classification for MCP `paid_call` handler: +- Inspect error structure, Horizon result codes (`op_no_trust`, `op_underfunded`, `tx_bad_auth`), HTTP status codes (402, 401, 503), and socket/timeout codes before message text inspection (resolves #410) +- Prevent fragile string matching misclassifications (e.g. messages containing the word "fetch" with underlying trustline errors now correctly map to `ERR_MISSING_TRUSTLINE` rather than `ERR_SERVICE_UNREACHABLE`) (resolves #410) +- Maintain text matching purely as an unstructured fallback for legacy third-party errors (resolves #410) +- Comprehensive unit tests covering Horizon operation codes, transaction codes, HTTP statuses, and anti-fragility against reworded messages (resolves #410) diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 8a6cd7de..7d1f76c9 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -292,30 +292,173 @@ export class BazaarMcpServer { }); } - private handlePaidCallError(e: any) { + /** + * Classify paid_call failures by inspecting error structure and codes first + * (Horizon result_codes, HTTP status codes, socket/network codes), using + * message substring matching only as a last-resort fallback for unstructured errors. + */ + public classifyPaidCallError(e: any): { code: McpErrorCode; reason: string } { if (isTimeoutError(e)) { - return this.createErrorResponse('ERR_SERVICE_UNREACHABLE', `Upstream did not respond within ${UPSTREAM_TIMEOUT_MS}ms`); + return { + code: 'ERR_SERVICE_UNREACHABLE', + reason: `Upstream did not respond within ${UPSTREAM_TIMEOUT_MS}ms`, + }; + } + + if (!e || typeof e !== 'object') { + const text = String(e || ''); + return { + code: 'ERR_INTERNAL_FAILURE', + reason: text || 'An unknown error occurred during the paid call.', + }; + } + + // 1. Inspect Horizon transaction failure codes (e.response.data.extras.result_codes or e.resultCodes) + const resultCodes = + e.response?.data?.extras?.result_codes ?? + e.data?.extras?.result_codes ?? + e.resultCodes ?? + e.extras?.result_codes; + + if (resultCodes) { + const txCode = typeof resultCodes.transaction === 'string' ? resultCodes.transaction : ''; + const opCodes: string[] = Array.isArray(resultCodes.operations) ? resultCodes.operations : []; + + if (opCodes.includes('op_no_trust')) { + return { + code: 'ERR_MISSING_TRUSTLINE', + reason: 'Missing trustline for the required asset.', + }; + } + + if ( + opCodes.includes('op_underfunded') || + opCodes.includes('op_low_reserve') || + txCode === 'tx_insufficient_balance' || + txCode === 'tx_insufficient_fee' + ) { + return { + code: 'ERR_INSUFFICIENT_BALANCE', + reason: 'Insufficient balance to complete the payment.', + }; + } + + if ( + txCode === 'tx_bad_auth' || + txCode === 'tx_bad_seq' || + txCode === 'tx_too_late' || + opCodes.includes('op_bad_auth') + ) { + return { + code: 'ERR_EXPIRED_AUTHORISATION', + reason: 'Transaction authorisation expired or signature rejected.', + }; + } + } + + // 2. Inspect explicit ledger / expiration fields on the error + if ( + ('signatureExpirationLedger' in e && 'currentLedger' in e) || + e.code === 'ERR_SIGNATURE_EXPIRED' || + e.code === 'SIGNATURE_EXPIRED' + ) { + return { + code: 'ERR_EXPIRED_AUTHORISATION', + reason: 'The transaction signature expired before it could be submitted.', + }; + } + + // 3. Inspect HTTP status codes from x402 client / upstream responses + const status = + typeof e.status === 'number' + ? e.status + : typeof e.statusCode === 'number' + ? e.statusCode + : typeof e.response?.status === 'number' + ? e.response.status + : undefined; + + if (status !== undefined) { + if (status === 402) { + return { + code: 'ERR_PAYMENT_REJECTED', + reason: 'Payment required but was rejected or failed.', + }; + } + if (status === 401 || status === 403) { + return { + code: 'ERR_EXPIRED_AUTHORISATION', + reason: 'Authorisation token invalid or expired.', + }; + } + if (status === 502 || status === 503 || status === 504) { + return { + code: 'ERR_SERVICE_UNREACHABLE', + reason: `Service returned gateway error with status ${status}`, + }; + } + if (status >= 400 && status < 500) { + return { + code: 'ERR_BAD_REQUEST', + reason: `Request rejected with client status ${status}`, + }; + } + } + + // 4. Inspect network error codes and causes (Node.js/fetch socket errors) + const errCode = e.code || e.cause?.code; + const isNetworkCode = + typeof errCode === 'string' && + ['ENOTFOUND', 'ECONNREFUSED', 'ECONNRESET', 'ETIMEDOUT', 'EAI_AGAIN', 'UND_ERR_CONNECT_TIMEOUT'].includes(errCode); + const isTimeout = e.name === 'TimeoutError' || e.name === 'AbortError'; + + if (isNetworkCode || isTimeout) { + return { + code: 'ERR_SERVICE_UNREACHABLE', + reason: `Failed to reach the service: ${e.message || errCode || 'Network error'}`, + }; } - const msg = String(e.message || e).toLowerCase(); + // 5. Fallback: string matching on message text (only for unstructured errors) + const msg = String(e.message || '').toLowerCase(); if (msg.includes('signatureexpirationledger') || msg.includes('signature expired')) { - return this.createErrorResponse('ERR_EXPIRED_AUTHORISATION', 'The transaction signature expired before it could be submitted.'); + return { + code: 'ERR_EXPIRED_AUTHORISATION', + reason: 'The transaction signature expired before it could be submitted.', + }; } - + if (msg.includes('trustline') || msg.includes('op_no_trust')) { - return this.createErrorResponse('ERR_MISSING_TRUSTLINE', 'Missing trustline for the required asset.'); + return { + code: 'ERR_MISSING_TRUSTLINE', + reason: 'Missing trustline for the required asset.', + }; } - + if (msg.includes('insufficient') || msg.includes('underfunded')) { - return this.createErrorResponse('ERR_INSUFFICIENT_BALANCE', 'Insufficient balance to complete the payment.'); + return { + code: 'ERR_INSUFFICIENT_BALANCE', + reason: 'Insufficient balance to complete the payment.', + }; } - if (msg.includes('fetch') || msg.includes('network')) { - return this.createErrorResponse('ERR_SERVICE_UNREACHABLE', 'Failed to reach the service: ' + e.message); + if (msg.includes('fetch') || msg.includes('network') || msg.includes('connection refused')) { + return { + code: 'ERR_SERVICE_UNREACHABLE', + reason: 'Failed to reach the service: ' + (e.message || msg), + }; } - - return this.createErrorResponse('ERR_INTERNAL_FAILURE', e.message || 'An unknown error occurred during the paid call.'); + + return { + code: 'ERR_INTERNAL_FAILURE', + reason: e.message || 'An unknown error occurred during the paid call.', + }; + } + + private handlePaidCallError(e: any) { + const { code, reason } = this.classifyPaidCallError(e); + return this.createErrorResponse(code, reason); } private createErrorResponse(code: McpErrorCode, reason: string) { diff --git a/tests/mcp.test.ts b/tests/mcp.test.ts index e6ee7007..f2a46fb9 100644 --- a/tests/mcp.test.ts +++ b/tests/mcp.test.ts @@ -18,122 +18,316 @@ describe('BazaarMcpServer — server identity', () => { }); describe('BazaarMcpServer', () => { - it('should map signature expiration ledger error to ERR_EXPIRED_AUTHORISATION', async () => { - const mockFetch = vi.fn().mockRejectedValue(new Error('Transaction signature expired. Signature expiration ledger 123 is less than current ledger 125')); - const server = new BazaarMcpServer({ fetchWithPayment: mockFetch, paidCallAllowlist: ['example.com'] }); + describe('baseline paid_call error tests', () => { + it('should map signature expiration ledger error to ERR_EXPIRED_AUTHORISATION', async () => { + const mockFetch = vi.fn().mockRejectedValue(new Error('Transaction signature expired. Signature expiration ledger 123 is less than current ledger 125')); + const server = new BazaarMcpServer({ fetchWithPayment: mockFetch, paidCallAllowlist: ['example.com'] }); - // The SDK server doesn't expose the router directly without a client, but we can test the handlePaidCallError method directly, - // or by overriding setRequestHandler to capture our handler. - let handler: any = null; - (server as any).server.setRequestHandler = (schema: any, fn: any) => { - if (schema === CallToolRequestSchema) { - handler = fn; - } - }; - - // Re-setup handlers to capture the handler - (server as any).setupHandlers(); - - const response = await handler({ - params: { - name: 'paid_call', - arguments: { - url: 'http://example.com/paid' + let handler: any = null; + (server as any).server.setRequestHandler = (schema: any, fn: any) => { + if (schema === CallToolRequestSchema) { + handler = fn; + } + }; + (server as any).setupHandlers(); + + const response = await handler({ + params: { + name: 'paid_call', + arguments: { + url: 'http://example.com/paid' + } } - } + }); + + expect(response.isError).toBe(true); + const content = JSON.parse(response.content[0].text); + expect(content.code).toBe('ERR_EXPIRED_AUTHORISATION'); }); - expect(response.isError).toBe(true); - const content = JSON.parse(response.content[0].text); - expect(content.code).toBe('ERR_EXPIRED_AUTHORISATION'); - }); + it('should map missing trustline error to ERR_MISSING_TRUSTLINE', async () => { + const mockFetch = vi.fn().mockRejectedValue(new Error('op_no_trust: The account does not have a trustline for this asset')); + const server = new BazaarMcpServer({ fetchWithPayment: mockFetch, paidCallAllowlist: ['example.com'] }); - it('should map missing trustline error to ERR_MISSING_TRUSTLINE', async () => { - const mockFetch = vi.fn().mockRejectedValue(new Error('op_no_trust: The account does not have a trustline for this asset')); - const server = new BazaarMcpServer({ fetchWithPayment: mockFetch, paidCallAllowlist: ['example.com'] }); + let handler: any = null; + (server as any).server.setRequestHandler = (schema: any, fn: any) => { + if (schema === CallToolRequestSchema) { + handler = fn; + } + }; + (server as any).setupHandlers(); + + const response = await handler({ + params: { + name: 'paid_call', + arguments: { + url: 'http://example.com/paid' + } + } + }); - let handler: any = null; - (server as any).server.setRequestHandler = (schema: any, fn: any) => { - if (schema === CallToolRequestSchema) { - handler = fn; - } - }; - (server as any).setupHandlers(); + expect(response.isError).toBe(true); + const content = JSON.parse(response.content[0].text); + expect(content.code).toBe('ERR_MISSING_TRUSTLINE'); + }); - const response = await handler({ - params: { - name: 'paid_call', - arguments: { - url: 'http://example.com/paid' + it('should map 402 rejected error to ERR_PAYMENT_REJECTED', async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: false, + status: 402, + text: async () => 'Payment Required' + }); + + const server = new BazaarMcpServer({ fetchWithPayment: mockFetch, paidCallAllowlist: ['example.com'] }); + + let handler: any = null; + (server as any).server.setRequestHandler = (schema: any, fn: any) => { + if (schema === CallToolRequestSchema) { + handler = fn; + } + }; + (server as any).setupHandlers(); + + const response = await handler({ + params: { + name: 'paid_call', + arguments: { + url: 'http://example.com/paid' + } } - } + }); + + expect(response.isError).toBe(true); + const content = JSON.parse(response.content[0].text); + expect(content.code).toBe('ERR_PAYMENT_REJECTED'); }); + + it('should return successfully on a successful call', async () => { + const mockFetch = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + text: async () => JSON.stringify({ success: true, data: "hello" }) + }); + + const server = new BazaarMcpServer({ fetchWithPayment: mockFetch, paidCallAllowlist: ['example.com'] }); - expect(response.isError).toBe(true); - const content = JSON.parse(response.content[0].text); - expect(content.code).toBe('ERR_MISSING_TRUSTLINE'); + let handler: any = null; + (server as any).server.setRequestHandler = (schema: any, fn: any) => { + if (schema === CallToolRequestSchema) { + handler = fn; + } + }; + (server as any).setupHandlers(); + + const response = await handler({ + params: { + name: 'paid_call', + arguments: { + url: 'http://example.com/paid' + } + } + }); + + expect(response.isError).toBeFalsy(); + const parsedText = JSON.parse(response.content[0].text); + expect(parsedText.success).toBe(true); + }); }); - it('should map 402 rejected error to ERR_PAYMENT_REJECTED', async () => { - const mockFetch = vi.fn().mockResolvedValue({ - ok: false, - status: 402, - text: async () => 'Payment Required' + describe('Structure-first error classification (Issue #410)', () => { + it('should classify Horizon op_no_trust by result_codes structure without relying on message text', async () => { + const horizonError = { + message: 'A customized failure happened on submission', // completely reworded, no "trustline" word + response: { + status: 400, + data: { + extras: { + result_codes: { + transaction: 'tx_failed', + operations: ['op_no_trust'] + } + } + } + } + }; + + const server = new BazaarMcpServer({ + fetchWithPayment: vi.fn().mockRejectedValue(horizonError), + paidCallAllowlist: ['bazaar.org'], + }); + let handler: any = null; + (server as any).server.setRequestHandler = (schema: any, fn: any) => { + if (schema === CallToolRequestSchema) handler = fn; + }; + (server as any).setupHandlers(); + + const response = await handler({ + params: { name: 'paid_call', arguments: { url: 'https://bazaar.org/pay' } } + }); + + expect(response.isError).toBe(true); + const content = JSON.parse(response.content[0].text); + expect(content.code).toBe('ERR_MISSING_TRUSTLINE'); + expect(content.reason).toBe('Missing trustline for the required asset.'); }); - - const server = new BazaarMcpServer({ fetchWithPayment: mockFetch, paidCallAllowlist: ['example.com'] }); - let handler: any = null; - (server as any).server.setRequestHandler = (schema: any, fn: any) => { - if (schema === CallToolRequestSchema) { - handler = fn; - } - }; - (server as any).setupHandlers(); + it('should classify Horizon op_underfunded and tx_insufficient_balance by result_codes structure', async () => { + const underfundedError = { + message: 'Balance error', + resultCodes: { + transaction: 'tx_failed', + operations: ['op_underfunded'] + } + }; + + const server = new BazaarMcpServer({ + fetchWithPayment: vi.fn().mockRejectedValue(underfundedError), + paidCallAllowlist: ['bazaar.org'], + }); + let handler: any = null; + (server as any).server.setRequestHandler = (schema: any, fn: any) => { + if (schema === CallToolRequestSchema) handler = fn; + }; + (server as any).setupHandlers(); + + const response = await handler({ + params: { name: 'paid_call', arguments: { url: 'https://bazaar.org/pay' } } + }); - const response = await handler({ - params: { - name: 'paid_call', - arguments: { - url: 'http://example.com/paid' + expect(response.isError).toBe(true); + const content = JSON.parse(response.content[0].text); + expect(content.code).toBe('ERR_INSUFFICIENT_BALANCE'); + expect(content.reason).toBe('Insufficient balance to complete the payment.'); + + // Also test transaction level code + const txFeeError = { + message: 'Fee error', + data: { + extras: { + result_codes: { + transaction: 'tx_insufficient_fee', + operations: [] + } + } } - } + }; + + const feeRes = (server as any).classifyPaidCallError(txFeeError); + expect(feeRes.code).toBe('ERR_INSUFFICIENT_BALANCE'); }); - expect(response.isError).toBe(true); - const content = JSON.parse(response.content[0].text); - expect(content.code).toBe('ERR_PAYMENT_REJECTED'); - }); - - it('should return successfully on a successful call', async () => { - const mockFetch = vi.fn().mockResolvedValue({ - ok: true, - status: 200, - text: async () => JSON.stringify({ success: true, data: "hello" }) + it('should classify Horizon tx_bad_auth and tx_too_late by structure', async () => { + const server = new BazaarMcpServer({ fetchWithPayment: vi.fn() }); + const badAuthError = { + response: { + data: { + extras: { + result_codes: { + transaction: 'tx_bad_auth' + } + } + } + } + }; + + const res = (server as any).classifyPaidCallError(badAuthError); + expect(res.code).toBe('ERR_EXPIRED_AUTHORISATION'); + + const tooLateError = { + resultCodes: { + transaction: 'tx_too_late' + } + }; + const tooLateRes = (server as any).classifyPaidCallError(tooLateError); + expect(tooLateRes.code).toBe('ERR_EXPIRED_AUTHORISATION'); }); - - const server = new BazaarMcpServer({ fetchWithPayment: mockFetch, paidCallAllowlist: ['example.com'] }); - let handler: any = null; - (server as any).server.setRequestHandler = (schema: any, fn: any) => { - if (schema === CallToolRequestSchema) { - handler = fn; - } - }; - (server as any).setupHandlers(); + it('should classify explicit signature expiration fields on the error object', async () => { + const server = new BazaarMcpServer({ fetchWithPayment: vi.fn() }); + const expirationObj = { + signatureExpirationLedger: 500, + currentLedger: 505, + message: 'Ledger limit exceeded' // no "expired" word + }; + + const res = (server as any).classifyPaidCallError(expirationObj); + expect(res.code).toBe('ERR_EXPIRED_AUTHORISATION'); + expect(res.reason).toContain('signature expired'); + }); + + it('should classify x402 HTTP status codes directly without inspecting message text', async () => { + const server = new BazaarMcpServer({ fetchWithPayment: vi.fn() }); + + // HTTP 402 + const err402 = { status: 402, message: 'Unspecified challenge failure' }; + expect((server as any).classifyPaidCallError(err402).code).toBe('ERR_PAYMENT_REJECTED'); + + // HTTP 401 + const err401 = { statusCode: 401, message: 'Bad bearer token' }; + expect((server as any).classifyPaidCallError(err401).code).toBe('ERR_EXPIRED_AUTHORISATION'); + + // HTTP 503 + const err503 = { response: { status: 503 }, message: 'Server temporarily overloaded' }; + expect((server as any).classifyPaidCallError(err503).code).toBe('ERR_SERVICE_UNREACHABLE'); + + // HTTP 400 + const err400 = { status: 400, message: 'Bad request headers' }; + expect((server as any).classifyPaidCallError(err400).code).toBe('ERR_BAD_REQUEST'); + }); + + it('should classify socket and network error codes directly without false positive string matches', async () => { + const server = new BazaarMcpServer({ fetchWithPayment: vi.fn() }); + + const econnError = { code: 'ECONNREFUSED', message: 'Connection refused' }; + expect((server as any).classifyPaidCallError(econnError).code).toBe('ERR_SERVICE_UNREACHABLE'); - const response = await handler({ - params: { - name: 'paid_call', - arguments: { - url: 'http://example.com/paid' + const dnsError = { cause: { code: 'ENOTFOUND' }, message: 'Address not found' }; + expect((server as any).classifyPaidCallError(dnsError).code).toBe('ERR_SERVICE_UNREACHABLE'); + + const timeoutError = { name: 'TimeoutError', message: 'Request exceeded deadline' }; + expect((server as any).classifyPaidCallError(timeoutError).code).toBe('ERR_SERVICE_UNREACHABLE'); + }); + + it('should not misclassify an error containing "fetch" when structured codes indicate trustline or balance', async () => { + const server = new BazaarMcpServer({ fetchWithPayment: vi.fn() }); + + // In the old code, msg.includes('fetch') would falsely report this as ERR_SERVICE_UNREACHABLE + const misleadingMessageError = { + message: 'Failed to fetch account balance due to op_no_trust on destination asset', + response: { + data: { + extras: { + result_codes: { + operations: ['op_no_trust'] + } + } + } } - } + }; + + const res = (server as any).classifyPaidCallError(misleadingMessageError); + expect(res.code).toBe('ERR_MISSING_TRUSTLINE'); + expect(res.code).not.toBe('ERR_SERVICE_UNREACHABLE'); }); - expect(response.isError).toBeFalsy(); - const parsedText = JSON.parse(response.content[0].text); - expect(parsedText.success).toBe(true); + it('should fall back to message text matching when error is unstructured', async () => { + const server = new BazaarMcpServer({ fetchWithPayment: vi.fn() }); + + const unstructuredTrustline = new Error('op_no_trust was encountered'); + expect((server as any).classifyPaidCallError(unstructuredTrustline).code).toBe('ERR_MISSING_TRUSTLINE'); + + const unstructuredFunding = new Error('The account is underfunded'); + expect((server as any).classifyPaidCallError(unstructuredFunding).code).toBe('ERR_INSUFFICIENT_BALANCE'); + + const unstructuredExpired = new Error('Signature expired'); + expect((server as any).classifyPaidCallError(unstructuredExpired).code).toBe('ERR_EXPIRED_AUTHORISATION'); + + const unstructuredNetwork = new Error('Failed to fetch from host'); + expect((server as any).classifyPaidCallError(unstructuredNetwork).code).toBe('ERR_SERVICE_UNREACHABLE'); + + const unknownError = new Error('Completely random unmapped exception'); + expect((server as any).classifyPaidCallError(unknownError).code).toBe('ERR_INTERNAL_FAILURE'); + }); }); });