From 42d9fcff7a527b010e6ddf80f928f65d4b951f1d Mon Sep 17 00:00:00 2001 From: baggiiiie Date: Thu, 3 Sep 2026 23:01:33 +0800 Subject: [PATCH 1/5] Surface unreachable upstreams as network error --- .changeset/openapi-transport-unreachable.md | 6 + .../openapi-unreachable-artifact.test.ts | 226 ++++++++++++++++++ packages/plugins/openapi/src/sdk/backing.ts | 11 +- .../openapi/src/sdk/upstream-failures.test.ts | 29 ++- 4 files changed, 269 insertions(+), 3 deletions(-) create mode 100644 .changeset/openapi-transport-unreachable.md create mode 100644 e2e/scenarios/openapi-unreachable-artifact.test.ts diff --git a/.changeset/openapi-transport-unreachable.md b/.changeset/openapi-transport-unreachable.md new file mode 100644 index 0000000000..8921b9b31f --- /dev/null +++ b/.changeset/openapi-transport-unreachable.md @@ -0,0 +1,6 @@ +--- +"executor": patch +"@executor-js/plugin-openapi": patch +--- + +OpenAPI tools that cannot reach the upstream server now return an `upstream_unreachable` error with an actionable network message instead of `Internal tool error [id]`. diff --git a/e2e/scenarios/openapi-unreachable-artifact.test.ts b/e2e/scenarios/openapi-unreachable-artifact.test.ts new file mode 100644 index 0000000000..179ca5048d --- /dev/null +++ b/e2e/scenarios/openapi-unreachable-artifact.test.ts @@ -0,0 +1,226 @@ +// Cross-target: an artifact whose OpenAPI query cannot reach its upstream gets +// an actionable network error, not the opaque defect mask. This walks the real +// path from a saved artifact through the nested shell, execute-action, sandbox, +// OpenAPI transport, and back into ArtifactError. +import { randomBytes } from "node:crypto"; +import { createServer } from "node:http"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import type { Page } from "playwright"; +import { composePluginApi } from "@executor-js/api/server"; +import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; +import { ConnectionName, IntegrationSlug, type ArtifactId } from "@executor-js/sdk/shared"; + +import { scenario } from "../src/scenario"; +import { Api, Browser, Mcp, Target } from "../src/services"; +import { visit } from "../src/surfaces/browser"; +import type { McpSession } from "../src/surfaces/mcp"; + +const api = composePluginApi([openApiHttpPlugin()] as const); + +const unique = (prefix: string) => `${prefix}_${randomBytes(4).toString("hex")}`; + +type DroppingUpstream = { + readonly url: string; + readonly requests: () => number; + readonly close: () => void; +}; + +// Accept the request, then drop the socket before sending response headers. +// This produces a real transport failure without relying on a hardcoded or +// temporarily-unused port. +const serveDroppingUpstream = () => + Effect.acquireRelease( + Effect.callback((resume) => { + let hits = 0; + const server = createServer((_request, response) => { + hits += 1; + response.destroy(); + }); + server.listen(0, "127.0.0.1", () => { + const address = server.address(); + const port = typeof address === "object" && address ? address.port : 0; + resume( + Effect.succeed({ + url: `http://127.0.0.1:${port}`, + requests: () => hits, + close: () => { + server.close(); + server.closeAllConnections(); + }, + }), + ); + }); + }), + (server) => Effect.sync(server.close), + ); + +const unreachableSpec = (baseUrl: string): string => + JSON.stringify({ + openapi: "3.0.3", + info: { title: "Unreachable API", version: "1.0.0" }, + servers: [{ url: baseUrl }], + paths: { + "/things": { + get: { + tags: ["things"], + operationId: "listThings", + summary: "List things", + responses: { + "200": { + description: "Things", + content: { + "application/json": { + schema: { type: "array", items: { type: "object" } }, + }, + }, + }, + }, + }, + }, + }, + }); + +const createConnectionCode = (slug: string) => ` +const created = await tools.executor.coreTools.connections.create({ + owner: "org", + name: "public", + integration: ${JSON.stringify(slug)}, + template: "none", +}); +return JSON.stringify(created.ok ? { ok: true } : { ok: false, error: created.error }); +`; + +const executeApproved = (session: McpSession, code: string) => + Effect.gen(function* () { + let result = yield* session.call("execute", { code }); + let guard = 0; + while (result.text.includes("executionId:") && guard < 10) { + result = yield* session.approvePaused(result.text); + guard += 1; + } + expect(result.ok, `execute completed (got: ${result.text.slice(0, 400)})`).toBe(true); + return result.text; + }); + +const artifactSource = (slug: string) => ` +function App() { + const query = useQuery(tools.${slug}.things.listThings.queryOptions({})); + const result = query.data; + return ( +
+

Upstream status

