Skip to content

Commit acebf64

Browse files
authored
fix(http): preserve Effect error responses (#1011)
1 parent a81bba6 commit acebf64

4 files changed

Lines changed: 172 additions & 21 deletions

File tree

packages/alchemy/src/Http.ts

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,15 @@ import * as Cause from "effect/Cause";
22
import * as Config from "effect/Config";
33
import * as Context from "effect/Context";
44
import * as Effect from "effect/Effect";
5+
import * as ErrorReporter from "effect/ErrorReporter";
56
import * as Layer from "effect/Layer";
67
import * as Option from "effect/Option";
78
import type { Scope } from "effect/Scope";
89
import type { HttpBodyError } from "effect/unstable/http/HttpBody";
9-
import type { HttpServerError } from "effect/unstable/http/HttpServerError";
10+
import {
11+
causeResponse,
12+
type HttpServerError,
13+
} from "effect/unstable/http/HttpServerError";
1014
import { HttpServerRequest } from "effect/unstable/http/HttpServerRequest";
1115
import * as HttpServerResponse from "effect/unstable/http/HttpServerResponse";
1216

@@ -82,26 +86,45 @@ export const safeHttpEffect = <Req = never>(
8286
: response,
8387
),
8488
) as any as HttpEffect<Req>,
85-
(cause) => {
86-
// ClientAbort interrupts are not real failures — the client closed
87-
// the connection. Skip logging and respond with 499 if applicable.
88-
if (Cause.hasInterruptsOnly(cause)) {
89-
return Effect.succeed(HttpServerResponse.empty({ status: 499 }));
90-
}
91-
// Log the full cause server-side so operators can debug, but return
92-
// a generic 500 to the client. Causes can contain sensitive data
93-
// (prompt contents, API keys baked into error messages, internal
94-
// file paths) and should never be echoed back to the network.
95-
return Effect.logError("HTTP handler failed", cause).pipe(
96-
Effect.as(
97-
HttpServerResponse.text("Internal Server Error", {
98-
status: 500,
99-
statusText: "Internal Server Error",
100-
}),
89+
(cause) =>
90+
// `causeResponse` is effect's native failure boundary: Respondable
91+
// failures keep their intended response (e.g. RouteNotFound -> 404),
92+
// client aborts map to 499, and everything else becomes an empty 500 —
93+
// the cause is never echoed to the network, as it can contain sensitive
94+
// data (prompt contents, API keys baked into error messages, internal
95+
// file paths).
96+
causeResponse(cause).pipe(
97+
Effect.flatMap(([response, reportableCause]) =>
98+
Effect.withFiber((fiber) =>
99+
fiber.getRef(ErrorReporter.CurrentErrorReporters).size > 0
100+
? ErrorReporter.report(reportableCause)
101+
: logUnreportedCause(reportableCause),
102+
).pipe(Effect.as(response)),
101103
),
102-
);
103-
},
104+
),
105+
);
106+
107+
/**
108+
* No `ErrorReporter` is registered by default, so without a fallback a defect
109+
* in a deployed Function/Worker would produce a bare 500 and vanish without a
110+
* trace. Log the cause server-side so operators can debug, applying the same
111+
* filtering `ErrorReporter.make` reporters do: interrupts (client aborts) and
112+
* `ErrorReporter.ignore`-annotated values (Respondable errors like
113+
* RouteNotFound, and the response `causeResponse` appends) are not failures
114+
* and are skipped.
115+
*/
116+
const logUnreportedCause = (cause: Cause.Cause<unknown>) => {
117+
const failures = cause.reasons.filter(
118+
(reason) =>
119+
reason._tag !== "Interrupt" &&
120+
!ErrorReporter.isIgnored(
121+
reason._tag === "Fail" ? reason.error : reason.defect,
122+
),
104123
);
124+
return failures.length === 0
125+
? Effect.void
126+
: Effect.logError("HTTP handler failed", Cause.fromReasons(failures));
127+
};
105128

106129
export const resolvePort = (options: { port?: number } | undefined) =>
107130
options?.port !== undefined

packages/alchemy/test/AWS/Lambda/HttpServer.test.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,9 +215,14 @@ describe("AWS.Lambda.HttpServer", () => {
215215
),
216216
);
217217

