diff --git a/apps/web/src/lib/services/common/http-client.test.ts b/apps/web/src/lib/services/common/http-client.test.ts new file mode 100644 index 000000000..86d47b933 --- /dev/null +++ b/apps/web/src/lib/services/common/http-client.test.ts @@ -0,0 +1,50 @@ +import { assert, describe, it } from "@effect/vitest" +import { Effect, Fiber, Result } from "effect" +import { TestClock } from "effect/testing" +import { FetchHttpClient, HttpClient, HttpClientError } from "effect/unstable/http" + +import { withRequestTimeout } from "./http-client" + +const NEVER_RESPONDS: typeof globalThis.fetch = () => new Promise(() => {}) + +const get = (fetchStub: typeof globalThis.fetch) => + Effect.gen(function* () { + const client = withRequestTimeout(yield* HttpClient.HttpClient) + return yield* client.get("https://api.test/anything") + }).pipe( + Effect.provide(FetchHttpClient.layer), + Effect.provideService(FetchHttpClient.Fetch, fetchStub), + ) + +/** + * The deadline used to live in the fetch implementation as + * `signal: init?.signal ?? AbortSignal.timeout(…)`. `FetchHttpClient` always + * supplies its own signal, so that branch never ran and a stalled request stayed + * pending forever. + */ +describe("withRequestTimeout", () => { + it.effect("fails a request that never responds, rather than hanging", () => + Effect.gen(function* () { + const fiber = yield* Effect.forkChild(Effect.result(get(NEVER_RESPONDS))) + + yield* TestClock.adjust("44 seconds") + assert.isUndefined(fiber.pollUnsafe(), "gave up before the deadline") + + yield* TestClock.adjust("1 second") + const result = yield* Fiber.join(fiber) + assert.isTrue(Result.isFailure(result), "request outlived the deadline") + assert.instanceOf( + Result.isFailure(result) ? result.failure : undefined, + HttpClientError.HttpClientError, + ) + }), + ) + + it.effect("leaves a responding request alone", () => + Effect.gen(function* () { + const response = yield* get(async () => new Response("{}", { status: 200 })) + + assert.strictEqual(response.status, 200) + }), + ) +}) diff --git a/apps/web/src/lib/services/common/http-client.ts b/apps/web/src/lib/services/common/http-client.ts index 10500ca29..72ef05bb6 100644 --- a/apps/web/src/lib/services/common/http-client.ts +++ b/apps/web/src/lib/services/common/http-client.ts @@ -1,10 +1,10 @@ -import { FetchHttpClient } from "effect/unstable/http" -import { Layer } from "effect" +import { FetchHttpClient, HttpClient, HttpClientError } from "effect/unstable/http" +import { Clock, Duration, Effect, Layer } from "effect" import { apiBaseUrl } from "./api-base-url" import { getMapleAuthHeaders } from "./auth-headers" import { noteReachable, noteUnreachable, originOf } from "./peer-reachability" -const CLIENT_TIMEOUT_MS = 45_000 +const CLIENT_TIMEOUT = Duration.seconds(45) const resolveRequestUrl = (input: RequestInfo | URL): string => { if (typeof input === "string") return input @@ -31,27 +31,58 @@ const mapleFetch: typeof globalThis.fetch = async (input, init) => { // only observes; `normalizeWarehouseError` is what reads it to decide whether // a failure is the network's fault. An abort is evidence of nothing either // way: we stopped listening. - return globalThis - .fetch(input, { - ...init, - headers, - signal: init?.signal ?? AbortSignal.timeout(CLIENT_TIMEOUT_MS), - }) - .then( - (response) => { - noteReachable(origin) - return response - }, - (cause: unknown) => { - if (!isAbort(cause)) noteUnreachable(origin, Date.now()) - throw cause - }, - ) + return globalThis.fetch(input, { ...init, headers }).then( + (response) => { + noteReachable(origin) + return response + }, + (cause: unknown) => { + if (!isAbort(cause)) noteUnreachable(origin, Date.now()) + throw cause + }, + ) } const isAbort = (cause: unknown): boolean => typeof cause === "object" && cause !== null && "name" in cause && cause.name === "AbortError" -export const MapleFetchHttpClientLive = FetchHttpClient.layer.pipe( +/** + * The client-wide request deadline. + * + * It belongs here rather than in `mapleFetch` because a deadline the fetch + * layer holds is not a deadline at all: `FetchHttpClient` always passes its own + * signal, so a `signal: init?.signal ?? AbortSignal.timeout(…)` never chose the + * timeout, and a stalled request stayed pending forever. Timing out the fiber + * interrupts it, and that interruption is what aborts the in-flight fetch — + * one deadline, enforced by the runtime rather than a second abort signal + * raced against the first. + * + * `mapleFetch` sees the abort and reads it as evidence of nothing, which is + * right for an interrupt and wrong for this, so the origin is marked here. + */ +export const withRequestTimeout = (client: HttpClient.HttpClient): HttpClient.HttpClient => + HttpClient.transform(client, (effect, request) => + effect.pipe( + Effect.timeoutOrElse({ + duration: CLIENT_TIMEOUT, + orElse: () => + Effect.gen(function* () { + noteUnreachable(originOf(request.url), yield* Clock.currentTimeMillis) + return yield* new HttpClientError.HttpClientError({ + reason: new HttpClientError.TransportError({ + request, + description: `No response within ${Duration.toMillis(CLIENT_TIMEOUT)}ms`, + }), + }) + }), + }), + ), + ) + +export const MapleFetchHttpClientLive = Layer.effect( + HttpClient.HttpClient, + Effect.map(HttpClient.HttpClient, withRequestTimeout), +).pipe( + Layer.provide(FetchHttpClient.layer), Layer.provideMerge(Layer.succeed(FetchHttpClient.Fetch, mapleFetch)), )