feat: improve mcp edge metadata and error handling

pull/715/head
Travis Fischer 2025-06-12 02:43:55 +07:00
rodzic 12d159f51d
commit f720323b5e
13 zmienionych plików z 193 dodań i 142 usunięć

Wyświetl plik

@ -1,5 +1,37 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
exports[`Basic MCP => OpenAPI @ 010332cf get_post success > 2.0: @dev/test-basic-openapi@010332cf/mcp get_post 1`] = `
{
"content": [],
"isError": false,
"structuredContent": {
"body": "dignissimos aperiam dolorem qui eum
facilis quibusdam animi sint suscipit qui sint possimus cum
quaerat magni maiores excepturi
ipsam ut commodi dolor voluptatum modi aut vitae",
"id": 8,
"title": "dolorem dolore est ipsam",
"userId": 1,
},
}
`;
exports[`Basic MCP => OpenAPI @ latest get_post success > 1.0: @dev/test-basic-openapi@latest/mcp get_post 1`] = `
{
"content": [],
"isError": false,
"structuredContent": {
"body": "et iusto sed quo iure
voluptatem occaecati omnis eligendi aut ad
voluptatem doloribus vel accusantium quis pariatur
molestiae porro eius odio et labore et velit aut",
"id": 3,
"title": "ea molestias quasi exercitationem repellat qui ipsa sit aut",
"userId": 1,
},
}
`;
exports[`Basic MCP => OpenAPI get_post success > 0.0: @dev/test-basic-openapi/mcp get_post 1`] = ` exports[`Basic MCP => OpenAPI get_post success > 0.0: @dev/test-basic-openapi/mcp get_post 1`] = `
{ {
"content": [], "content": [],

Wyświetl plik

@ -36,10 +36,15 @@ for (const [i, fixtureSuite] of fixtureSuites.entries()) {
fixture.response?.snapshot ?? fixture.response?.snapshot ??
fixtureSuite.snapshot ?? fixtureSuite.snapshot ??
(status >= 200 && status < 300) (status >= 200 && status < 300)
const debugFixture = !!(fixture.debug ?? fixtureSuite.debug) const debugFixture = !!(
fixture.debug ??
fixtureSuite.debug ??
fixture.only ??
fixtureSuite.only
)
const fixtureName = `${i}.${j}: ${method} ${fixture.path}` const fixtureName = `${i}.${j}: ${method} ${fixture.path}`
let testFn = fixture.only ? test.only : test let testFn = (fixture.only ?? fixture.debug) ? test.only : test
if (fixtureSuite.sequential) { if (fixtureSuite.sequential) {
testFn = testFn.sequential testFn = testFn.sequential
} }

Wyświetl plik

@ -111,7 +111,6 @@ export const fixtureSuites: E2ETestFixtureSuite[] = [
fixtures: [ fixtures: [
{ {
path: '@dev/test-basic-openapi/getPost', path: '@dev/test-basic-openapi/getPost',
response: { response: {
// Missing `postId` parameter. // Missing `postId` parameter.
status: 400 status: 400

Wyświetl plik

@ -33,14 +33,22 @@ for (const [i, fixtureSuite] of fixtureSuites.entries()) {
const { const {
isError, isError,
result: expectedResult, result: expectedResult,
content: expectedContent,
structuredContent: expectedStructuredContent,
_meta: expectedMeta,
validate validate
} = fixture.response ?? {} } = fixture.response ?? {}
const snapshot = const snapshot =
fixture.response?.snapshot ?? fixtureSuite.snapshot ?? !isError fixture.response?.snapshot ?? fixtureSuite.snapshot ?? !isError
const debugFixture = !!(fixture.debug ?? fixtureSuite.debug) const debugFixture = !!(
fixture.debug ??
fixtureSuite.debug ??
fixture.only ??
fixtureSuite.only
)
const fixtureName = `${i}.${j}: ${fixtureSuite.path} ${fixture.request.name}` const fixtureName = `${i}.${j}: ${fixtureSuite.path} ${fixture.request.name}`
let testFn = fixture.only ? test.only : test let testFn = (fixture.only ?? fixture.debug) ? test.only : test
if (fixtureSuite.sequential) { if (fixtureSuite.sequential) {
testFn = testFn.sequential testFn = testFn.sequential
} }
@ -60,6 +68,11 @@ for (const [i, fixtureSuite] of fixtureSuites.entries()) {
name: fixture.request.name, name: fixture.request.name,
arguments: fixture.request.args arguments: fixture.request.args
}) })
if (debugFixture) {
console.log(fixtureName, '=>', result)
}
if (isError) { if (isError) {
expect(result.isError).toBeTruthy() expect(result.isError).toBeTruthy()
} else { } else {
@ -70,6 +83,21 @@ for (const [i, fixtureSuite] of fixtureSuites.entries()) {
expect(result).toEqual(expectedResult) expect(result).toEqual(expectedResult)
} }
if (expectedContent) {
expect(result.content).toEqual(expectedContent)
}
if (expectedStructuredContent) {
expect(result.structuredContent).toEqual(expectedStructuredContent)
}
if (expectedMeta) {
expect(result._meta).toBeDefined()
for (const [key, value] of Object.entries(expectedMeta)) {
expect(result._meta![key]).toEqual(value)
}
}
if (snapshot) { if (snapshot) {
expect(result).toMatchSnapshot() expect(result).toMatchSnapshot()
} }
@ -85,10 +113,6 @@ for (const [i, fixtureSuite] of fixtureSuites.entries()) {
expect(result).toEqual(fixtureResult) expect(result).toEqual(fixtureResult)
} }
} }
if (debugFixture) {
console.log(fixtureName, '=>', result)
}
} }
) )
} }

