Skip to content

Commit d8467f4

Browse files
committed
diag(mcp): reuse sanitizeUrlForLog + cancel log reader on abort
Review fixes (Cursor): - Replace the local safeUrl helper with the shared sanitizeUrlForLog so URL redaction/truncation stays consistent. - The detached initialize-body log reader now observes init.signal and cancels its tee-branch reader on abort, so it can't keep the response stream / connection alive after the SDK's initialize timeout gives up (the hang we're tracing).
1 parent b5edb9e commit d8467f4

1 file changed

Lines changed: 16 additions & 10 deletions

File tree

apps/sim/lib/mcp/http-diagnostics.ts

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { FetchLike } from '@modelcontextprotocol/sdk/shared/transport.js'
22
import { createLogger } from '@sim/logger'
33
import { sanitizeForLogging } from '@/lib/core/security/redaction'
4+
import { sanitizeUrlForLog } from '@/lib/core/utils/logging'
45
import { getMcpSafeErrorDiagnostics } from '@/lib/mcp/error-diagnostics'
56

67
const logger = createLogger('McpHttpDiag')
@@ -28,15 +29,8 @@ function diagnosticsEnabled(): boolean {
2829
return process.env.MCP_HTTP_DIAGNOSTICS !== 'false'
2930
}
3031

31-
/** Origin + path only — query values can carry `code`/`token`/`api_key`, so they're dropped. */
32-
function safeUrl(input: string | URL | Request): string {
33-
const raw = typeof input === 'string' ? input : input instanceof URL ? input.href : input.url
34-
try {
35-
const u = new URL(raw)
36-
return u.search ? `${u.origin}${u.pathname}?<redacted>` : `${u.origin}${u.pathname}`
37-
} catch {
38-
return '<unparseable-url>'
39-
}
32+
function rawUrl(input: string | URL | Request): string {
33+
return typeof input === 'string' ? input : input instanceof URL ? input.href : input.url
4034
}
4135

4236
/**
@@ -65,7 +59,7 @@ export function withMcpHttpDiagnostics(
6559
if (!diagnosticsEnabled()) return fetchFn
6660

6761
return async (input, init) => {
68-
const url = safeUrl(input as string | URL | Request)
62+
const url = sanitizeUrlForLog(rawUrl(input as string | URL | Request))
6963
const method = init?.method ?? 'GET'
7064
const reqHeaders = new Headers((init?.headers as HeadersInit | undefined) ?? undefined)
7165
const startedAt = Date.now()
@@ -113,8 +107,18 @@ export function withMcpHttpDiagnostics(
113107
return res
114108
}
115109

110+
// Cancel the detached log reader when the caller aborts (e.g. the SDK's 30s
111+
// initialize timeout — exactly the hang we're tracing) so this tee branch can't
112+
// keep the response stream / connection alive after the SDK has given up.
113+
const signal = init?.signal
116114
void (async () => {
117115
const reader = logBranch.getReader()
116+
const cancelReader = () => void reader.cancel().catch(() => {})
117+
if (signal?.aborted) {
118+
cancelReader()
119+
return
120+
}
121+
signal?.addEventListener('abort', cancelReader, { once: true })
118122
const decoder = new TextDecoder()
119123
let chunks = 0
120124
let bytes = 0
@@ -150,6 +154,8 @@ export function withMcpHttpDiagnostics(
150154
ms: Date.now() - startedAt,
151155
error: getMcpSafeErrorDiagnostics(error),
152156
})
157+
} finally {
158+
signal?.removeEventListener('abort', cancelReader)
153159
}
154160
})()
155161

0 commit comments

Comments
 (0)