From ab542ce6a17fc7907253a46a34a0039fd718aa45 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Tue, 6 Oct 2026 20:09:59 +0900 Subject: [PATCH 1/3] Forward asynchronous Express failures Forward rejected context-data factories and federation fetches to Express error middleware so applications can send their normal error response. Normalize empty rejection reasons and keep the existing behavior after Fedify hands off a request to avoid calling next twice. Cover both rejection paths, error identity, empty rejection reasons, synchronous factory errors and deferred successful context data with HTTP regression tests. Fixes https://github.com/fedify-dev/fedify/issues/1244 Assisted-by: Codex:gpt-6.1-sol Assisted-by: Claude Code:claude-fable-5-1 --- CHANGES.md | 11 ++ changes.d/express/async-failures.md | 10 ++ packages/express/src/index.test.ts | 164 ++++++++++++++++++++++++++++ packages/express/src/index.ts | 10 +- 4 files changed, 193 insertions(+), 2 deletions(-) create mode 100644 changes.d/express/async-failures.md diff --git a/CHANGES.md b/CHANGES.md index 9f6543c5b..fb7ebb560 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -19,6 +19,17 @@ To be released. [#1255]: https://github.com/fedify-dev/fedify/issues/1255 [#1260]: https://github.com/fedify-dev/fedify/pull/1260 +### @fedify/express + + - Fixed asynchronous `contextDataFactory` and `federation.fetch()` failures + being left as unhandled rejections in the Express integration. Errors + that occur before Fedify passes the request to the next middleware now + reach Express error-handling middleware, so applications can send their + usual error response. [[#1244], [#1261]] + +[#1244]: https://github.com/fedify-dev/fedify/issues/1244 +[#1261]: https://github.com/fedify-dev/fedify/pull/1261 + Version 2.0.31 -------------- diff --git a/changes.d/express/async-failures.md b/changes.d/express/async-failures.md new file mode 100644 index 000000000..e751ea288 --- /dev/null +++ b/changes.d/express/async-failures.md @@ -0,0 +1,10 @@ +--- +links: + '#1244': https://github.com/fedify-dev/fedify/issues/1244 + '#1261': https://github.com/fedify-dev/fedify/pull/1261 +--- + - Fixed asynchronous `contextDataFactory` and `federation.fetch()` failures + being left as unhandled rejections in the Express integration. Errors + that occur before Fedify passes the request to the next middleware now + reach Express error-handling middleware, so applications can send their + usual error response. [[#1244], [#1261]] diff --git a/packages/express/src/index.test.ts b/packages/express/src/index.test.ts index 9c4aec47c..18b80589b 100644 --- a/packages/express/src/index.test.ts +++ b/packages/express/src/index.test.ts @@ -19,10 +19,174 @@ async function withServer( try { await callback(`http://127.0.0.1:${port}`); } finally { + server.closeAllConnections(); await new Promise((resolve) => server.close(() => resolve())); } } +for (const failure of ["contextDataFactory", "federation.fetch"] as const) { + test(`integrateFederation() forwards a rejected ${failure} to error middleware`, async () => { + const error = new Error(`${failure} failed`); + const context = Promise.withResolvers(); + const requested = Promise.withResolvers(); + let fetchCalls = 0; + const errors: unknown[] = []; + const federation = { + fetch(_request: Request, options: { contextData: string }) { + fetchCalls++; + assert.equal(options.contextData, "context value"); + return Promise.reject(error); + }, + }; + const app = express(); + app.use(integrateFederation(federation as never, () => { + requested.resolve(); + return context.promise; + })); + app.use(( + error: Error, + _req: express.Request, + res: express.Response, + _next: express.NextFunction, + ) => { + errors.push(error); + res.status(500).send(error.message); + }); + + await withServer(app, async (origin) => { + const responsePromise = fetch(origin, { + signal: AbortSignal.timeout(5000), + }); + await requested.promise; + assert.equal(fetchCalls, 0); + if (failure === "contextDataFactory") context.reject(error); + else context.resolve("context value"); + const response = await responsePromise; + assert.equal(response.status, 500); + assert.equal(await response.text(), error.message); + }); + assert.equal(fetchCalls, failure === "contextDataFactory" ? 0 : 1); + assert.equal(errors.length, 1); + assert.strictEqual(errors[0], error); + }); +} + +test("integrateFederation() forwards a rejection without a reason to error middleware", async () => { + const errors: unknown[] = []; + let fetchCalls = 0; + const federation = { + fetch() { + fetchCalls++; + return Promise.resolve(new Response("ok")); + }, + }; + const app = express(); + app.use(integrateFederation( + federation as never, + () => Promise.reject(), + )); + app.use((_req: express.Request, res: express.Response) => { + res.status(200).send("unexpected fallthrough"); + }); + app.use(( + error: Error, + _req: express.Request, + res: express.Response, + _next: express.NextFunction, + ) => { + errors.push(error); + res.status(500).send(error.message); + }); + + await withServer(app, async (origin) => { + const response = await fetch(origin, { + signal: AbortSignal.timeout(5000), + }); + assert.equal(response.status, 500); + assert.ok((await response.text()).length > 0); + }); + assert.equal(fetchCalls, 0); + assert.equal(errors.length, 1); + assert.ok(errors[0] instanceof Error); +}); + +test("integrateFederation() leaves synchronous factory errors to Express", async () => { + const error = new Error("context failed"); + const errors: unknown[] = []; + let fetchCalls = 0; + const federation = { + fetch() { + fetchCalls++; + return Promise.resolve(new Response("ok")); + }, + }; + const app = express(); + app.use(integrateFederation(federation as never, () => { + throw error; + })); + app.use(( + error: Error, + _req: express.Request, + res: express.Response, + _next: express.NextFunction, + ) => { + errors.push(error); + res.status(500).send(error.message); + }); + + await withServer(app, async (origin) => { + const response = await fetch(origin, { + signal: AbortSignal.timeout(5000), + }); + assert.equal(response.status, 500); + assert.equal(await response.text(), error.message); + }); + assert.equal(fetchCalls, 0); + assert.equal(errors.length, 1); + assert.strictEqual(errors[0], error); +}); + +test("integrateFederation() waits for successful asynchronous context data", async () => { + const context = Promise.withResolvers(); + const requested = Promise.withResolvers(); + let fetchCalls = 0; + const errors: unknown[] = []; + const federation = { + fetch(_request: Request, options: { contextData: string }) { + fetchCalls++; + return Promise.resolve(new Response(options.contextData)); + }, + }; + const app = express(); + app.use(integrateFederation(federation as never, () => { + requested.resolve(); + return context.promise; + })); + app.use(( + error: Error, + _req: express.Request, + res: express.Response, + _next: express.NextFunction, + ) => { + errors.push(error); + res.status(500).send(error.message); + }); + + await withServer(app, async (origin) => { + const responsePromise = fetch(origin, { + signal: AbortSignal.timeout(5000), + }); + await requested.promise; + assert.equal(fetchCalls, 0); + context.resolve("context value"); + const response = await responsePromise; + assert.equal(response.status, 200); + assert.equal(await response.text(), "context value"); + }); + assert.equal(fetchCalls, 1); + assert.deepEqual(errors, []); +}); + // Sends the body only after a delay, so that the request reaches Fedify // before any of its body has arrived: function delayedBody(body: string): ReadableStream { diff --git a/packages/express/src/index.ts b/packages/express/src/index.ts index 422ca6163..05952661c 100644 --- a/packages/express/src/index.ts +++ b/packages/express/src/index.ts @@ -26,9 +26,9 @@ export function integrateFederation( const contextDataPromise = contextData instanceof Promise ? contextData : Promise.resolve(contextData); + let notFound = false; + let notAcceptable = false; contextDataPromise.then(async (contextData) => { - let notFound = false; - let notAcceptable = false; const response = await federation.fetch(request, { contextData, onNotFound: () => { @@ -70,6 +70,12 @@ export function integrateFederation( res.json = () => res; res.removeHeader = () => res; res.setHeader = () => res; + }).catch((error) => { + // Once Fedify has handed the request to Express, do not call next again. + if (notFound || notAcceptable) throw error; + next( + error || new Error("The federation middleware promise was rejected."), + ); }); }; } From 033d6e8f648439fed2d299f7cecd97c715ff75ef Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Tue, 6 Oct 2026 21:49:05 +0900 Subject: [PATCH 2/3] Bound Express context factory test waits Race the factory-start signal against the HTTP response so an early response or fetch timeout fails the test when the factory never runs. This also ensures the server cleanup runs instead of waiting forever. https://github.com/fedify-dev/fedify/pull/1261#discussion_r4195271164 Changelog: none Assisted-by: Codex:gpt-6.1-sol --- packages/express/src/index.test.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/express/src/index.test.ts b/packages/express/src/index.test.ts index 18b80589b..271fd5da7 100644 --- a/packages/express/src/index.test.ts +++ b/packages/express/src/index.test.ts @@ -57,7 +57,12 @@ for (const failure of ["contextDataFactory", "federation.fetch"] as const) { const responsePromise = fetch(origin, { signal: AbortSignal.timeout(5000), }); - await requested.promise; + await Promise.race([ + requested.promise, + responsePromise.then(() => { + assert.fail("The request completed before the context factory ran."); + }), + ]); assert.equal(fetchCalls, 0); if (failure === "contextDataFactory") context.reject(error); else context.resolve("context value"); @@ -176,7 +181,12 @@ test("integrateFederation() waits for successful asynchronous context data", asy const responsePromise = fetch(origin, { signal: AbortSignal.timeout(5000), }); - await requested.promise; + await Promise.race([ + requested.promise, + responsePromise.then(() => { + assert.fail("The request completed before the context factory ran."); + }), + ]); assert.equal(fetchCalls, 0); context.resolve("context value"); const response = await responsePromise; From 0766bb892e952aa6af51b2ef1ec2320a356717dd Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Tue, 6 Oct 2026 21:54:43 +0900 Subject: [PATCH 3/3] Handle Express control-value rejections Express treats route and router strings passed to next() as control signals. Wrap those promise rejection reasons in Error objects so asynchronous failures reach error middleware instead of falling through or leaving the router. Preserve other rejection reasons as before. Cover both reserved strings from the context factory and federation fetch paths with HTTP regression tests. https://github.com/fedify-dev/fedify/pull/1261#discussion_r4195271168 Assisted-by: Codex:gpt-6.1-sol --- packages/express/src/index.test.ts | 47 ++++++++++++++++++++++++++++++ packages/express/src/index.ts | 3 +- 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/packages/express/src/index.test.ts b/packages/express/src/index.test.ts index 271fd5da7..553a6af77 100644 --- a/packages/express/src/index.test.ts +++ b/packages/express/src/index.test.ts @@ -76,6 +76,53 @@ for (const failure of ["contextDataFactory", "federation.fetch"] as const) { }); } +for (const failure of ["contextDataFactory", "federation.fetch"] as const) { + for (const reason of ["route", "router"]) { + test(`integrateFederation() forwards a ${reason} rejection from ${failure} to error middleware`, async () => { + const errors: unknown[] = []; + let fetchCalls = 0; + const federation = { + fetch() { + fetchCalls++; + return Promise.reject(reason); + }, + }; + const app = express(); + app.use(integrateFederation( + federation as never, + () => + failure === "contextDataFactory" + ? Promise.reject(reason) + : Promise.resolve(undefined), + )); + app.use((_req: express.Request, res: express.Response) => { + res.send("unexpected fallthrough"); + }); + app.use(( + error: Error, + _req: express.Request, + res: express.Response, + _next: express.NextFunction, + ) => { + errors.push(error); + res.status(500).send(error.message); + }); + + await withServer(app, async (origin) => { + const response = await fetch(origin, { + signal: AbortSignal.timeout(5000), + }); + assert.equal(response.status, 500); + assert.equal(await response.text(), reason); + }); + assert.equal(fetchCalls, failure === "contextDataFactory" ? 0 : 1); + assert.equal(errors.length, 1); + assert.ok(errors[0] instanceof Error); + assert.equal(errors[0].message, reason); + }); + } +} + test("integrateFederation() forwards a rejection without a reason to error middleware", async () => { const errors: unknown[] = []; let fetchCalls = 0; diff --git a/packages/express/src/index.ts b/packages/express/src/index.ts index 05952661c..3add45ef6 100644 --- a/packages/express/src/index.ts +++ b/packages/express/src/index.ts @@ -74,7 +74,8 @@ export function integrateFederation( // Once Fedify has handed the request to Express, do not call next again. if (notFound || notAcceptable) throw error; next( - error || new Error("The federation middleware promise was rejected."), + error === "route" || error === "router" ? new Error(error) : error || + new Error("The federation middleware promise was rejected."), ); }); };