openvibers/openvibe.reviews Commits

Move Reviews errors and shutdown onto openvibe-sdk/service (#3)

OP OpenVibers committed 2 Oct 2026 · bd447e9864 · 5 files, +150 −37

checks run by GitHub CI (test) · CI ✓ 3/3 · review SHIP by DeepSeek harness

https://github.com/OpenVibers/OpenVibe.Reviews/pull/3

Changes

ddiff --git a/package-lock.json b/package-lock.json
iindex 791784a..e1e5d59 100644
---- a/package-lock.json
++++ b/package-lock.json
@@@ -16,7 +16,7 @@
"iovalkey": "^0.4.0",
"openvibe-contracts": "https://codeload.github.com/OpenVibers/OpenVibe.Contracts/tar.gz/refs/tags/v0.76.0",
"openvibe-publishing": "https://codeload.github.com/OpenVibers/OpenVibe.Publishing/tar.gz/refs/tags/v1.1.0",
-- "openvibe-sdk": "https://codeload.github.com/OpenVibers/OpenVibe.SDK/tar.gz/refs/tags/v0.20.0",
++ "openvibe-sdk": "https://codeload.github.com/OpenVibers/OpenVibe.SDK/tar.gz/refs/tags/v0.26.0",
"openvibe-shared": "https://codeload.github.com/OpenVibers/OpenVibe.Shared/tar.gz/refs/tags/v2.2.0",
"pg": "^8.23.0"
},
@@@ -1167,9 +1167,9 @@
}
},
"node_modules/openvibe-sdk": {
-- "version": "0.20.0",
-- "resolved": "https://codeload.github.com/OpenVibers/OpenVibe.SDK/tar.gz/refs/tags/v0.20.0",
-- "integrity": "sha512-CqZFfVuSv3ZPoyG1LuWCAdvo8TG2ZuUCeGaqTsxDKS3gsymSFV2QXPq/5QUotTd82WShTGUKKhTgcyp9ggUMfQ==",
++ "version": "0.26.0",
++ "resolved": "https://codeload.github.com/OpenVibers/OpenVibe.SDK/tar.gz/refs/tags/v0.26.0",
++ "integrity": "sha512-+RK89ZG4WAKIRh89yr3ieIvnyJVNSOI+dsbBqGNcr9UHsJStdxtM75zSDq8cUNpBPQ1X6ElatZrjU9S3s0aEkg==",
"license": "MIT",
"engines": {
"node": ">=20"
ddiff --git a/package.json b/package.json
iindex dc4a8af..05aef40 100644
---- a/package.json
++++ b/package.json
@@@ -26,7 +26,7 @@
"iovalkey": "^0.4.0",
"openvibe-contracts": "https://codeload.github.com/OpenVibers/OpenVibe.Contracts/tar.gz/refs/tags/v0.76.0",
"openvibe-publishing": "https://codeload.github.com/OpenVibers/OpenVibe.Publishing/tar.gz/refs/tags/v1.1.0",
-- "openvibe-sdk": "https://codeload.github.com/OpenVibers/OpenVibe.SDK/tar.gz/refs/tags/v0.20.0",
++ "openvibe-sdk": "https://codeload.github.com/OpenVibers/OpenVibe.SDK/tar.gz/refs/tags/v0.26.0",
"openvibe-shared": "https://codeload.github.com/OpenVibers/OpenVibe.Shared/tar.gz/refs/tags/v2.2.0",
"pg": "^8.23.0"
},
ddiff --git a/server/http/common.js b/server/http/common.js
iindex d2e3757..028d6f9 100644
---- a/server/http/common.js
++++ b/server/http/common.js
@@@ -3,7 +3,7 @@
* Shared HTTP helpers: the actor middleware, capability guards and error mapping.
*/
const contracts = require('openvibe-contracts');
--const cache = require('openvibe-shared/cache-policy');
++const svc = require('openvibe-sdk/service');
const { checkCapability } = require('../auth/capabilities');
const { AuthError } = require('../auth/viewer');
@@@ -49,25 +49,19 @@ function crossSite(req, origin) {
return !!(o && o !== 'null' && o !== origin);
}
--/** Service errors → problem+json. */
++/**
++ * Service errors → problem+json (openvibe-sdk/service, plan T1): the kit's defaults are this module's —
++ * the error's status/code/detail, `extra` spread into the body, 'internal.error'/'Internal error' at 500,
++ * 'request.invalid' otherwise, and only 5xx (apart from 503) logged, under `[Reviews]`. Kept as a named export
++ * so no call site moves.
++ */
function sendError(res, req, err, log = console) {
-- const status = err && Number.isInteger(err.status) ? err.status : 500;
-- if (status >= 500 && status !== 503) log.error('[Reviews]', err && err.stack ? err.stack : err);
-- const code = (err && err.code) || (status === 500 ? 'internal.error' : 'request.invalid');
-- const detail = status === 500 ? 'Internal error' : (err && err.message) || undefined;
-- return http.sendProblem(res, status, code, { detail, ctx: req.ov, extra: err && err.extra ? err.extra : undefined });
++ return svc.sendError(res, req, err, log, { name: 'Reviews' });
}
++/** A JSON handler: its return value is the body, answers `private, no-store`, errors through sendError. */
function run(fn, status = 200, log = console) {
-- return async (req, res) => {
-- try {
-- const out = await fn(req, res);
-- if (res.headersSent) return;
-- res.status(typeof status === 'function' ? status(out) : status).set('Cache-Control', cache.htmlHeaders({ private: true })).json(out);
-- } catch (err) {
-- sendError(res, req, err, log);
-- }
-- };
++ return svc.run(fn, status, { name: 'Reviews', log, noStore: true });
}
module.exports = { actorMiddleware, guard, sendError, run, crossSite };
ddiff --git a/server/index.js b/server/index.js
iindex fb86502..9678346 100644
---- a/server/index.js
++++ b/server/index.js
@@@ -18,8 +18,30 @@ const { createKeyStore } = require('./auth/keys');
const { createViewerResolver } = require('./auth/viewer');
const { createApp } = require('./app');
const { createIndexNow } = require('openvibe-shared/indexnow');
++const { gracefulStop } = require('openvibe-sdk/service');
--async function start({ config, db: givenDb = null, now = () => Date.now(), fetchImpl = globalThis.fetch, tokens = null, publicKey = null, log = console, listen = true, workers = listen, rateLimits = true, limitsNow = null, indexnow: givenIndexnow = undefined } = {}) {
++/**
++ * The process stop (openvibe-sdk/service, plan T1): the job timers and the Events outbox relay stop taking new
++ * work, then the HTTP server drains (in-flight requests get 8 s), then the database closes — today's order. Past
++ * the 10 s deadline the process exits 0, as the hand-rolled timer did. `closeDb` is false when a test handed in
++ * its own database (start's `givenDb`), which stays open. `exit` and `signals` are injectable so a test can watch
++ * the stop and no test process installs signal handlers.
++ */
++function createLifecycle({ server, db, valkey = null, platform, timers = [], closeDb = true, exit, signals, log } = {}) {
++ return gracefulStop({
++ name: 'Reviews', server, log, drainMs: 8000, deadlineMs: 10000, deadlineExitCode: 0, exit, signals,
++ stop: [
++ () => timers.forEach((t) => { clearInterval(t); clearTimeout(t); }),
++ () => platform.outbox.stop(),
++ ],
++ close: [
++ () => { if (closeDb) return db.close(); },
++ () => { if (valkey) return valkey.close(); },
++ ],
++ });
++}
++
++async function start({ config, db: givenDb = null, now = () => Date.now(), fetchImpl = globalThis.fetch, tokens = null, publicKey = null, log = console, listen = true, workers = listen, rateLimits = true, limitsNow = null, indexnow: givenIndexnow = undefined, signals = false, exit = () => {} } = {}) {
config = config || load();
// PostgreSQL (ADR-035): opened and migrated here unless the caller (a test) hands in a migrated handle.
const db = givenDb || await openDb(config, { log });
@@@ -60,28 +82,19 @@ async function start({ config, db: givenDb = null, now = () => Date.now(), fetch
log.log(`[Reviews] ${config.nodeEnv} on http://${config.host}:${config.port} → ${config.baseUrl}`);
}
++ const lifecycle = createLifecycle({ server, db, platform, timers, closeDb: !givenDb, signals, exit, log });
++
async function stop() {
-- for (const t of timers) { clearInterval(t); clearTimeout(t); }
-- await platform.outbox.stop();
-- if (server) await new Promise((resolve) => server.close(resolve));
-- if (!givenDb) await db.close();
++ return lifecycle.stop('stop');
}
-- return { app, db, svc, sync, stores, platform, keys, viewers, indexnow, server, config, stop };
++ return { app, db, svc, sync, stores, platform, keys, viewers, indexnow, server, config, timers, lifecycle, stop };
}
--module.exports = { start };
++module.exports = { start, createLifecycle };
if (require.main === module) {
require('dotenv').config();
-- start().then((h) => {
-- const shutdown = (signal) => {
-- console.log(`[Reviews] ${signal} — closing`);
-- h.stop().finally(() => process.exit(0));
-- setTimeout(() => process.exit(0), 10000).unref();
-- };
-- process.on('SIGTERM', () => shutdown('SIGTERM'));
-- process.on('SIGINT', () => shutdown('SIGINT'));
-- }).catch((err) => {
++ start({ signals: true, exit: (code) => process.exit(code) }).catch((err) => {
console.error('[Reviews] failed to start:', err);
process.exit(1);
});
ddiff --git a/test/service-kit.test.js b/test/service-kit.test.js
nnew file mode 100644
iindex 0000000..3ea9992
---- /dev/null
++++ b/test/service-kit.test.js
@@@ -0,0 +1,106 @@
++'use strict';
++/**
++ * openvibe-sdk/service (plan T1): Reviews' error half is the kit's sendError/run with Reviews' options, so a
++ * refusal answers its own status/code/detail with `extra` spread into the body (never nested as `details`),
++ * a 503 keeps its detail, an unexpected throw answers 500 internal.error with 'Internal error', and a
++ * successful run answers `private, no-store`. The entry point's stop runs its stop and close steps in order
++ * (timers, outbox, then db) and exits 0; a database a test handed in is left open.
++ */
++const assert = require('assert');
++const http = require('http');
++const express = require('express');
++const { sendError, run } = require('../server/http/common');
++const { createLifecycle } = require('../server/index');
++
++const quiet = { log() {}, warn() {}, error() {} };
++const errors = [];
++const rec = { log() {}, warn() {}, error(...args) { errors.push(args); } };
++
++(async () => {
++ // A service-shaped app on Reviews' run/sendError, the way the API router wires them.
++ const app = express();
++ app.get('/ok', run(async () => ({ ok: true }), 200, quiet));
++ app.post('/refuse', run(async () => { throw Object.assign(new Error('A name is required'), { status: 422, code: 'reviews.name_required', extra: { field: 'name' } }); }, 200, quiet));
++ app.post('/gone', run(async () => { throw Object.assign(new Error('Sources is not answering right now'), { status: 503, code: 'sources.unavailable' }); }, 200, rec));
++ app.post('/boom', run(async () => { throw new Error('boom'); }, 200, rec));
++ app.get('/direct', (req, res) => sendError(res, req, Object.assign(new Error('nope'), { status: 422, code: 'reviews.direct', extra: { n: 1 } }), quiet));
++ const server = await new Promise((resolve) => { const s = app.listen(0, '127.0.0.1', () => resolve(s)); });
++ const base = `http://127.0.0.1:${server.address().port}`;
++ const post = (p) => fetch(base + p, { method: 'POST', headers: { 'content-type': 'application/json' }, body: '{}' });
++ const read = async (r) => { const text = await r.text(); return { status: r.status, headers: r.headers, json: JSON.parse(text) }; };
++
++ const ok = await read(await fetch(`${base}/ok`));
++ assert.strictEqual(ok.status, 200, JSON.stringify(ok.json));
++ assert.deepStrictEqual(ok.json, { ok: true });
++ assert.strictEqual(ok.headers.get('cache-control'), 'private, no-store');
++ assert.match(String(ok.headers.get('vary')), /Cookie/, 'the private policy varies on Cookie/Authorization');
++
++ const refused = await read(await post('/refuse'));
++ assert.strictEqual(refused.status, 422, JSON.stringify(refused.json));
++ assert.strictEqual(refused.json.code, 'reviews.name_required');
++ assert.strictEqual(refused.json.detail, 'A name is required');
++ assert.deepStrictEqual(refused.json.field, 'name', 'extra is spread into the body');
++ assert.strictEqual(refused.json.details, undefined, 'extra is not nested as { details }');
++
++ const gone = await read(await post('/gone'));
++ assert.strictEqual(gone.status, 503, JSON.stringify(gone.json));
++ assert.strictEqual(gone.json.code, 'sources.unavailable');
++ assert.strictEqual(gone.json.detail, 'Sources is not answering right now');
++
++ const boom = await read(await post('/boom'));
++ assert.strictEqual(boom.status, 500, JSON.stringify(boom.json));
++ assert.strictEqual(boom.json.code, 'internal.error');
++ assert.strictEqual(boom.json.detail, 'Internal error');
++ assert.strictEqual(boom.json.error, 'Internal error');
++ assert.strictEqual(boom.json.error, boom.json.detail);
++ // Only a 5xx that is not 503 is logged, under [Reviews]; the 422 and the 503 above were not.
++ assert.strictEqual(errors.length, 1, JSON.stringify(errors));
++ assert.strictEqual(errors[0][0], '[Reviews]');
++ assert.match(String(errors[0][1]), /^Error: boom/);
++
++ const direct = await read(await fetch(`${base}/direct`));
++ assert.strictEqual(direct.status, 422, JSON.stringify(direct.json));
++ assert.strictEqual(direct.json.code, 'reviews.direct');
++ assert.strictEqual(direct.json.n, 1, 'sendError spreads extra too');
++
++ server.close();
++
++ // The stop path: stop steps in order, then close steps, then exit 0.
++ const steps = [];
++ const stopServer = await new Promise((resolve) => { const s = http.createServer((_req, res) => res.end('ok')); s.listen(0, '127.0.0.1', () => resolve(s)); });
++ const timers = [setInterval(() => {}, 1000)];
++ const cleared = [];
++ const realClear = global.clearInterval;
++ global.clearInterval = (t) => { cleared.push(t); return realClear(t); };
++ const exits = [];
++ try {
++ const lifecycle = createLifecycle({
++ server: stopServer,
++ db: { close: async () => { steps.push('db.close'); } },
++ platform: { outbox: { stop: async () => { steps.push('outbox.stop'); } } },
++ timers, closeDb: true, exit: (code) => exits.push(code), signals: false, log: quiet,
++ });
++ assert.strictEqual(lifecycle.stopping(), false, 'not stopping before the signal');
++ const code = await lifecycle.stop('SIGTERM');
++ assert.strictEqual(lifecycle.stopping(), true, 'stopping after the signal');
++ assert.strictEqual(code, 0);
++ assert.deepStrictEqual(exits, [0], 'exits 0, not 1 (the deadlineExitCode is 0)');
++ assert.deepStrictEqual(steps, ['outbox.stop', 'db.close'], 'today\'s order');
++ assert.ok(cleared.includes(timers[0]), 'the job timer was cleared');
++ assert.strictEqual(stopServer.listening, false, 'the HTTP server stopped listening');
++ } finally {
++ global.clearInterval = realClear;
++ }
++
++ // A database handed in stays open (closeDb false) and an absent Valkey has no close step.
++ const steps2 = [];
++ const lifecycle2 = createLifecycle({
++ server: null, db: { close: async () => { steps2.push('db.close'); } }, valkey: null,
++ platform: { outbox: { stop: async () => { steps2.push('outbox.stop'); } } },
++ closeDb: false, exit: () => {}, signals: false, log: quiet,
++ });
++ assert.strictEqual(await lifecycle2.stop(), 0);
++ assert.deepStrictEqual(steps2, ['outbox.stop']);
++
++ console.log('service-kit ok');
++})().catch((err) => { console.error(err); process.exit(1); });