Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-httplayerrouter-shared-error-encoder.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@effect/platform": patch
---

Fix `HttpLayerRouter.addHttpApi` so two registered APIs no longer share error encoders. Previously each call's middleware was added to a shared context map and applied to every route, so a 500 response from one API was sometimes encoded against the other API's error schema. The fix scopes each API's middleware to its own endpoints (matched by method + path) and wraps each route handler directly.
26 changes: 18 additions & 8 deletions packages/platform/src/HttpLayerRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1008,31 +1008,44 @@ export const addHttpApi = <Id extends string, Groups extends HttpApiGroup.HttpAp
| R
| HttpApiGroup.HttpApiGroup.ToService<Id, Groups>
| HttpApiGroup.HttpApiGroup.ErrorContext<Groups>
> => {
const ApiMiddleware = middleware(HttpApiBuilder.buildMiddleware(api)).layer as Layer.Layer<never>
return HttpApiBuilder.Router.unwrap(Effect.fnUntraced(function*(router_) {
> =>
HttpApiBuilder.Router.unwrap(Effect.fnUntraced(function*(router_) {
const router = yield* HttpRouter
let existing = existingRoutesMap.get(router)
if (!existing) {
existing = new Set()
existingRoutesMap.set(router, existing)
}
const apiMiddleware = yield* HttpApiBuilder.buildMiddleware(api)
const context = yield* Effect.context<
| Etag.Generator
| HttpRouter
| FileSystem
| HttpPlatform
| Path
>()
const apiEndpointKeys = new Set<string>()
for (const groupKey of Object.keys((api as any).groups)) {
const group = (api as any).groups[groupKey]
for (const endpointKey of Object.keys(group.endpoints)) {
const endpoint = group.endpoints[endpointKey]
apiEndpointKeys.add(`${endpoint.method} ${endpoint.path}`)
}
}
const routes = Arr.empty<Route<any, any>>()
for (const route of router_.routes) {
if (!apiEndpointKeys.has(`${(route as any).method} ${(route as any).path}`)) {
continue
}
if (existing.has(route)) {
continue
}
existing.add(route)
routes.push(makeRoute({
...route as any,
handler: Effect.mapInputContext(route.handler, (input) => Context.merge(context, input))
handler: apiMiddleware(
Effect.mapInputContext(route.handler, (input) => Context.merge(context, input)) as any
)
}))
}

Expand All @@ -1042,10 +1055,7 @@ export const addHttpApi = <Id extends string, Groups extends HttpApiGroup.HttpAp
const spec = OpenApi.fromApi(api)
yield* router.add("GET", options.openapiPath, Effect.succeed(HttpServerResponse.unsafeJson(spec)))
}
}, Layer.effectDiscard)).pipe(
Layer.provide(ApiMiddleware)
)
}
}, Layer.effectDiscard))

const existingRoutesMap = new WeakMap<HttpRouter, Set<any>>()

Expand Down
71 changes: 71 additions & 0 deletions packages/platform/test/HttpLayerRouter.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { HttpApi, HttpApiBuilder, HttpApiEndpoint, HttpApiGroup, HttpLayerRouter, HttpServer } from "@effect/platform"
import { describe, test } from "@effect/vitest"
import { strictEqual } from "@effect/vitest/utils"
import { Effect, Layer, Schema } from "effect"

describe("HttpLayerRouter", () => {
describe("addHttpApi", () => {
test("two registered APIs use their own error schemas (regression #6243)", async () => {
const Api1 = HttpApi.make("Api1").add(
HttpApiGroup.make("group1").add(
HttpApiEndpoint.get("endpoint1")`/1`.addError(Schema.transformLiteral("BAD", "x"))
)
)
const Handlers1 = HttpApiBuilder.group(
Api1,
"group1",
(_) => _.handle("endpoint1", () => Effect.fail("x" as const))
)
const Routes1 = HttpLayerRouter.addHttpApi(Api1).pipe(
Layer.provide(Layer.mergeAll(Handlers1))
)

const Api2 = HttpApi.make("Api2").add(
HttpApiGroup.make("group2").add(
HttpApiEndpoint.get("endpoint2")`/2`.addError(Schema.transformLiteral("GOOD", "x"))
)
)
const Handlers2 = HttpApiBuilder.group(
Api2,
"group2",
(_) => _.handle("endpoint2", () => Effect.fail("x" as const))
)
const Routes2 = HttpLayerRouter.addHttpApi(Api2).pipe(
Layer.provide(Layer.mergeAll(Handlers2))
)

const AllRoutes = Layer.mergeAll(Routes1, Routes2)
const { handler } = HttpLayerRouter.toWebHandler(
AllRoutes.pipe(Layer.provide(HttpServer.layerContext))
)

const response1 = await handler(new Request("http://localhost:3000/1"))
const body1 = await response1.text()
strictEqual(body1, "\"BAD\"", "Api1's endpoint must use Api1's error schema")

const response2 = await handler(new Request("http://localhost:3000/2"))
const body2 = await response2.text()
strictEqual(body2, "\"GOOD\"", "Api2's endpoint must use Api2's error schema, not Api1's")
})

test("single registered API still encodes errors using its own schema", async () => {
const Api = HttpApi.make("Api").add(
HttpApiGroup.make("group").add(
HttpApiEndpoint.get("endpoint")`/`.addError(Schema.transformLiteral("OK", "x"))
)
)
const Handlers = HttpApiBuilder.group(Api, "group", (_) => _.handle("endpoint", () => Effect.fail("x" as const)))
const Routes = HttpLayerRouter.addHttpApi(Api).pipe(
Layer.provide(Layer.mergeAll(Handlers))
)

const { handler } = HttpLayerRouter.toWebHandler(
Routes.pipe(Layer.provide(HttpServer.layerContext))
)

const response = await handler(new Request("http://localhost:3000/"))
const body = await response.text()
strictEqual(body, "\"OK\"")
})
})
})
Loading