Skip to content

Commit f7c3ada

Browse files
committed
Classify transport failures without hiding invocation defects
1 parent 49bd178 commit f7c3ada

4 files changed

Lines changed: 84 additions & 11 deletions

File tree

‎packages/plugins/openapi/src/sdk/backing.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -727,7 +727,7 @@ export const invokeOpenApiBackedTool = (input: {
727727
details: error.cause ?? error,
728728
}),
729729
})
730-
: error.cause !== undefined && Option.isNone(error.statusCode)
730+
: error.reason === "transport_error"
731731
? Effect.succeed({
732732
ok: false as const,
733733
failure: ToolResult.fail({

‎packages/plugins/openapi/src/sdk/errors.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,11 @@ export class OpenApiSpecOverrideError extends Schema.TaggedErrorClass<OpenApiSpe
3939
export class OpenApiInvocationError extends Data.TaggedError("OpenApiInvocationError")<{
4040
readonly message: string;
4141
readonly statusCode: Option.Option<number>;
42-
readonly reason?: "response_headers_timeout" | "response_body_timeout" | "unknown_arguments";
42+
readonly reason?:
43+
| "response_headers_timeout"
44+
| "response_body_timeout"
45+
| "unknown_arguments"
46+
| "transport_error";
4347
readonly cause?: unknown;
4448
}> {}
4549

‎packages/plugins/openapi/src/sdk/invoke.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Effect, Exit, Fiber, Layer, Option, Schema, Stream } from "effect";
1+
import { Effect, Exit, Fiber, Layer, Option, Predicate, Schema, Stream } from "effect";
22
import { HttpClient, HttpClientRequest, HttpClientResponse } from "effect/unstable/http";
33
import { isToolFile, type ToolFileValue } from "@executor-js/sdk/core";
44

@@ -1183,6 +1183,9 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* (
11831183
(err) =>
11841184
new OpenApiInvocationError({
11851185
message: "HTTP request failed",
1186+
...(Predicate.isTagged(err.reason, "TransportError")
1187+
? { reason: "transport_error" as const }
1188+
: {}),
11861189
statusCode: Option.none(),
11871190
cause: err,
11881191
}),
@@ -1197,7 +1200,6 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* (
11971200
}),
11981201
),
11991202
);
1200-
const fiber = runFork(responseEffect);
12011203
const interrupt = () => {
12021204
runFork(Fiber.interrupt(fiber));
12031205
};
@@ -1207,6 +1209,7 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* (
12071209
interrupt();
12081210
resume(Effect.succeed(Option.none()));
12091211
}, responseHeadersTimeoutMs);
1212+
const fiber = runFork(responseEffect);
12101213
signal.addEventListener("abort", interrupt, { once: true });
12111214
return Effect.sync(() => {
12121215
clearTimeout(timer);
@@ -1257,7 +1260,6 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* (
12571260
}),
12581261
),
12591262
);
1260-
const fiber = runFork(bodyEffect);
12611263
const interrupt = () => {
12621264
runFork(Fiber.interrupt(fiber));
12631265
};
@@ -1267,6 +1269,7 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* (
12671269
interrupt();
12681270
resume(Effect.succeed(Option.none()));
12691271
}, responseBodyTimeoutMs);
1272+
const fiber = runFork(bodyEffect);
12701273
signal.addEventListener("abort", interrupt, { once: true });
12711274
return Effect.sync(() => {
12721275
clearTimeout(timer);

‎packages/plugins/openapi/src/sdk/upstream-failures.test.ts‎

Lines changed: 72 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,14 @@
1111
// ---------------------------------------------------------------------------
1212

1313
import { describe, expect, it } from "@effect/vitest";
14-
import { Effect, Exit, Schema } from "effect";
15-
import { FetchHttpClient, HttpServerRequest, HttpServerResponse } from "effect/unstable/http";
14+
import { Cause, Data, Effect, Exit, Layer, Schema } from "effect";
15+
import {
16+
FetchHttpClient,
17+
HttpClient,
18+
HttpClientError,
19+
HttpServerRequest,
20+
HttpServerResponse,
21+
} from "effect/unstable/http";
1622
import {
1723
HttpApi,
1824
HttpApiBuilder,
@@ -43,8 +49,10 @@ import {
4349

4450
import { openApiPlugin } from "./plugin";
4551

46-
const testPlugins = () =>
47-
[openApiPlugin({ httpClientLayer: FetchHttpClient.layer }), memoryCredentialsPlugin()] as const;
52+
class AdapterDefect extends Data.TaggedError("AdapterDefect") {}
53+
54+
const testPlugins = (httpClientLayer = FetchHttpClient.layer) =>
55+
[openApiPlugin({ httpClientLayer }), memoryCredentialsPlugin()] as const;
4856

4957
// `/things` GET op `listThings` under group "things" → tool path
5058
// `things.listThings`, used verbatim (dots and all) as the address tool segment.
@@ -114,9 +122,11 @@ const FailureApi = HttpApi.make("failuresTest")
114122

115123
// Build an executor + connection from the FailureApi HttpApi against an
116124
// arbitrary baseUrl (used for the Node-transport socket-drop / slow cases).
117-
const buildExecutor = (baseUrl: string) =>
125+
const buildExecutor = (baseUrl: string, httpClientLayer = FetchHttpClient.layer) =>
118126
Effect.gen(function* () {
119-
const executor = yield* createExecutor(makeTestConfig({ plugins: testPlugins() }));
127+
const executor = yield* createExecutor(
128+
makeTestConfig({ plugins: testPlugins(httpClientLayer) }),
129+
);
120130
yield* executor.openapi.addSpec(
121131
makeOpenApiHttpApiTestIntegrationConfig(FailureApi, { slug: "f", baseUrl }),
122132
);
@@ -450,6 +460,62 @@ describe("OpenAPI upstream failure modes", () => {
450460
}),
451461
);
452462

463+
it.effect("request encoding failures remain invocation failures", () =>
464+
Effect.gen(function* () {
465+
const httpClientLayer = Layer.succeed(
466+
HttpClient.HttpClient,
467+
HttpClient.make((request) =>
468+
Effect.fail(
469+
new HttpClientError.HttpClientError({
470+
reason: new HttpClientError.EncodeError({ request, cause: new AdapterDefect() }),
471+
}),
472+
),
473+
),
474+
);
475+
const { executor, address } = yield* buildExecutor(
476+
"https://upstream.example",
477+
httpClientLayer,
478+
);
479+
const exit = yield* executor.execute(address, {}).pipe(Effect.exit);
480+
expect(Exit.isFailure(exit)).toBe(true);
481+
}),
482+
);
483+
484+
it.effect("transport defects remain defects", () =>
485+
Effect.gen(function* () {
486+
const defect = new AdapterDefect();
487+
const httpClientLayer = Layer.succeed(
488+
HttpClient.HttpClient,
489+
HttpClient.make(() => Effect.die(defect)),
490+
);
491+
const { executor, address } = yield* buildExecutor(
492+
"https://upstream.example",
493+
httpClientLayer,
494+
);
495+
const exit = yield* executor.execute(address, {}).pipe(Effect.exit);
496+
expect(Exit.isFailure(exit)).toBe(true);
497+
expect(Exit.match(exit, { onFailure: Cause.hasDies, onSuccess: () => false })).toBe(true);
498+
}),
499+
);
500+
501+
it.effect("interrupted transport remains interrupted", () =>
502+
Effect.gen(function* () {
503+
const httpClientLayer = Layer.succeed(
504+
HttpClient.HttpClient,
505+
HttpClient.make(() => Effect.interrupt),
506+
);
507+
const { executor, address } = yield* buildExecutor(
508+
"https://upstream.example",
509+
httpClientLayer,
510+
);
511+
const exit = yield* executor.execute(address, {}).pipe(Effect.exit);
512+
expect(Exit.isFailure(exit)).toBe(true);
513+
expect(Exit.match(exit, { onFailure: Cause.hasInterrupts, onSuccess: () => false })).toBe(
514+
true,
515+
);
516+
}),
517+
);
518+
453519
// Port 1 refuses immediately. The same path used to throw `Internal tool
454520
// error [hex]` because the raw HttpClientError carries the request URL.
455521
it.effect("connection refused returns upstream_unreachable without leaking the path", () =>

0 commit comments

Comments
 (0)