feat(plugins): api.mountApp for wrapped node-style apps (#583)#590
Conversation
The second seam #206 forced (after appPaths and getAgent): a plugin that hands requests to an existing (req, res) handler — a wrapped HTTP app, reverse proxy, or framework adapter — hangs, because Fastify's content parsers drain the request stream before the handler runs. The Tideholm and bridge adapters each hand-rolled the same scoped-parser + hijack incantation to work around it. api.mountApp(handler, { prefix }) bundles the four things every such plugin needs: the appPaths WAC exemption, a scoped pass-through content parser (unconsumed body stream), reply.hijack(), and registration on the bare prefix plus its subtree. Defaults to the entry's prefix; an explicit prefix mounts and exempts a second app. Tests: the exact Tideholm case (JSON POST round-trips through a node handler), bare + subtree serving, secondary mount, and host parsing intact outside the mount. docs/configuration.md updated.
There was a problem hiding this comment.
Pull request overview
This PR adds a first-class plugin API helper (api.mountApp) to support mounting wrapped Node-style (req, res) applications under a plugin prefix without Fastify consuming the request stream first, solving the hang described in #583 while keeping host parsing behavior unchanged outside the mount scope.
Changes:
- Add
api.mountApp(handler, { prefix })to the plugin loader API to register a hijacked route with a scoped pass-through content parser and WAC exemption support for secondary prefixes. - Add a dedicated test suite covering body round-trip through a mounted handler, subtree routing, secondary-prefix WAC exemption, and non-interference with host WAC behavior.
- Document
mountAppusage and its behavioral guarantees indocs/configuration.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/plugin-mountapp.test.js | Adds regression/acceptance tests for mounting a node-style handler under plugin prefixes. |
| src/plugins.js | Implements api.mountApp, including scoped parser setup, hijacking, and secondary-prefix appPaths exemption. |
| docs/configuration.md | Documents api.mountApp for plugin authors alongside the loader API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const mountPrefix = normalizePrefix(opts.prefix) || prefix; | ||
| if (!mountPrefix) { | ||
| throw new Error(`plugin ${id}: mountApp needs a prefix (entry.prefix or opts.prefix)`); | ||
| } |
| const wrapped = (request, reply) => { | ||
| reply.hijack(); | ||
| handler(request.raw, reply.raw); | ||
| }; |
There was a problem hiding this comment.
Confirmed and fixed in 6b300e4 — wrapped now mirrors ws.route's contract: try/catch + Promise.resolve().catch, log, 500 if nothing has gone out, else destroy the socket. Regression tests cover sync throw and async rejection, both asserting the server keeps answering afterwards. Full suite 1023/1023.
- a provided-but-invalid opts.prefix now fails activate() (same rule as entry.prefix) instead of silently mounting — and WAC-exempting — the app at the entry prefix - the wrapped handler is guarded like ws.route: after hijack() Fastify sends nothing, so a sync throw hung the client and an unawaited async rejection could take the process down; both now log and answer 500 when nothing has gone out, else drop the one affected socket Three regression tests; full suite 1023/1023.
| // as ws.route below): log, answer 500 if nothing went out yet, | ||
| // else drop the one affected socket. | ||
| const fail = (res, err) => { | ||
| log.error(`plugin ${id}: mounted app handler failed: ${err.message}`); |
There was a problem hiding this comment.
Confirmed and fixed in 92ffb28 — errMessage() normalizes non-Error throws (instanceof Error ? .message : String(err)); applied to fail() and also to both ws.route guards, which shipped with the same latent bug. Regression test covers a string throw and an undefined rejection, both answering 500 with the server still up. 1024/1024.
err.message on a thrown string or undefined made fail() itself throw inside the catch — recreating the unhandled rejection the guard exists to prevent. errMessage() normalizes whatever a plugin threw; applied to mountApp's fail() and both ws.route guards, which shipped with the same latent bug. Regression test: string throw and undefined rejection both answer 500 with the server still up. Full suite 1024/1024.
| Promise.resolve(handler(socket, request)).catch((err) => { | ||
| log.error(`plugin ${id}: ws handler failed: ${err.message}`); | ||
| log.error(`plugin ${id}: ws handler failed: ${errMessage(err)}`); | ||
| socket.terminate?.(); |
There was a problem hiding this comment.
Fixed in ceb9e21 — all three guard sites now log { err } per house style; stacks survive, non-Error throws still handled (this also retired the errMessage helper). 1024/1024.
| } catch (err) { | ||
| log.error(`plugin ${id}: ws handler failed: ${err.message}`); | ||
| log.error(`plugin ${id}: ws handler failed: ${errMessage(err)}`); | ||
| socket.terminate?.(); |
There was a problem hiding this comment.
Fixed in ceb9e21 — all three guard sites now log { err } per house style; stacks survive, non-Error throws still handled (this also retired the errMessage helper). 1024/1024.
| const fail = (res, err) => { | ||
| log.error(`plugin ${id}: mounted app handler failed: ${errMessage(err)}`); | ||
| if (!res.headersSent && !res.writableEnded) { |
There was a problem hiding this comment.
Fixed in ceb9e21 — all three guard sites now log { err } per house style; stacks survive, non-Error throws still handled (this also retired the errMessage helper). 1024/1024.
String interpolation dropped Error stack traces; { err } is the house
idiom (src/server.js:1127), pino serializes it with stack and console
prints it whole, and it handles non-Error throws without touching
.message — which also retires the errMessage() helper from the previous
round. All three guard sites (mountApp fail, both ws.route paths).
Suite 1024/1024; non-Error regression tests unchanged and passing.
Four plugin-API seams land, all surfaced by the out-of-tree plugin
experiment (github.com/JavaScriptSolidServer/plugins), each with a
consumer proving the need:
- api.mountApp(handler, { prefix }) (#583, #590): mount a wrapped
node-style (req, res) app — a reverse proxy or framework adapter —
under the plugin's prefix. Bundles the appPaths WAC exemption, a
scoped pass-through parser that hands the app an undrained body
stream, and reply.hijack(). The raw-body-mode answer to #583.
- api.serverInfo() -> { baseUrl, protocol, host, port, listening }
(#601, #605): a plugin learns the server's own origin instead of
repeating baseUrl/loopbackUrl in config, where a wrong value fails
quietly. A function, not a snapshot — with port 0 the real port
exists only once listening; an explicit idpIssuer wins as the
canonical baseUrl.
- api.reservePath(path, opts?) (#602, #607): claim and WAC-exempt a
route outside the plugin's prefix. Literal paths ('/xrpc') own their
subtree; parameterized paths ('/:user/did.json') match an exact
shape (read-only by default) for pinned documents inside pod
namespaces. Two plugins reserving the same path fail the boot with
both names — loud beats the silent-loser outcome.
- api.plugins -> [{ id, prefix, module }] (#610, #612): a read-only,
frozen boot-time roster of every loaded entry, so a plugin
describing the deployment (a status dashboard, an admin console)
enumerates its co-loaded siblings instead of being hand-fed a
duplicate of the operator's plugins array.
All documented in docs/configuration.md App Plugins. Remaining plugin
seams tracked upstream: api.events.onResourceChange (#603),
api.authorize (#604).
Closes #583. The second seam #206 forced, now that the loader (#589) has landed.
Problem
Fastify's content parsers consume the request stream before handlers run. A plugin that hands requests to an existing node-style
(req, res)handler — a wrapped HTTP app, reverse proxy, or framework adapter — then hangs forever: the wrapped app waits fordata/endon a stream Fastify already drained. The Tideholm and bridge adapters each hand-rolled the same scoped-parser +reply.hijack()incantation to work around it — exactly the folklore this issue predicted every plugin of this shape would rediscover.Solution
api.mountApp(handler, { prefix })bundles the four things every wrapped-app plugin needs together:{ prefix }is exempted too),reply.hijack()so Fastify releases the response,/*).Defaults to the entry's
prefix; pass{ prefix }to mount a second app elsewhere.Acceptance
test/plugin-mountapp.test.js, 4 testsFull suite: 1020/1020. Refs #206, #582, #589.