Skip to content

Commit 58797b3

Browse files
melvincarvalhomelvincarvalho
authored
feat(plugins): api.mountApp for wrapped node-style apps (#583) (#590)
* feat(plugins): api.mountApp for wrapped node-style apps (#583) 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. * review: validate mountApp secondary prefix; guard handler after hijack - 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. * review: failure guards must not throw on non-Error throws 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. * review: log { err } in failure guards so stacks survive 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.
1 parent 3cb57c9 commit 58797b3

3 files changed

Lines changed: 242 additions & 2 deletions

File tree

‎docs/configuration.md‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,25 @@ routed through the same upgrade path as the built-in realtime features, so
430430
plugins never attach their own `'upgrade'` listener. Return
431431
`{ deactivate }` to run teardown (state saves, timers) on server close.
432432

433+
To mount an **existing node-style app** — a `(req, res)` handler, a reverse
434+
proxy, or a framework adapter — use `api.mountApp(handler, { prefix })`:
435+
436+
```js
437+
export async function activate(api) {
438+
const app = createMyNodeApp();
439+
await api.mountApp((req, res) => app.handle(req, res));
440+
}
441+
```
442+
443+
`mountApp` bundles the four things a wrapped-app plugin needs and would
444+
otherwise rediscover: the appPaths WAC exemption, a **scoped pass-through
445+
content parser** (so the wrapped app receives an unconsumed body stream
446+
instead of one Fastify already drained — the failure that hangs any
447+
body-reading app), `reply.hijack()` so Fastify releases the response, and
448+
registration on both the bare prefix and its subtree. It defaults to the
449+
entry's `prefix`; pass `{ prefix }` to mount a second app elsewhere (that
450+
prefix is WAC-exempted too).
451+
433452
A plugin that fails to import or activate fails `listen()` loudly rather
434453
than booting a server silently missing an app.
435454

‎src/plugins.js‎

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,62 @@ export async function loadPlugins(fastify, entries, ctx) {
152152
return dir;
153153
},
154154
},
155+
// Mount a node-style (req, res) handler — a wrapped HTTP app, reverse
156+
// proxy, or framework adapter — under the plugin's prefix (#583). This
157+
// bundles the four things every such plugin needs and otherwise
158+
// rediscovers: the appPaths WAC exemption (already applied above), a
159+
// scoped pass-through content parser so the wrapped app receives an
160+
// unconsumed body stream, reply.hijack() so Fastify releases the
161+
// response, and registration on both the bare prefix and its subtree.
162+
// Without the scoped parser, Fastify drains the request stream before
163+
// the handler runs and any body-reading app hangs forever.
164+
async mountApp(handler, opts = {}) {
165+
if (typeof handler !== 'function') {
166+
throw new Error(`plugin ${id}: mountApp(handler) needs a (req, res) function`);
167+
}
168+
// Any provided prefix must validate — same rule as entry.prefix: a
169+
// falsy normalization silently falling back to the entry prefix
170+
// would mount the app (and WAC-exempt it) somewhere unexpected.
171+
const secondary = normalizePrefix(opts.prefix);
172+
if (opts.prefix !== undefined && !secondary) {
173+
throw new Error(`plugin ${id}: mountApp invalid prefix ${JSON.stringify(opts.prefix)} (must start with '/'; omit to use the entry prefix)`);
174+
}
175+
const mountPrefix = secondary || prefix;
176+
if (!mountPrefix) {
177+
throw new Error(`plugin ${id}: mountApp needs a prefix (entry.prefix or opts.prefix)`);
178+
}
179+
if (mountPrefix !== prefix && !ctx.appPaths.includes(mountPrefix)) {
180+
ctx.appPaths.push(mountPrefix); // exempt a secondary mount too
181+
}
182+
await fastify.register(async (scope) => {
183+
scope.removeAllContentTypeParsers();
184+
scope.addContentTypeParser('*', (req, payload, done) => done(null, payload));
185+
// After hijack() Fastify sends nothing, so a handler bug must not
186+
// hang the client or become an unhandled rejection (same contract
187+
// as ws.route below): log, answer 500 if nothing went out yet,
188+
// else drop the one affected socket.
189+
const fail = (res, err) => {
190+
log.error({ err }, `plugin ${id}: mounted app handler failed`);
191+
if (!res.headersSent && !res.writableEnded) {
192+
res.statusCode = 500;
193+
res.end();
194+
} else {
195+
res.destroy();
196+
}
197+
};
198+
const wrapped = (request, reply) => {
199+
reply.hijack();
200+
try {
201+
Promise.resolve(handler(request.raw, reply.raw))
202+
.catch((err) => fail(reply.raw, err));
203+
} catch (err) {
204+
fail(reply.raw, err);
205+
}
206+
};
207+
scope.all(mountPrefix, wrapped);
208+
scope.all(mountPrefix + '/*', wrapped);
209+
});
210+
},
155211
ws: {
156212
async route(wsPath, handler) {
157213
if (typeof wsPath !== 'string' || !wsPath.startsWith('/')) {
@@ -168,11 +224,11 @@ export async function loadPlugins(fastify, entries, ctx) {
168224
// takes the host down: log it and close the one affected socket.
169225
try {
170226
Promise.resolve(handler(socket, request)).catch((err) => {
171-
log.error(`plugin ${id}: ws handler failed: ${err.message}`);
227+
log.error({ err }, `plugin ${id}: ws handler failed`);
172228
socket.terminate?.();
173229
});
174230
} catch (err) {
175-
log.error(`plugin ${id}: ws handler failed: ${err.message}`);
231+
log.error({ err }, `plugin ${id}: ws handler failed`);
176232
socket.terminate?.();
177233
}
178234
});

‎test/plugin-mountapp.test.js‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
/**
2+
* api.mountApp (#583) — a plugin mounting a node-style (req, res) handler
3+
* receives unconsumed request bodies, the exact shape the Tideholm and
4+
* bridge adapters hand-rolled. Verifies the scoped pass-through parser lets
5+
* a body-reading app work, host parsing stays intact outside the mount, and
6+
* a secondary mount prefix is WAC-exempted too.
7+
*/
8+
9+
import { describe, it, before, after, afterEach } from 'node:test';
10+
import assert from 'node:assert';
11+
import fs from 'fs-extra';
12+
import path from 'path';
13+
import os from 'os';
14+
15+
const TEST_DATA_DIR = './test-data-mountapp';
16+
const FIXTURE_DIR = path.join(os.tmpdir(), 'jss-mountapp-fixture');
17+
18+
// A plugin that mounts a plain node handler which reads the whole body and
19+
// echoes it back — the app that hangs if Fastify drained the stream first.
20+
const FIXTURE = `
21+
export async function activate(api) {
22+
await api.mountApp((req, res) => {
23+
let body = '';
24+
req.on('data', (c) => { body += c; });
25+
req.on('end', () => {
26+
res.writeHead(200, { 'content-type': 'application/json' });
27+
res.end(JSON.stringify({ echoed: body, method: req.method, url: req.url }));
28+
});
29+
});
30+
// A second mount under a different prefix, to prove secondary exemption.
31+
await api.mountApp((req, res) => {
32+
res.writeHead(200, { 'content-type': 'text/plain' });
33+
res.end('secondary');
34+
}, { prefix: '/wrapped2' });
35+
// Handlers that fail — sync throw and async rejection — to prove a
36+
// handler bug after hijack() answers 500 instead of hanging the client.
37+
await api.mountApp(() => { throw new Error('sync boom'); }, { prefix: '/boom' });
38+
await api.mountApp(async () => { throw new Error('async boom'); }, { prefix: '/boom-async' });
39+
// Non-Error throw: the failure guard itself must not throw on err.message.
40+
await api.mountApp(() => { throw 'string boom'; }, { prefix: '/boom-raw' });
41+
await api.mountApp(async () => Promise.reject(undefined), { prefix: '/boom-undef' });
42+
}
43+
`;
44+
45+
// A plugin whose secondary mount prefix is invalid — must fail the boot.
46+
const BAD_PREFIX_FIXTURE = `
47+
export async function activate(api) {
48+
await api.mountApp((req, res) => res.end('x'), { prefix: 'chat' });
49+
}
50+
`;
51+
52+
let server;
53+
let baseUrl;
54+
let originalDataRoot;
55+
56+
async function start() {
57+
await fs.emptyDir(TEST_DATA_DIR);
58+
const { createServer } = await import('../src/server.js');
59+
server = createServer({
60+
logger: false,
61+
forceCloseConnections: true,
62+
root: TEST_DATA_DIR,
63+
plugins: [
64+
{ id: 'wrapped', module: path.join(FIXTURE_DIR, 'plugin.js'), prefix: '/wrapped' },
65+
],
66+
});
67+
await server.listen({ port: 0, host: '127.0.0.1' });
68+
baseUrl = `http://127.0.0.1:${server.server.address().port}`;
69+
}
70+
71+
describe('api.mountApp (#583)', () => {
72+
before(async () => {
73+
originalDataRoot = process.env.DATA_ROOT;
74+
await fs.emptyDir(FIXTURE_DIR);
75+
await fs.writeFile(path.join(FIXTURE_DIR, 'plugin.js'), FIXTURE);
76+
});
77+
after(async () => {
78+
await fs.remove(FIXTURE_DIR);
79+
if (originalDataRoot === undefined) delete process.env.DATA_ROOT;
80+
else process.env.DATA_ROOT = originalDataRoot;
81+
});
82+
afterEach(async () => {
83+
if (server) { await server.close(); server = null; }
84+
await fs.remove(TEST_DATA_DIR);
85+
});
86+
87+
it('a JSON POST body round-trips through the wrapped node handler', async () => {
88+
await start();
89+
const res = await fetch(`${baseUrl}/wrapped/api/thing`, {
90+
method: 'POST',
91+
headers: { 'content-type': 'application/json' },
92+
body: JSON.stringify({ hello: 'world' }),
93+
});
94+
assert.strictEqual(res.status, 200);
95+
const body = await res.json();
96+
assert.strictEqual(body.echoed, JSON.stringify({ hello: 'world' }));
97+
assert.strictEqual(body.method, 'POST');
98+
});
99+
100+
it('serves the bare prefix and the subtree, unauthenticated (WAC-exempt)', async () => {
101+
await start();
102+
const bare = await fetch(`${baseUrl}/wrapped`, { method: 'GET' });
103+
assert.strictEqual(bare.status, 200);
104+
const deep = await fetch(`${baseUrl}/wrapped/a/b/c`, { method: 'GET' });
105+
assert.strictEqual(deep.status, 200);
106+
});
107+
108+
it('a secondary mount prefix is also served and WAC-exempt', async () => {
109+
await start();
110+
const res = await fetch(`${baseUrl}/wrapped2/anything`);
111+
assert.strictEqual(res.status, 200);
112+
assert.strictEqual(await res.text(), 'secondary');
113+
});
114+
115+
it('host body parsing is unaffected outside the mount (LDP PUT still WAC-guarded)', async () => {
116+
await start();
117+
const res = await fetch(`${baseUrl}/somepod/private/x`, { method: 'PUT', body: 'data' });
118+
assert.ok([401, 403].includes(res.status), `expected WAC rejection, got ${res.status}`);
119+
});
120+
121+
it('a handler that throws sync answers 500 and the server survives', async () => {
122+
await start();
123+
const res = await fetch(`${baseUrl}/boom`);
124+
assert.strictEqual(res.status, 500);
125+
// Process and server still alive: a healthy mount keeps answering.
126+
const ok = await fetch(`${baseUrl}/wrapped2/still-up`);
127+
assert.strictEqual(ok.status, 200);
128+
});
129+
130+
it('a handler that rejects async answers 500 instead of leaking the rejection', async () => {
131+
await start();
132+
const res = await fetch(`${baseUrl}/boom-async`);
133+
assert.strictEqual(res.status, 500);
134+
const ok = await fetch(`${baseUrl}/wrapped2/still-up`);
135+
assert.strictEqual(ok.status, 200);
136+
});
137+
138+
it('a handler that throws a non-Error still answers 500 (guard must not throw on err.message)', async () => {
139+
await start();
140+
const raw = await fetch(`${baseUrl}/boom-raw`);
141+
assert.strictEqual(raw.status, 500);
142+
const undef = await fetch(`${baseUrl}/boom-undef`);
143+
assert.strictEqual(undef.status, 500);
144+
const ok = await fetch(`${baseUrl}/wrapped2/still-up`);
145+
assert.strictEqual(ok.status, 200);
146+
});
147+
148+
it('a provided-but-invalid secondary prefix fails the boot instead of mounting at the entry prefix', async () => {
149+
await fs.writeFile(path.join(FIXTURE_DIR, 'bad-prefix.js'), BAD_PREFIX_FIXTURE);
150+
await fs.emptyDir(TEST_DATA_DIR);
151+
const { createServer } = await import('../src/server.js');
152+
server = createServer({
153+
logger: false,
154+
forceCloseConnections: true,
155+
root: TEST_DATA_DIR,
156+
plugins: [
157+
{ id: 'bad', module: path.join(FIXTURE_DIR, 'bad-prefix.js'), prefix: '/ok' },
158+
],
159+
});
160+
await assert.rejects(
161+
server.listen({ port: 0, host: '127.0.0.1' }),
162+
/invalid prefix "chat"/,
163+
);
164+
});
165+
});

0 commit comments

Comments
 (0)