Skip to content

Commit b334fbd

Browse files
melvincarvalhomelvincarvalho
committed
review: plugins docs section, collision-resistant ids, guarded ws handlers
- docs/configuration.md gains an App Plugins section beside appPaths and getAgent: entry shape, the automatic appPaths exemption, the activate api, and the fail-loudly contract - pluginId derives from the full specifier for bare package imports (@scope1/pkg and @scope2/pkg no longer collide) but keeps the basename for file paths, where a machine-specific prefix must not name the data dir; duplicate ids across entries now fail the boot instead of sharing storage - ws.route wraps plugin handlers: a sync throw or rejected promise logs and terminates the one affected socket instead of surfacing as an unhandled rejection in the host
1 parent 10ec550 commit b334fbd

3 files changed

Lines changed: 118 additions & 3 deletions

File tree

‎docs/configuration.md‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,50 @@ for the design discussion and
389389
for a complete example (a multiplayer game where pod WebIDs are the player
390390
accounts).
391391

392+
## App Plugins (plugins)
393+
394+
The plugin loader
395+
([#206](https://github.com/JavaScriptSolidServer/JavaScriptSolidServer/issues/206))
396+
does the appPaths wiring for you: declare the apps in config and the server
397+
imports, mounts, and tears them down itself.
398+
399+
```js
400+
const fastify = createServer({
401+
root: './data',
402+
idp: true,
403+
plugins: [
404+
{ module: 'tideholm/jss-plugin/tideholm-jss.js', prefix: '/tideholm',
405+
config: { bots: 8 } },
406+
{ module: './my-app/plugin.js', prefix: '/myapp' },
407+
],
408+
});
409+
```
410+
411+
Each entry:
412+
413+
- `module` — import specifier: a package path (resolved from JSS's module
414+
graph) or a file path (`./…` or absolute, resolved from the process cwd).
415+
The module exports `activate(api)`, called during startup.
416+
- `prefix` — the app's mount point. Added to `appPaths` automatically, so
417+
the app owns authentication below it (see the section above). Must start
418+
with `/`; invalid prefixes fail startup.
419+
- `config` — passed to the plugin verbatim as `api.config`.
420+
- `id` — optional stable identifier (defaults to a name derived from
421+
`module`); names the plugin's private data dir, so set it explicitly if
422+
you load two plugins whose specifiers reduce to the same name.
423+
424+
`activate(api)` receives: `api.fastify` (register routes here),
425+
`api.prefix`, `api.config`, `api.log`, `api.auth.getAgent(request)`
426+
(identity, as above), `api.storage.pluginDir()` (a private server-side
427+
directory under the data root, never served over HTTP), and
428+
`api.ws.route(path, (socket, request) => {})` for WebSocket endpoints —
429+
routed through the same upgrade path as the built-in realtime features, so
430+
plugins never attach their own `'upgrade'` listener. Return
431+
`{ deactivate }` to run teardown (state saves, timers) on server close.
432+
433+
A plugin that fails to import or activate fails `listen()` loudly rather
434+
than booting a server silently missing an app.
435+
392436
## Storage Quotas
393437

394438
Limit storage per pod to prevent abuse and manage resources:

‎src/plugins.js‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,11 +67,24 @@ export function normalizePrefix(p) {
6767
return trimmed.startsWith('/') && trimmed.length > 1 ? trimmed : '';
6868
}
6969

70-
/** Directory-safe plugin id: from entry.id or derived from the module spec. */
70+
/**
71+
* Directory-safe plugin id: entry.id, or derived from the module spec.
72+
* Bare package specifiers keep their full path ('@scope/pkg/plugin.js' ->
73+
* 'scope-pkg-plugin') so same-named files in different packages don't
74+
* collide; file paths use the basename, because a machine-specific
75+
* directory prefix must not name the plugin's data dir (the id — and with
76+
* it pluginDir — would change whenever the deployment moves). The loader
77+
* additionally rejects duplicate ids, so any residual collision fails the
78+
* boot instead of silently sharing storage.
79+
*/
7180
export function pluginId(spec) {
81+
const module = String(spec.module);
7282
const raw = typeof spec.id === 'string' && spec.id
7383
? spec.id
74-
: path.basename(String(spec.module)).replace(/\.[cm]?js$/, '');
84+
: (module.startsWith('.') || path.isAbsolute(module)
85+
? path.basename(module)
86+
: module
87+
).replace(/\.[cm]?js$/, '');
7588
const id = raw.toLowerCase().replace(/[^a-z0-9_-]+/g, '-').replace(/^-+|-+$/g, '');
7689
if (!id) throw new Error(`plugins: cannot derive an id from ${JSON.stringify(spec.module)}; set entry.id`);
7790
return id;
@@ -88,12 +101,17 @@ export function pluginId(spec) {
88101
*/
89102
export async function loadPlugins(fastify, entries, ctx) {
90103
const log = makePluginLog(ctx.log);
104+
const seenIds = new Set();
91105
for (const entry of entries) {
92106
const spec = typeof entry === 'string' ? { module: entry } : entry;
93107
if (!spec || typeof spec.module !== 'string' || !spec.module) {
94108
throw new Error('plugins: each entry needs a module (import specifier or path)');
95109
}
96110
const id = pluginId(spec);
111+
if (seenIds.has(id)) {
112+
throw new Error(`plugins: duplicate id '${id}' — set entry.id to keep the plugins' data dirs apart`);
113+
}
114+
seenIds.add(id);
97115

98116
// Paths resolve from the operator's cwd; bare specifiers stay package
99117
// imports resolved from JSS's own module graph.
@@ -142,7 +160,18 @@ export async function loadPlugins(fastify, entries, ctx) {
142160
fastify.get(wsPath, { websocket: true }, (connection, request) => {
143161
// @fastify/websocket v8 hands a SocketStream; the ws socket is
144162
// .socket. Later majors hand the socket directly — accept both.
145-
handler(connection.socket ?? connection, request);
163+
const socket = connection.socket ?? connection;
164+
// A plugin bug here must not become an unhandled rejection that
165+
// takes the host down: log it and close the one affected socket.
166+
try {
167+
Promise.resolve(handler(socket, request)).catch((err) => {
168+
log.error(`plugin ${id}: ws handler failed: ${err.message}`);
169+
socket.terminate?.();
170+
});
171+
} catch (err) {
172+
log.error(`plugin ${id}: ws handler failed: ${err.message}`);
173+
socket.terminate?.();
174+
}
146175
});
147176
},
148177
},

‎test/plugins.test.js‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import path from 'path';
1717
import { WebSocket } from 'ws';
1818
import fs from 'fs-extra';
1919
import { createServer } from '../src/server.js';
20+
import { pluginId } from '../src/plugins.js';
2021

2122
const TEST_DATA_DIR = './test-data-plugins';
2223
const FIXTURE_DIR = './test-fixtures-plugins';
@@ -53,6 +54,9 @@ export async function activate(api) {
5354
await api.ws.route(api.prefix + '/ws', (socket) => {
5455
socket.on('message', (data) => socket.send('pong:' + String(data)));
5556
});
57+
await api.ws.route(api.prefix + '/ws-throw', () => {
58+
throw new Error('plugin bug');
59+
});
5660
5761
return {
5862
deactivate() {
@@ -187,6 +191,44 @@ describe('plugin loader (#206)', () => {
187191
);
188192
});
189193

194+
it('a throwing ws handler closes that socket but not the server', async () => {
195+
await startWith([
196+
{ module: `${FIXTURE_DIR}/fixture-plugin.js`, prefix: '/game' },
197+
]);
198+
const ws = new WebSocket(`${baseUrl.replace('http', 'ws')}/game/ws-throw`);
199+
await new Promise((resolve) => {
200+
ws.on('close', resolve);
201+
ws.on('error', resolve);
202+
});
203+
// The host survives its plugin's bug.
204+
const res = await fetch(`${baseUrl}/game/echo`);
205+
assert.strictEqual(res.status, 200);
206+
});
207+
208+
it('derives collision-resistant ids and rejects duplicates', async () => {
209+
// Bare specifiers keep their full path; file paths use the basename.
210+
assert.strictEqual(pluginId({ module: '@scope1/pkg/plugin.js' }), 'scope1-pkg-plugin');
211+
assert.strictEqual(pluginId({ module: '@scope2/pkg/plugin.js' }), 'scope2-pkg-plugin');
212+
assert.strictEqual(pluginId({ module: '/some/machine/path/foo.js' }), 'foo');
213+
assert.strictEqual(pluginId({ module: './x.js', id: 'Custom Id!' }), 'custom-id');
214+
215+
// Two entries reducing to the same id fail the boot, not share a dir.
216+
await fs.emptyDir(TEST_DATA_DIR);
217+
server = createServer({
218+
logger: false,
219+
forceCloseConnections: true,
220+
root: TEST_DATA_DIR,
221+
plugins: [
222+
{ module: `${FIXTURE_DIR}/fixture-plugin.js`, prefix: '/a' },
223+
{ module: `${FIXTURE_DIR}/fixture-plugin.js`, prefix: '/b' },
224+
],
225+
});
226+
await assert.rejects(
227+
server.listen({ port: 0, host: '127.0.0.1' }),
228+
/duplicate id/,
229+
);
230+
});
231+
190232
it('an invalid prefix fails listen() loudly', async () => {
191233
await fs.emptyDir(TEST_DATA_DIR);
192234
server = createServer({

0 commit comments

Comments
 (0)