218+
// Defects render effect's native `causeResponse` fallback — an empty
219+
// `500` with no body or content-type (the same wire shape effect's own
220+
// servers produce) — instead of alchemy's former hand-rolled
221+
// "Internal Server Error" text response. The cause is reported/logged
222+
// server-side, never echoed to the client.
218223
expect(result.statusCode).toBe(500);
219-
expect(result.headers?.["content-type"]).toContain("text/plain");
220-
expect(result.body).toBe("Internal Server Error");
224+
expect(result.headers?.["content-type"]).toBeUndefined();
225+
expect(result.body).toBeUndefined();
221226
});
222227
});
223228

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import * as Cloudflare from "@/Cloudflare/index.ts";
2+
import * as Alchemy from "@/index.ts";
3+
import * as Test from "@/Test/Alchemy";
4+
import { expect } from "alchemy-test";
5+
import * as Effect from "effect/Effect";
6+
import * as Schedule from "effect/Schedule";
7+
import * as HttpClient from "effect/unstable/http/HttpClient";
8+
import { expectUrlContains } from "../Utils/Http.ts";
9+
import HttpServerWorker, {
10+
readyMarker,
11+
sensitiveContext,
12+
} from "./fixtures/http-server-worker.ts";
13+
14+
const { test, beforeAll, afterAll, deploy, destroy } = Test.make({
15+
providers: Cloudflare.providers(),
16+
});
17+
18+
const Stack = Alchemy.Stack(
19+
"WorkersHttpServerStack",
20+
{ providers: Cloudflare.providers(), state: Cloudflare.state() },
21+
Effect.gen(function* () {
22+
const worker = yield* HttpServerWorker;
23+
return { url: worker.url.as<string>() };
24+
}),
25+
);
26+
27+
const stack = beforeAll(deploy(Stack));
28+
afterAll.skipIf(!!process.env.NO_DESTROY)(destroy(Stack));
29+
30+
/**
31+
* GET `url` until it serves `status` with an empty body. Both error routes
32+
* respond with no body, which also distinguishes them from the workers.dev
33+
* placeholder page (a 404 *with* a body) during edge propagation.
34+
*/
35+
const getEmptyResponse = (url: string, status: number) =>
36+
Effect.gen(function* () {
37+
const client = yield* HttpClient.HttpClient;
38+
const response = yield* client.get(url);
39+
const body = yield* response.text;
40+
if (response.status !== status || body !== "") {
41+
return yield* Effect.fail(
42+
new Error(
43+
`expected empty ${status} from ${url}, got ${response.status}: ${body.slice(0, 160)}`,
44+
),
45+
);
46+
}
47+
return response;
48+
}).pipe(
49+
Effect.retry({ schedule: Schedule.spaced("1500 millis"), times: 20 }),
50+
);
51+
52+
test(
53+
"a Respondable defect keeps its intended response over the wire",
54+
Effect.gen(function* () {
55+
const { url } = yield* stack;
56+
yield* expectUrlContains(url, readyMarker);
57+
58+
yield* getEmptyResponse(`${url}/missing`, 404);
59+
}),
60+
{ timeout: 180_000 },
61+
);
62+
63+
test(
64+
"a failed handler responds 500 without exposing the cause",
65+
Effect.gen(function* () {
66+
const { url } = yield* stack;
67+
yield* expectUrlContains(url, readyMarker);
68+
69+
const response = yield* getEmptyResponse(`${url}/boom`, 500);
70+
const wireResponse = JSON.stringify(response.headers);
71+
for (const sensitiveValue of sensitiveContext) {
72+
expect(wireResponse).not.toContain(sensitiveValue);
73+
}
74+
}),
75+
{ timeout: 180_000 },
76+
);
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import * as Cloudflare from "@/Cloudflare/index.ts";
2+
import * as Effect from "effect/Effect";
3+
import * as HttpServerError from "effect/unstable/http/HttpServerError";
4+
import { HttpServerRequest } from "effect/unstable/http/HttpServerRequest";
5+
import * as HttpServerResponse from "effect/unstable/http/HttpServerResponse";
6+
7+
export const readyMarker = "http-server-worker-ready";
8+
9+
/**
10+
* Values that must never escape the Worker over the wire when a handler
11+
* fails. The test asserts none of them appear in the HTTP response.
12+
*/
13+
export const sensitiveContext = [
14+
"sk_live_alchemy_super_secret",
15+
"tenant-customer-42",
16+
"/srv/alchemy/private/customer-42.json",
17+
"10.42.0.17",
18+
];
19+
20+
export default class HttpServerWorker extends Cloudflare.Worker<HttpServerWorker>()(
21+
"HttpServerWorker",
22+
{
23+
main: import.meta.url,
24+
},
25+
Effect.gen(function* () {
26+
return {
27+
fetch: Effect.gen(function* () {
28+
const request = yield* HttpServerRequest;
29+
if (request.url.startsWith("/missing")) {
30+
// A Respondable error escaping as a defect must keep its intended
31+
// response (404), not be flattened into a generic 500.
32+
return yield* Effect.die(
33+
new HttpServerError.RouteNotFound({ request }),
34+
);
35+
}
36+
if (request.url.startsWith("/boom")) {
37+
return yield* Effect.fail(
38+
new Error(
39+
`Sensitive handler context: ${sensitiveContext.join(" ")}`,
40+
),
41+
).pipe(Effect.orDie);
42+
}
43+
return HttpServerResponse.text(readyMarker);
44+
}),
45+
};
46+
}),
47+
) {}

0 commit comments

Comments
 (0)