Wyświetl plik

@ -14,8 +14,11 @@ export type MCPE2ETestFixture = {
} }
response?: { response?: {
isError?: boolean
result?: any result?: any
isError?: boolean
content?: Array<Record<string, unknown>>
structuredContent?: any
_meta?: Record<string, unknown>
validate?: (result: any) => void | Promise<void> validate?: (result: any) => void | Promise<void>
/** @default true */ /** @default true */
snapshot?: boolean snapshot?: boolean
@ -60,6 +63,34 @@ export const fixtureSuites: MCPE2ETestFixtureSuite[] = [
} }
] ]
}, },
{
title: 'Basic MCP => OpenAPI @ latest get_post success ',
path: '@dev/test-basic-openapi@latest/mcp',
fixtures: [
{
request: {
name: 'get_post',
args: {
postId: 3
}
}
}
]
},
{
title: 'Basic MCP => OpenAPI @ 010332cf get_post success ',
path: '@dev/test-basic-openapi@010332cf/mcp',
fixtures: [
{
request: {
name: 'get_post',
args: {
postId: 8
}
}
}
]
},
{ {
title: 'Basic MCP => MCP "echo" tool call success', title: 'Basic MCP => MCP "echo" tool call success',
path: '@dev/test-basic-mcp/mcp', path: '@dev/test-basic-mcp/mcp',
@ -75,14 +106,12 @@ export const fixtureSuites: MCPE2ETestFixtureSuite[] = [
} }
}, },
response: { response: {
result: { content: [
content: [ {
{ type: 'text',
type: 'text', text: JSON.stringify({ nala: 'kitten', num: 123, now })
text: JSON.stringify({ nala: 'kitten', num: 123, now }) }
} ]
]
}
} }
}, },
{ {
@ -95,18 +124,37 @@ export const fixtureSuites: MCPE2ETestFixtureSuite[] = [
} }
}, },
response: { response: {
result: { content: [
content: [ {
{ type: 'text',
type: 'text', text: JSON.stringify({
text: JSON.stringify({ nala: 'kitten',
nala: 'kitten', num: 123,
num: 123, now: `${now}`
now: `${now}` })
}) }
} ]
] }
}
]
},
{
title: 'Basic MCP => MCP "echo" tool call errors',
path: '@dev/test-basic-openapi/mcp',
snapshot: false,
only: true,
fixtures: [
{
request: {
name: 'get_post',
args: {
nala: 'kitten',
num: 123,
now
} }
},
response: {
isError: true
} }
} }
] ]