+
+ {query.isLoading ? ( + + ) : query.error ? ( + + ) : result?.ok === false ? ( + + ) : ( +

Unexpected upstream success

+ )} +
+
+ ); +} +`; + +const structuredOf = (result: { readonly raw: unknown }): Record => + ((result.raw as { structuredContent?: Record }).structuredContent ?? + {}) as Record; + +const artifactContent = (page: Page) => + page.frameLocator('[data-testid="artifact-shell-frame"]').frameLocator("iframe"); + +scenario( + "Artifacts · an unreachable OpenAPI host shows actionable retry guidance instead of an internal error", + { timeout: 180_000 }, + Effect.scoped( + Effect.gen(function* () { + const target = yield* Target; + const browser = yield* Browser; + const mcp = yield* Mcp; + const { client: makeClient } = yield* Api; + + const identity = yield* target.newIdentity(); + const client = yield* makeClient(api, identity); + const session = mcp.session(identity); + const upstream = yield* serveDroppingUpstream(); + const slug = unique("unreachable"); + const title = `Unreachable upstream ${randomBytes(4).toString("hex")}`; + let artifactId: ArtifactId | undefined; + + yield* Effect.ensuring( + Effect.gen(function* () { + yield* client.openapi.addSpec({ + payload: { + spec: { kind: "blob", value: unreachableSpec(upstream.url) }, + slug, + baseUrl: upstream.url, + }, + }); + + const created = yield* executeApproved(session, createConnectionCode(slug)); + expect(created, `the no-auth connection was created: ${created}`).toContain('"ok":true'); + + const rendered = yield* session.call("create-artifact", { + code: artifactSource(slug), + title, + description: "Shows whether the upstream API is reachable", + connections: { [slug]: `${slug}.org.public` }, + }); + expect(rendered.ok, `create-artifact succeeded: ${rendered.text}`).toBe(true); + + const structured = structuredOf(rendered); + artifactId = structured.artifactId as ArtifactId; + expect(artifactId, "the artifact was persisted").toBeTruthy(); + + yield* browser.session(identity, async ({ page, step }) => { + await step("Open the artifact that reads from the unreachable API", async () => { + await visit(page, String(structured.url)); + await page.getByRole("heading", { name: title }).waitFor({ timeout: 20_000 }); + }); + + await step( + "The artifact explains that the upstream host could not be reached", + async () => { + const state = artifactContent(page).getByTestId("upstream-state"); + await state.locator('[data-slot="artifact-error"]').waitFor({ timeout: 30_000 }); + const message = await state.innerText(); + + expect(message, "the user gets actionable network guidance").toContain( + "Could not reach the upstream server", + ); + expect(message, "the opaque defect mask never reaches the artifact").not.toContain( + "Internal tool error", + ); + expect(message, "the request path is not leaked").not.toContain("/things"); + }, + ); + }); + + expect(upstream.requests(), "the artifact made a real upstream request").toBeGreaterThan( + 0, + ); + }), + Effect.gen(function* () { + if (artifactId !== undefined) { + yield* client.artifacts.remove({ params: { artifactId } }).pipe(Effect.ignore); + } + yield* client.connections + .remove({ + params: { + owner: "org", + integration: IntegrationSlug.make(slug), + name: ConnectionName.make("public"), + }, + }) + .pipe(Effect.ignore); + yield* client.openapi.removeSpec({ params: { slug } }).pipe(Effect.ignore); + }), + ); + }), + ), +); diff --git a/packages/plugins/openapi/src/sdk/backing.ts b/packages/plugins/openapi/src/sdk/backing.ts index b7a54d4f43..251822cf68 100644 --- a/packages/plugins/openapi/src/sdk/backing.ts +++ b/packages/plugins/openapi/src/sdk/backing.ts @@ -727,7 +727,16 @@ export const invokeOpenApiBackedTool = (input: { details: error.cause ?? error, }), }) - : Effect.fail(error), + : error.cause !== undefined && Option.isNone(error.statusCode) + ? Effect.succeed({ + ok: false as const, + failure: ToolResult.fail({ + code: "upstream_unreachable", + message: + "Could not reach the upstream server. Check your network and try again.", + }), + }) + : Effect.fail(error), ), ); diff --git a/packages/plugins/openapi/src/sdk/upstream-failures.test.ts b/packages/plugins/openapi/src/sdk/upstream-failures.test.ts index 2089e2fdf0..f07144d4c5 100644 --- a/packages/plugins/openapi/src/sdk/upstream-failures.test.ts +++ b/packages/plugins/openapi/src/sdk/upstream-failures.test.ts @@ -394,9 +394,12 @@ describe("OpenAPI upstream failure modes", () => { const { baseUrl } = yield* startDroppingServer(); const { executor, address } = yield* buildExecutor(baseUrl); - const exit = yield* executor.execute(address, {}).pipe(Effect.exit); + const result = yield* executor.execute(address, {}); - expect(Exit.isFailure(exit)).toBe(true); + expect(result).toMatchObject({ + ok: false, + error: { code: "upstream_unreachable" }, + }); }), ); @@ -446,4 +449,26 @@ describe("OpenAPI upstream failure modes", () => { expect(result.data).toEqual([]); }), ); + + // Port 1 refuses immediately. The same path used to throw `Internal tool + // error [hex]` because the raw HttpClientError carries the request URL. + it.effect("connection refused returns upstream_unreachable without leaking the path", () => + Effect.gen(function* () { + const { executor, address } = yield* buildExecutor("http://127.0.0.1:1"); + + const result = yield* executor.execute(address, {}); + + expect(result).toMatchObject({ + ok: false, + error: { code: "upstream_unreachable" }, + }); + const failure = result as { + readonly ok: false; + readonly error: { readonly message: string }; + }; + expect(failure.error.message).toContain("Could not reach"); + expect(failure.error.message).not.toContain("Internal tool error"); + expect(failure.error.message).not.toContain("/things"); + }), + ); }); From e2d2319ae1a57dfba2a89d5d1275387605b83ae3 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Sat, 12 Sep 2026 10:12:06 -0700 Subject: [PATCH 2/5] Test queue timeout with a controlled clock --- apps/cloud/src/mcp/session-build-semaphore.test.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/apps/cloud/src/mcp/session-build-semaphore.test.ts b/apps/cloud/src/mcp/session-build-semaphore.test.ts index 3d4ad76343..584b65ee0e 100644 --- a/apps/cloud/src/mcp/session-build-semaphore.test.ts +++ b/apps/cloud/src/mcp/session-build-semaphore.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, beforeEach } from "@effect/vitest"; +import { describe, expect, it, beforeEach, afterEach, vi } from "@effect/vitest"; import { acquireBuildSlot, @@ -13,6 +13,10 @@ describe("session-build-semaphore", () => { resetBuildSlotsForTest(); }); + afterEach(() => { + vi.useRealTimers(); + }); + it("grants up to the cap immediately, with no wait", async () => { const results = await Promise.all([ acquireBuildSlot().promise, @@ -214,6 +218,7 @@ describe("session-build-semaphore", () => { }); it("proceeds without a slot when the queue wait exceeds the timeout, and does not count it as active", async () => { + vi.useFakeTimers(); await Promise.all([ acquireBuildSlot().promise, acquireBuildSlot().promise, @@ -223,6 +228,10 @@ describe("session-build-semaphore", () => { expect(currentActiveBuildsForTest()).toBe(4); const timedOutHandle = acquireBuildSlot(10); + await vi.advanceTimersByTimeAsync(9); + expect(currentQueueLengthForTest()).toBe(1); + expect(currentActiveBuildsForTest()).toBe(4); + await vi.advanceTimersByTimeAsync(1); const result = await timedOutHandle.promise; expect(result).toEqual({ acquired: false, waitMs: expect.any(Number), timedOut: true }); From 616fa135b3f04c061641285e4f95368357ad973a Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Sat, 12 Sep 2026 11:39:37 -0700 Subject: [PATCH 3/5] Classify transport failures without hiding invocation defects --- packages/plugins/openapi/src/sdk/backing.ts | 2 +- packages/plugins/openapi/src/sdk/errors.ts | 6 +- packages/plugins/openapi/src/sdk/invoke.ts | 9 ++- .../openapi/src/sdk/upstream-failures.test.ts | 78 +++++++++++++++++-- 4 files changed, 84 insertions(+), 11 deletions(-) diff --git a/packages/plugins/openapi/src/sdk/backing.ts b/packages/plugins/openapi/src/sdk/backing.ts index 251822cf68..767753d302 100644 --- a/packages/plugins/openapi/src/sdk/backing.ts +++ b/packages/plugins/openapi/src/sdk/backing.ts @@ -727,7 +727,7 @@ export const invokeOpenApiBackedTool = (input: { details: error.cause ?? error, }), }) - : error.cause !== undefined && Option.isNone(error.statusCode) + : error.reason === "transport_error" ? Effect.succeed({ ok: false as const, failure: ToolResult.fail({ diff --git a/packages/plugins/openapi/src/sdk/errors.ts b/packages/plugins/openapi/src/sdk/errors.ts index 6a5fc4a8cc..b2042c5a4a 100644 --- a/packages/plugins/openapi/src/sdk/errors.ts +++ b/packages/plugins/openapi/src/sdk/errors.ts @@ -39,7 +39,11 @@ export class OpenApiSpecOverrideError extends Schema.TaggedErrorClass; - readonly reason?: "response_headers_timeout" | "response_body_timeout" | "unknown_arguments"; + readonly reason?: + | "response_headers_timeout" + | "response_body_timeout" + | "unknown_arguments" + | "transport_error"; readonly cause?: unknown; }> {} diff --git a/packages/plugins/openapi/src/sdk/invoke.ts b/packages/plugins/openapi/src/sdk/invoke.ts index 2f9bd0f6f5..4cf45a739d 100644 --- a/packages/plugins/openapi/src/sdk/invoke.ts +++ b/packages/plugins/openapi/src/sdk/invoke.ts @@ -1,4 +1,4 @@ -import { Effect, Exit, Fiber, Layer, Option, Schema, Stream } from "effect"; +import { Effect, Exit, Fiber, Layer, Option, Predicate, Schema, Stream } from "effect"; import { HttpClient, HttpClientRequest, HttpClientResponse } from "effect/unstable/http"; import { isToolFile, type ToolFileValue } from "@executor-js/sdk/core"; @@ -1183,6 +1183,9 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* ( (err) => new OpenApiInvocationError({ message: "HTTP request failed", + ...(Predicate.isTagged(err.reason, "TransportError") + ? { reason: "transport_error" as const } + : {}), statusCode: Option.none(), cause: err, }), @@ -1197,7 +1200,6 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* ( }), ), ); - const fiber = runFork(responseEffect); const interrupt = () => { runFork(Fiber.interrupt(fiber)); }; @@ -1207,6 +1209,7 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* ( interrupt(); resume(Effect.succeed(Option.none())); }, responseHeadersTimeoutMs); + const fiber = runFork(responseEffect); signal.addEventListener("abort", interrupt, { once: true }); return Effect.sync(() => { clearTimeout(timer); @@ -1257,7 +1260,6 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* ( }), ), ); - const fiber = runFork(bodyEffect); const interrupt = () => { runFork(Fiber.interrupt(fiber)); }; @@ -1267,6 +1269,7 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* ( interrupt(); resume(Effect.succeed(Option.none())); }, responseBodyTimeoutMs); + const fiber = runFork(bodyEffect); signal.addEventListener("abort", interrupt, { once: true }); return Effect.sync(() => { clearTimeout(timer); diff --git a/packages/plugins/openapi/src/sdk/upstream-failures.test.ts b/packages/plugins/openapi/src/sdk/upstream-failures.test.ts index f07144d4c5..e167694220 100644 --- a/packages/plugins/openapi/src/sdk/upstream-failures.test.ts +++ b/packages/plugins/openapi/src/sdk/upstream-failures.test.ts @@ -11,8 +11,14 @@ // --------------------------------------------------------------------------- import { describe, expect, it } from "@effect/vitest"; -import { Effect, Exit, Schema } from "effect"; -import { FetchHttpClient, HttpServerRequest, HttpServerResponse } from "effect/unstable/http"; +import { Cause, Data, Effect, Exit, Layer, Schema } from "effect"; +import { + FetchHttpClient, + HttpClient, + HttpClientError, + HttpServerRequest, + HttpServerResponse, +} from "effect/unstable/http"; import { HttpApi, HttpApiBuilder, @@ -43,8 +49,10 @@ import { import { openApiPlugin } from "./plugin"; -const testPlugins = () => - [openApiPlugin({ httpClientLayer: FetchHttpClient.layer }), memoryCredentialsPlugin()] as const; +class AdapterDefect extends Data.TaggedError("AdapterDefect") {} + +const testPlugins = (httpClientLayer = FetchHttpClient.layer) => + [openApiPlugin({ httpClientLayer }), memoryCredentialsPlugin()] as const; // `/things` GET op `listThings` under group "things" → tool path // `things.listThings`, used verbatim (dots and all) as the address tool segment. @@ -114,9 +122,11 @@ const FailureApi = HttpApi.make("failuresTest") // Build an executor + connection from the FailureApi HttpApi against an // arbitrary baseUrl (used for the Node-transport socket-drop / slow cases). -const buildExecutor = (baseUrl: string) => +const buildExecutor = (baseUrl: string, httpClientLayer = FetchHttpClient.layer) => Effect.gen(function* () { - const executor = yield* createExecutor(makeTestConfig({ plugins: testPlugins() })); + const executor = yield* createExecutor( + makeTestConfig({ plugins: testPlugins(httpClientLayer) }), + ); yield* executor.openapi.addSpec( makeOpenApiHttpApiTestIntegrationConfig(FailureApi, { slug: "f", baseUrl }), ); @@ -450,6 +460,62 @@ describe("OpenAPI upstream failure modes", () => { }), ); + it.effect("request encoding failures remain invocation failures", () => + Effect.gen(function* () { + const httpClientLayer = Layer.succeed( + HttpClient.HttpClient, + HttpClient.make((request) => + Effect.fail( + new HttpClientError.HttpClientError({ + reason: new HttpClientError.EncodeError({ request, cause: new AdapterDefect() }), + }), + ), + ), + ); + const { executor, address } = yield* buildExecutor( + "https://upstream.example", + httpClientLayer, + ); + const exit = yield* executor.execute(address, {}).pipe(Effect.exit); + expect(Exit.isFailure(exit)).toBe(true); + }), + ); + + it.effect("transport defects remain defects", () => + Effect.gen(function* () { + const defect = new AdapterDefect(); + const httpClientLayer = Layer.succeed( + HttpClient.HttpClient, + HttpClient.make(() => Effect.die(defect)), + ); + const { executor, address } = yield* buildExecutor( + "https://upstream.example", + httpClientLayer, + ); + const exit = yield* executor.execute(address, {}).pipe(Effect.exit); + expect(Exit.isFailure(exit)).toBe(true); + expect(Exit.match(exit, { onFailure: Cause.hasDies, onSuccess: () => false })).toBe(true); + }), + ); + + it.effect("interrupted transport remains interrupted", () => + Effect.gen(function* () { + const httpClientLayer = Layer.succeed( + HttpClient.HttpClient, + HttpClient.make(() => Effect.interrupt), + ); + const { executor, address } = yield* buildExecutor( + "https://upstream.example", + httpClientLayer, + ); + const exit = yield* executor.execute(address, {}).pipe(Effect.exit); + expect(Exit.isFailure(exit)).toBe(true); + expect(Exit.match(exit, { onFailure: Cause.hasInterrupts, onSuccess: () => false })).toBe( + true, + ); + }), + ); + // Port 1 refuses immediately. The same path used to throw `Internal tool // error [hex]` because the raw HttpClientError carries the request URL. it.effect("connection refused returns upstream_unreachable without leaking the path", () => From aaba4aa719684f175264ff7a425a23de6f7d9eab Mon Sep 17 00:00:00 2001 From: baggiiiie Date: Mon, 14 Sep 2026 23:34:27 +0800 Subject: [PATCH 4/5] Name the integration and origin in the unreachable-upstream message Executor sends the request, not the user's browser, so "check your network" pointed at the wrong place. Name the integration and the origin that could not be reached and tell the user to verify the base URL and that the service is online. Only the host is lifted off the transport failure; the path, query, and headers stay out of the message. --- .../openapi-unreachable-artifact.test.ts | 2 +- packages/plugins/openapi/src/sdk/backing.ts | 6 ++-- packages/plugins/openapi/src/sdk/errors.ts | 4 +++ packages/plugins/openapi/src/sdk/invoke.ts | 29 ++++++++++++++++--- .../openapi/src/sdk/upstream-failures.test.ts | 14 +++++++-- 5 files changed, 45 insertions(+), 10 deletions(-) diff --git a/e2e/scenarios/openapi-unreachable-artifact.test.ts b/e2e/scenarios/openapi-unreachable-artifact.test.ts index 179ca5048d..1b5adae46e 100644 --- a/e2e/scenarios/openapi-unreachable-artifact.test.ts +++ b/e2e/scenarios/openapi-unreachable-artifact.test.ts @@ -191,7 +191,7 @@ scenario( const message = await state.innerText(); expect(message, "the user gets actionable network guidance").toContain( - "Could not reach the upstream server", + `Could not reach the upstream server for "${slug}"`, ); expect(message, "the opaque defect mask never reaches the artifact").not.toContain( "Internal tool error", diff --git a/packages/plugins/openapi/src/sdk/backing.ts b/packages/plugins/openapi/src/sdk/backing.ts index 767753d302..3be4064c8e 100644 --- a/packages/plugins/openapi/src/sdk/backing.ts +++ b/packages/plugins/openapi/src/sdk/backing.ts @@ -732,8 +732,10 @@ export const invokeOpenApiBackedTool = (input: { ok: false as const, failure: ToolResult.fail({ code: "upstream_unreachable", - message: - "Could not reach the upstream server. Check your network and try again.", + // Executor sends the request, not the user's browser, so + // point at what the user can act on: the configured + // origin and the service behind it. + message: `Could not reach the upstream server for "${integration}"${error.upstreamHost ? ` at ${error.upstreamHost}` : ""}. Verify the integration's base URL and that the service is online, then try again.`, }), }) : Effect.fail(error), diff --git a/packages/plugins/openapi/src/sdk/errors.ts b/packages/plugins/openapi/src/sdk/errors.ts index b2042c5a4a..deeabed4ec 100644 --- a/packages/plugins/openapi/src/sdk/errors.ts +++ b/packages/plugins/openapi/src/sdk/errors.ts @@ -44,6 +44,10 @@ export class OpenApiInvocationError extends Data.TaggedError("OpenApiInvocationE | "response_body_timeout" | "unknown_arguments" | "transport_error"; + // `host[:port]` of a request that failed at the transport layer. It is the + // integration's configured origin, so it is safe to show; the path, query, + // and headers stay on `cause`. + readonly upstreamHost?: string | undefined; readonly cause?: unknown; }> {} diff --git a/packages/plugins/openapi/src/sdk/invoke.ts b/packages/plugins/openapi/src/sdk/invoke.ts index 4cf45a739d..6d2713812f 100644 --- a/packages/plugins/openapi/src/sdk/invoke.ts +++ b/packages/plugins/openapi/src/sdk/invoke.ts @@ -1,5 +1,10 @@ import { Effect, Exit, Fiber, Layer, Option, Predicate, Schema, Stream } from "effect"; -import { HttpClient, HttpClientRequest, HttpClientResponse } from "effect/unstable/http"; +import { + HttpClient, + type HttpClientError, + HttpClientRequest, + HttpClientResponse, +} from "effect/unstable/http"; import { isToolFile, type ToolFileValue } from "@executor-js/sdk/core"; import { OpenApiInvocationError } from "./errors"; @@ -1148,6 +1153,24 @@ export const buildRequest = Effect.fn("OpenApi.buildRequest")(function* ( return request; }); +// --------------------------------------------------------------------------- +// Transport failure classification +// --------------------------------------------------------------------------- + +const urlHost = Option.liftThrowable((url: string) => new URL(url).host); + +// A transport failure produced no response: DNS, connection refused, TLS, or a +// socket dropped before headers. The TransportError carries the whole request +// (URL, headers, credentials), so only the origin is lifted onto the +// invocation error for user-facing copy. +const transportFailureFields = (reason: HttpClientError.HttpClientError["reason"]) => + Predicate.isTagged(reason, "TransportError") + ? { + reason: "transport_error" as const, + upstreamHost: Option.getOrUndefined(urlHost(reason.request.url)), + } + : {}; + // --------------------------------------------------------------------------- // Public API — invoke a single operation // --------------------------------------------------------------------------- @@ -1183,9 +1206,7 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* ( (err) => new OpenApiInvocationError({ message: "HTTP request failed", - ...(Predicate.isTagged(err.reason, "TransportError") - ? { reason: "transport_error" as const } - : {}), + ...transportFailureFields(err.reason), statusCode: Option.none(), cause: err, }), diff --git a/packages/plugins/openapi/src/sdk/upstream-failures.test.ts b/packages/plugins/openapi/src/sdk/upstream-failures.test.ts index e167694220..5287c2b6a2 100644 --- a/packages/plugins/openapi/src/sdk/upstream-failures.test.ts +++ b/packages/plugins/openapi/src/sdk/upstream-failures.test.ts @@ -518,7 +518,9 @@ describe("OpenAPI upstream failure modes", () => { // Port 1 refuses immediately. The same path used to throw `Internal tool // error [hex]` because the raw HttpClientError carries the request URL. - it.effect("connection refused returns upstream_unreachable without leaking the path", () => + // Executor makes the request, so the message names the integration and + // origin the user can fix instead of blaming their own network. + it.effect("connection refused names the integration and origin without the path", () => Effect.gen(function* () { const { executor, address } = yield* buildExecutor("http://127.0.0.1:1"); @@ -526,13 +528,19 @@ describe("OpenAPI upstream failure modes", () => { expect(result).toMatchObject({ ok: false, - error: { code: "upstream_unreachable" }, + error: { + code: "upstream_unreachable", + message: expect.stringContaining( + 'Could not reach the upstream server for "f" at 127.0.0.1:1.', + ), + }, }); const failure = result as { readonly ok: false; readonly error: { readonly message: string }; }; - expect(failure.error.message).toContain("Could not reach"); + expect(failure.error.message).toContain("base URL"); + expect(failure.error.message).not.toContain("your network"); expect(failure.error.message).not.toContain("Internal tool error"); expect(failure.error.message).not.toContain("/things"); }), From d7cf1916b3557e2ea40e0c2e2c743d2e3d2d0540 Mon Sep 17 00:00:00 2001 From: baggiiiie Date: Mon, 14 Sep 2026 23:36:24 +0800 Subject: [PATCH 5/5] Record the cause of unreachable upstreams without the request Classifying transport failures as a typed tool result took them off the hosts' defect path, which was the only place the cause was logged. Lift the errno-style code (ECONNREFUSED, ENOTFOUND, UND_ERR_SOCKET, ...) off the fetch cause chain, log a warning and annotate the span with the integration, host, and code, and return the same sanitized pair in the tool result details. The raw TransportError stays out of details on purpose: it carries the whole request, including resolved auth headers. --- .changeset/openapi-transport-unreachable.md | 2 +- packages/plugins/openapi/src/sdk/backing.ts | 44 ++++++++++--- packages/plugins/openapi/src/sdk/errors.ts | 4 ++ packages/plugins/openapi/src/sdk/invoke.ts | 24 ++++++- .../openapi/src/sdk/upstream-failures.test.ts | 66 ++++++++++++++++++- 5 files changed, 125 insertions(+), 15 deletions(-) diff --git a/.changeset/openapi-transport-unreachable.md b/.changeset/openapi-transport-unreachable.md index 8921b9b31f..05623ff515 100644 --- a/.changeset/openapi-transport-unreachable.md +++ b/.changeset/openapi-transport-unreachable.md @@ -3,4 +3,4 @@ "@executor-js/plugin-openapi": patch --- -OpenAPI tools that cannot reach the upstream server now return an `upstream_unreachable` error with an actionable network message instead of `Internal tool error [id]`. +OpenAPI tools that cannot reach the upstream server now return an `upstream_unreachable` error instead of `Internal tool error [id]`. The message names the integration and origin that could not be reached, `details` carries the sanitized `host` and errno-style `code` (`ECONNREFUSED`, `ENOTFOUND`, …), and the failure is logged with the same classification. diff --git a/packages/plugins/openapi/src/sdk/backing.ts b/packages/plugins/openapi/src/sdk/backing.ts index 3be4064c8e..58427c5f54 100644 --- a/packages/plugins/openapi/src/sdk/backing.ts +++ b/packages/plugins/openapi/src/sdk/backing.ts @@ -629,6 +629,22 @@ export const resolveOpenApiBackedTools = ({ }; }); +// Transport failures used to escape as defects, which the hosts log with a +// correlation id. As a typed tool failure nothing else records them, so log +// and annotate the span with the sanitized classification operators need to +// tell DNS from refused from TLS. +const recordUpstreamUnreachable = (integration: string, error: OpenApiInvocationError) => { + const annotations = { + "plugin.openapi.integration": integration, + "plugin.openapi.upstream.host": error.upstreamHost ?? "unknown", + "plugin.openapi.upstream.transport_code": error.transportCode ?? "unknown", + }; + return Effect.logWarning("OpenAPI upstream unreachable").pipe( + Effect.annotateLogs(annotations), + Effect.andThen(Effect.annotateCurrentSpan(annotations)), + ); +}; + export const invokeOpenApiBackedTool = (input: { readonly ctx: PluginCtx; readonly toolRow: { readonly integration: string; readonly name: string }; @@ -728,16 +744,26 @@ export const invokeOpenApiBackedTool = (input: { }), }) : error.reason === "transport_error" - ? Effect.succeed({ - ok: false as const, - failure: ToolResult.fail({ - code: "upstream_unreachable", - // Executor sends the request, not the user's browser, so - // point at what the user can act on: the configured - // origin and the service behind it. - message: `Could not reach the upstream server for "${integration}"${error.upstreamHost ? ` at ${error.upstreamHost}` : ""}. Verify the integration's base URL and that the service is online, then try again.`, + ? recordUpstreamUnreachable(integration, error).pipe( + Effect.as({ + ok: false as const, + failure: ToolResult.fail({ + code: "upstream_unreachable", + // Executor sends the request, not the user's browser, so + // point at what the user can act on: the configured + // origin and the service behind it. + message: `Could not reach the upstream server for "${integration}"${error.upstreamHost ? ` at ${error.upstreamHost}` : ""}. Verify the integration's base URL and that the service is online, then try again.`, + // Unlike the timeout branches, `error.cause` is withheld: + // the TransportError carries the whole request, including + // resolved auth headers. Absent fields are dropped, not + // `undefined`: the result must stay a JSON value. + details: { + ...(error.upstreamHost !== undefined ? { host: error.upstreamHost } : {}), + ...(error.transportCode !== undefined ? { code: error.transportCode } : {}), + }, + }), }), - }) + ) : Effect.fail(error), ), ); diff --git a/packages/plugins/openapi/src/sdk/errors.ts b/packages/plugins/openapi/src/sdk/errors.ts index deeabed4ec..a13a2aa29e 100644 --- a/packages/plugins/openapi/src/sdk/errors.ts +++ b/packages/plugins/openapi/src/sdk/errors.ts @@ -48,6 +48,10 @@ export class OpenApiInvocationError extends Data.TaggedError("OpenApiInvocationE // integration's configured origin, so it is safe to show; the path, query, // and headers stay on `cause`. readonly upstreamHost?: string | undefined; + // Errno-style code behind a transport failure (`ECONNREFUSED`, `ENOTFOUND`, + // `UND_ERR_SOCKET`, …) when the runtime exposes one. Tells DNS from refused + // from TLS without exposing the request. + readonly transportCode?: string | undefined; readonly cause?: unknown; }> {} diff --git a/packages/plugins/openapi/src/sdk/invoke.ts b/packages/plugins/openapi/src/sdk/invoke.ts index 6d2713812f..8d180e6952 100644 --- a/packages/plugins/openapi/src/sdk/invoke.ts +++ b/packages/plugins/openapi/src/sdk/invoke.ts @@ -1159,15 +1159,35 @@ export const buildRequest = Effect.fn("OpenApi.buildRequest")(function* ( const urlHost = Option.liftThrowable((url: string) => new URL(url).host); +// `fetch` rejects with a generic `TypeError("fetch failed")`; the errno-style +// code (`ECONNREFUSED`, `ENOTFOUND`, `UND_ERR_SOCKET`, …) sits on the innermost +// link of its `cause` chain. The walk is bounded so a cyclic cause cannot spin. +const TransportCauseLink = Schema.Struct({ + code: Schema.optional(Schema.String), + cause: Schema.optional(Schema.Unknown), +}); +const decodeTransportCauseLink = Schema.decodeUnknownOption(TransportCauseLink); +const TRANSPORT_CAUSE_MAX_DEPTH = 5; + +const transportFailureCode = (cause: unknown, depth = 0): string | undefined => + Option.match(decodeTransportCauseLink(cause), { + onNone: () => undefined, + onSome: (link) => + (link.cause !== undefined && depth < TRANSPORT_CAUSE_MAX_DEPTH + ? transportFailureCode(link.cause, depth + 1) + : undefined) ?? link.code, + }); + // A transport failure produced no response: DNS, connection refused, TLS, or a // socket dropped before headers. The TransportError carries the whole request -// (URL, headers, credentials), so only the origin is lifted onto the -// invocation error for user-facing copy. +// (URL, headers, credentials), so only the origin and the errno-style code are +// lifted onto the invocation error. const transportFailureFields = (reason: HttpClientError.HttpClientError["reason"]) => Predicate.isTagged(reason, "TransportError") ? { reason: "transport_error" as const, upstreamHost: Option.getOrUndefined(urlHost(reason.request.url)), + transportCode: transportFailureCode(reason.cause), } : {}; diff --git a/packages/plugins/openapi/src/sdk/upstream-failures.test.ts b/packages/plugins/openapi/src/sdk/upstream-failures.test.ts index 5287c2b6a2..1f52b30c50 100644 --- a/packages/plugins/openapi/src/sdk/upstream-failures.test.ts +++ b/packages/plugins/openapi/src/sdk/upstream-failures.test.ts @@ -11,7 +11,7 @@ // --------------------------------------------------------------------------- import { describe, expect, it } from "@effect/vitest"; -import { Cause, Data, Effect, Exit, Layer, Schema } from "effect"; +import { Cause, Data, Effect, Exit, Layer, Logger, References, Schema } from "effect"; import { FetchHttpClient, HttpClient, @@ -110,6 +110,30 @@ const startDroppingServer = () => (s) => Effect.sync(() => s.close()), ); +// Bind an ephemeral port, then release it so nothing listens there and the +// kernel refuses the connection (`ECONNREFUSED`). Port 1 is not equivalent: +// `fetch` rejects it as a bad port before dialing, with no errno. +const refusedBaseUrl = () => + Effect.callback((resume) => { + const server = createServer(); + server.listen(0, "127.0.0.1", () => { + const port = (server.address() as AddressInfo).port; + server.close(() => resume(Effect.succeed(`http://127.0.0.1:${port}`))); + }); + }); + +type CapturedLog = { readonly message: string; readonly annotations: Record }; + +const capturingLogger = (sink: Array) => + Logger.layer([ + Logger.make((options) => { + sink.push({ + message: String(options.message), + annotations: options.fiber.getRef(References.CurrentLogAnnotations), + }); + }), + ]); + const ThingsGroup = HttpApiGroup.make("things").add( HttpApiEndpoint.get("listThings", "/things", { success: Schema.Array(Schema.Record(Schema.String, Schema.Unknown)), @@ -408,7 +432,7 @@ describe("OpenAPI upstream failure modes", () => { expect(result).toMatchObject({ ok: false, - error: { code: "upstream_unreachable" }, + error: { code: "upstream_unreachable", details: { code: expect.any(String) } }, }); }), ); @@ -537,12 +561,48 @@ describe("OpenAPI upstream failure modes", () => { }); const failure = result as { readonly ok: false; - readonly error: { readonly message: string }; + readonly error: { readonly message: string; readonly details?: unknown }; }; + // No errno here (`fetch` rejects port 1 before dialing). The result + // crosses a JSON boundary, so the missing code must be absent rather + // than an `undefined` property. + expect(failure.error.details).toStrictEqual({ host: "127.0.0.1:1" }); expect(failure.error.message).toContain("base URL"); expect(failure.error.message).not.toContain("your network"); expect(failure.error.message).not.toContain("Internal tool error"); expect(failure.error.message).not.toContain("/things"); }), ); + + // Classifying the failure took it off the hosts' correlation-id defect log, + // so the sanitized cause must reach both the caller (details) and operators + // (log) — and never the request, which carries the resolved auth header. + it.effect("connection refused reports the errno code to the caller and the log", () => + Effect.gen(function* () { + const baseUrl = yield* refusedBaseUrl(); + const { executor, address } = yield* buildExecutor(baseUrl); + const logged: Array = []; + + const result = yield* executor + .execute(address, {}) + .pipe(Effect.provide(capturingLogger(logged))); + + const host = new URL(baseUrl).host; + expect(result).toMatchObject({ + ok: false, + error: { code: "upstream_unreachable", details: { host, code: "ECONNREFUSED" } }, + }); + const rendered = JSON.stringify(result); + expect(rendered).not.toContain("/things"); + // The apiKey value `buildExecutor` puts on the connection. + expect(rendered).not.toContain("token"); + + const warning = logged.find((entry) => entry.message.includes("upstream unreachable")); + expect(warning?.annotations).toMatchObject({ + "plugin.openapi.integration": "f", + "plugin.openapi.upstream.host": host, + "plugin.openapi.upstream.transport_code": "ECONNREFUSED", + }); + }), + ); });