From 376ca88397ffda3c927b000eca280346ee418224 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 5 May 2026 15:09:16 +0000 Subject: [PATCH 1/2] fix: Downgrade SSE controller-closed write errors to softFail WebStandardStreamableHTTPServerTransport.writeSSEEvent throws ERR_INVALID_STATE ("Invalid state: Controller is already closed") when the SSE ReadableStream controller was closed by client cancellation before a queued event was flushed. The SDK forwards it to onerror. This is client-disconnect noise, not a server fault, so route it through the same softFail branch as the other known disconnect patterns to avoid Mezmo error alerts (#793). https://claude.ai/code/session_01LpjxNt1eBv8qBZcKLozmj1 --- src/mcp/server.ts | 5 +- tests/unit/mcp.server.error_handling.test.ts | 71 ++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 tests/unit/mcp.server.error_handling.test.ts diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 947f6609..94e4bc36 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -535,8 +535,11 @@ export class ActorsMcpServer { // - "Failed to send response: Error: Not connected" (client vanished mid-flight) // - "Conflict: Only one SSE stream is allowed per session" (duplicate GET on the // streamable-http transport, e.g. tab refresh before the old SSE controller is GC'd) + // - "Invalid state: Controller is already closed" (ERR_INVALID_STATE from + // WebStandardStreamableHTTPServerTransport.writeSSEEvent when the SSE ReadableStream + // controller was closed by client cancellation before a queued event was flushed) // All are expected; log as softFail so they don't flood Mezmo error alerts. - if (/Not connected|No connection established|Only one SSE stream/i.test(error.message ?? '')) { + if (/Not connected|No connection established|Only one SSE stream|Controller is already closed/i.test(error.message ?? '')) { // Mezmo (logDNA) promotes log entries to errors when the message contains "error". // Use errMessage key and sanitize the string to preserve the soft-fail log level. const errMessage = (error.message ?? '').replace(/ error:/gi, ' failure:'); diff --git a/tests/unit/mcp.server.error_handling.test.ts b/tests/unit/mcp.server.error_handling.test.ts new file mode 100644 index 00000000..7c645f0f --- /dev/null +++ b/tests/unit/mcp.server.error_handling.test.ts @@ -0,0 +1,71 @@ +import { InMemoryTaskStore } from '@modelcontextprotocol/sdk/experimental/tasks/stores/in-memory.js'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import log from '@apify/log'; + +import { ActorsMcpServer } from '../../src/mcp/server.js'; + +function makeServer(): ActorsMcpServer { + return new ActorsMcpServer({ + taskStore: new InMemoryTaskStore(), + setupSigintHandler: false, + telemetry: { enabled: false }, + }); +} + +describe('ActorsMcpServer onerror handler', () => { + const servers: ActorsMcpServer[] = []; + + afterEach(async () => { + while (servers.length > 0) { + const server = servers.pop(); + await server?.close(); + } + vi.restoreAllMocks(); + }); + + const track = (server: ActorsMcpServer): ActorsMcpServer => { + servers.push(server); + return server; + }; + + it('downgrades "Controller is already closed" SSE-write errors to softFail', () => { + const server = track(makeServer()); + const softFail = vi.spyOn(log, 'softFail').mockImplementation(() => undefined); + const error = vi.spyOn(log, 'error').mockImplementation(() => undefined); + + const err = new TypeError('Invalid state: Controller is already closed'); + (err as unknown as { code?: string }).code = 'ERR_INVALID_STATE'; + server.server.onerror?.(err); + + expect(softFail).toHaveBeenCalledOnce(); + expect(error).not.toHaveBeenCalled(); + }); + + it.each([ + 'Not connected', + 'Failed to send response: Error: Not connected', + 'No connection established for request ID: 42', + 'Conflict: Only one SSE stream is allowed per session', + ])('downgrades known client-disconnect noise: %s', (message) => { + const server = track(makeServer()); + const softFail = vi.spyOn(log, 'softFail').mockImplementation(() => undefined); + const error = vi.spyOn(log, 'error').mockImplementation(() => undefined); + + server.server.onerror?.(new Error(message)); + + expect(softFail).toHaveBeenCalledOnce(); + expect(error).not.toHaveBeenCalled(); + }); + + it('still logs unexpected errors at error level', () => { + const server = track(makeServer()); + const softFail = vi.spyOn(log, 'softFail').mockImplementation(() => undefined); + const error = vi.spyOn(log, 'error').mockImplementation(() => undefined); + + server.server.onerror?.(new Error('Boom: something unexpected')); + + expect(error).toHaveBeenCalledOnce(); + expect(softFail).not.toHaveBeenCalled(); + }); +}); From 493556febf2fb1e52ce057db422e37a4d7f455d3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 5 May 2026 18:29:57 +0000 Subject: [PATCH 2/2] chore: Drop the SSE softFail unit test https://claude.ai/code/session_01LpjxNt1eBv8qBZcKLozmj1 --- tests/unit/mcp.server.error_handling.test.ts | 71 -------------------- 1 file changed, 71 deletions(-) delete mode 100644 tests/unit/mcp.server.error_handling.test.ts diff --git a/tests/unit/mcp.server.error_handling.test.ts b/tests/unit/mcp.server.error_handling.test.ts deleted file mode 100644 index 7c645f0f..00000000 --- a/tests/unit/mcp.server.error_handling.test.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { InMemoryTaskStore } from '@modelcontextprotocol/sdk/experimental/tasks/stores/in-memory.js'; -import { afterEach, describe, expect, it, vi } from 'vitest'; - -import log from '@apify/log'; - -import { ActorsMcpServer } from '../../src/mcp/server.js'; - -function makeServer(): ActorsMcpServer { - return new ActorsMcpServer({ - taskStore: new InMemoryTaskStore(), - setupSigintHandler: false, - telemetry: { enabled: false }, - }); -} - -describe('ActorsMcpServer onerror handler', () => { - const servers: ActorsMcpServer[] = []; - - afterEach(async () => { - while (servers.length > 0) { - const server = servers.pop(); - await server?.close(); - } - vi.restoreAllMocks(); - }); - - const track = (server: ActorsMcpServer): ActorsMcpServer => { - servers.push(server); - return server; - }; - - it('downgrades "Controller is already closed" SSE-write errors to softFail', () => { - const server = track(makeServer()); - const softFail = vi.spyOn(log, 'softFail').mockImplementation(() => undefined); - const error = vi.spyOn(log, 'error').mockImplementation(() => undefined); - - const err = new TypeError('Invalid state: Controller is already closed'); - (err as unknown as { code?: string }).code = 'ERR_INVALID_STATE'; - server.server.onerror?.(err); - - expect(softFail).toHaveBeenCalledOnce(); - expect(error).not.toHaveBeenCalled(); - }); - - it.each([ - 'Not connected', - 'Failed to send response: Error: Not connected', - 'No connection established for request ID: 42', - 'Conflict: Only one SSE stream is allowed per session', - ])('downgrades known client-disconnect noise: %s', (message) => { - const server = track(makeServer()); - const softFail = vi.spyOn(log, 'softFail').mockImplementation(() => undefined); - const error = vi.spyOn(log, 'error').mockImplementation(() => undefined); - - server.server.onerror?.(new Error(message)); - - expect(softFail).toHaveBeenCalledOnce(); - expect(error).not.toHaveBeenCalled(); - }); - - it('still logs unexpected errors at error level', () => { - const server = track(makeServer()); - const softFail = vi.spyOn(log, 'softFail').mockImplementation(() => undefined); - const error = vi.spyOn(log, 'error').mockImplementation(() => undefined); - - server.server.onerror?.(new Error('Boom: something unexpected')); - - expect(error).toHaveBeenCalledOnce(); - expect(softFail).not.toHaveBeenCalled(); - }); -});