Wyświetl plik

@ -107,52 +107,49 @@ export class DurableMcpServerBase extends McpAgent<
waitUntil: this.ctx.waitUntil.bind(this.ctx) waitUntil: this.ctx.waitUntil.bind(this.ctx)
}) })
const { if (resolvedOriginToolCallResult.originResponse) {
originResponse,
toolCallResponse: resolvedToolCallResponse,
rateLimitResult
} = resolvedOriginToolCallResult
if (originResponse) {
toolCallResponse = await transformHttpResponseToMcpToolCallResponse({ toolCallResponse = await transformHttpResponseToMcpToolCallResponse({
tool, ...resolvedOriginToolCallResult,
...resolvedOriginToolCallResult tool
}) })
} else if (resolvedToolCallResponse) {
if (resolvedToolCallResponse._meta || rateLimitResult) {
toolCallResponse = {
...resolvedToolCallResponse,
_meta: {
...resolvedToolCallResponse._meta,
...pruneEmpty({
headers: rateLimitResult
? getRateLimitHeaders(rateLimitResult)
: undefined
})
}
}
} else {
toolCallResponse = resolvedToolCallResponse
}
} else { } else {
assert(false, 500) toolCallResponse = resolvedOriginToolCallResult.toolCallResponse
assert(toolCallResponse, 500, 'Missing tool call response')
} }
assert(toolCallResponse, 500, 'Missing tool call response')
return toolCallResponse return toolCallResponse
} catch (err: unknown) { } catch (err: unknown) {
// Gracefully handle tool call exceptions, whether they're thrown by the // Gracefully handle tool call exceptions, whether they're thrown by the
// origin or internally by the gateway. // origin or internally by the gateway.
toolCallResponse = handleMcpToolCallError(err, { toolCallResponse = handleMcpToolCallError(err, {
deployment,
consumer,
toolName, toolName,
sessionId,
env: this.env env: this.env
}) })
return toolCallResponse return toolCallResponse
} finally { } finally {
assert(toolCallResponse, 500, 'Missing tool call response')
// Augment the MCP tool call response with agentic metadata, which
// makes it easier to debug tool calls and adds some much-needed HTTP
// header-like functionality to tool call responses.
toolCallResponse._meta = {
...toolCallResponse._meta,
agentic: pruneEmpty({
...(toolCallResponse._meta?.agentic as any),
deploymentId: deployment.id,
consumerId: consumer?.id,
cacheStatus: resolvedOriginToolCallResult?.cacheStatus,
toolName,
headers: {
...(toolCallResponse._meta?.agentic as any)?.headers,
...getRateLimitHeaders(
resolvedOriginToolCallResult?.rateLimitResult
)
}
})
}
// Record tool call usage, whether the call was successful or not. // Record tool call usage, whether the call was successful or not.
recordToolCallUsage({ recordToolCallUsage({
...this.props, ...this.props,

Wyświetl plik

@ -1,44 +1,25 @@
import type { AdminDeployment } from '@agentic/platform-types'
import type { ContentfulStatusCode } from 'hono/utils/http-status' import type { ContentfulStatusCode } from 'hono/utils/http-status'
import { import { HttpError } from '@agentic/platform-core'
getRateLimitHeaders,
HttpError,
pruneEmpty
} from '@agentic/platform-core'
import * as Sentry from '@sentry/cloudflare' import * as Sentry from '@sentry/cloudflare'
import { HTTPException } from 'hono/http-exception' import { HTTPException } from 'hono/http-exception'
import { HTTPError } from 'ky' import { HTTPError } from 'ky'
import type { RawEnv } from './env' import type { RawEnv } from './env'
import type { import type { McpToolCallResponse } from './types'
AdminConsumer,
McpToolCallResponse,
RateLimitResult
} from './types'
/** /**
* Turns a thrown error into an MCP error tool call response, and attempts to * Turns a thrown error into an MCP error tool call response, and attempts to
* capture as much context as possible for potential debugging. * capture as much context as possible for potential debugging.
* *
* @note This function is synchronous and must never throw. * @note This function is synchronous and should never throw.
*/ */
export function handleMcpToolCallError( export function handleMcpToolCallError(
err: any, err: any,
{ {
deployment,
consumer,
toolName, toolName,
sessionId,
requestId,
rateLimitResult,
env env
}: { }: {
deployment: AdminDeployment
consumer?: AdminConsumer
toolName: string toolName: string
sessionId: string
requestId?: string
rateLimitResult?: RateLimitResult
env: RawEnv env: RawEnv
} }
): McpToolCallResponse { ): McpToolCallResponse {
@ -47,16 +28,6 @@ export function handleMcpToolCallError(
let status: ContentfulStatusCode = 500 let status: ContentfulStatusCode = 500
const res: McpToolCallResponse = { const res: McpToolCallResponse = {
_meta: pruneEmpty({
deploymentId: deployment.id,
consumerId: consumer?.id,
toolName,
sessionId,
requestId,
headers: rateLimitResult
? getRateLimitHeaders(rateLimitResult)
: undefined
}),
isError: true, isError: true,
content: [ content: [
{ {

Wyświetl plik

@ -276,7 +276,7 @@ export async function resolveOriginToolCall({
body: JSON.stringify({ body: JSON.stringify({
name: tool.name, name: tool.name,
args: toolCallArgs, args: toolCallArgs,
metadata: originMcpRequestMetadata! metadata: originMcpRequestMetadata
}) })
}) })
@ -301,7 +301,7 @@ export async function resolveOriginToolCall({
const toolCallResponseString = await originMcpClient.callTool({ const toolCallResponseString = await originMcpClient.callTool({
name: tool.name, name: tool.name,
args: toolCallArgs, args: toolCallArgs,
metadata: originMcpRequestMetadata! metadata: originMcpRequestMetadata
}) })
const toolCallResponse = JSON.parse( const toolCallResponse = JSON.parse(
toolCallResponseString toolCallResponseString

Wyświetl plik

@ -1,36 +1,26 @@
import type { Tool } from '@agentic/platform-types' import type { Tool } from '@agentic/platform-types'
import { import { assert, HttpError } from '@agentic/platform-core'
assert,
getRateLimitHeaders,
HttpError,
pruneEmpty
} from '@agentic/platform-core'
import contentType from 'fast-content-type-parse' import contentType from 'fast-content-type-parse'
import type { import type { McpToolCallResponse, ToolCallArgs } from './types'
McpToolCallResponse,
RateLimitResult,
ToolCallArgs
} from './types'
import { cfValidateJsonSchema } from './cf-validate-json-schema' import { cfValidateJsonSchema } from './cf-validate-json-schema'
export async function transformHttpResponseToMcpToolCallResponse({ export async function transformHttpResponseToMcpToolCallResponse({
originRequest, originRequest,
originResponse, originResponse,
tool, tool,
toolCallArgs, toolCallArgs
rateLimitResult
}: { }: {
originRequest: Request originRequest: Request
originResponse: Response originResponse: Response
tool: Tool tool: Tool
toolCallArgs: ToolCallArgs toolCallArgs: ToolCallArgs
rateLimitResult?: RateLimitResult
}) { }) {
const { type: mimeType } = contentType.safeParse( const { type: mimeType } = contentType.safeParse(
originResponse.headers.get('content-type') || 'application/octet-stream' originResponse.headers.get('content-type') || 'application/octet-stream'
) )
// TODO: move these logs should be higher up
// eslint-disable-next-line no-console // eslint-disable-next-line no-console
console.log('httpOriginResponse', { console.log('httpOriginResponse', {
tool: tool.name, tool: tool.name,
@ -41,8 +31,7 @@ export async function transformHttpResponseToMcpToolCallResponse({
mimeType, mimeType,
status: originResponse.status status: originResponse.status
// headers: Object.fromEntries(originResponse.headers.entries()) // headers: Object.fromEntries(originResponse.headers.entries())
}, }
rateLimitResult
}) })
if (originResponse.status >= 400) { if (originResponse.status >= 400) {
@ -62,22 +51,19 @@ export async function transformHttpResponseToMcpToolCallResponse({
status: originResponse.status, status: originResponse.status,
// headers: Object.fromEntries(originResponse.headers.entries()), // headers: Object.fromEntries(originResponse.headers.entries()),
message message
}, }
rateLimitResult
}) })
throw new HttpError({ throw new HttpError({
statusCode: originResponse.status, statusCode: originResponse.status,
message, message,
cause: originResponse, cause: originResponse
headers: getRateLimitHeaders(rateLimitResult)
}) })
} }
const result: McpToolCallResponse = pruneEmpty({ const result: McpToolCallResponse = {
isError: originResponse.status >= 400, isError: originResponse.status >= 400
_meta: getRateLimitHeaders(rateLimitResult) }
})
if (tool.outputSchema) { if (tool.outputSchema) {
assert( assert(

Wyświetl plik

@ -26,15 +26,3 @@ export function getRateLimitHeaders(
return headers return headers
} }
export function applyRateLimitHeaders(
rateLimitResult: RateLimitResult,
res: Response
) {
const rateLimitHeaders = getRateLimitHeaders(rateLimitResult)
if (!rateLimitHeaders) return
for (const [key, value] of Object.entries(rateLimitHeaders)) {
res.headers.set(key, value)
}
}

Wyświetl plik

@ -16,7 +16,7 @@ import {
* Hono error handler that sanitizes all types of internal, http, json-rpc, and * Hono error handler that sanitizes all types of internal, http, json-rpc, and
* unexpected errors and responds with an appropate HTTP Response. * unexpected errors and responds with an appropate HTTP Response.
* *
* @note This function is synchronous and must never throw. * @note This function is synchronous and should never throw.
*/ */
export function errorHandler( export function errorHandler(
err: Error | HTTPResponseError, err: Error | HTTPResponseError,

Wyświetl plik

@ -3,18 +3,6 @@ import {
type RateLimitResult type RateLimitResult
} from '@agentic/platform-core' } from '@agentic/platform-core'
export function applyRateLimitHeaders({
res,
rateLimitResult
}: {
res: Response
rateLimitResult?: RateLimitResult
}) {
const rateLimitHeaders = getRateLimitHeaders(rateLimitResult)
applyHeaders({ res, headers: rateLimitHeaders })
}
export function applyHeaders({ export function applyHeaders({
res, res,
headers headers
@ -28,3 +16,15 @@ export function applyHeaders({
res.headers.set(key, value) res.headers.set(key, value)
} }
} }
export function applyRateLimitHeaders({
res,
rateLimitResult
}: {
res: Response
rateLimitResult?: RateLimitResult
}) {
const rateLimitHeaders = getRateLimitHeaders(rateLimitResult)
applyHeaders({ res, headers: rateLimitHeaders })
}

Wyświetl plik

@ -42,6 +42,7 @@
- **Public MCP server interface** - **Public MCP server interface**
- how does oauth work with this flow? - how does oauth work with this flow?
- proper error handling support within this flow; will currently get generic errors - proper error handling support within this flow; will currently get generic errors
- pass requestId to DurableMcpServer somehow on a per-request basis
- **Origin MCP servers** - **Origin MCP servers**
- how to guarantee that the request is coming from agentic? - how to guarantee that the request is coming from agentic?
- `_meta` for tool calls - `_meta` for tool calls