Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { dbPlugin } from './db/index.js';
import { webrtcPlugin } from './webrtc/index.js';
import { tunnelPlugin } from './tunnel/index.js';
import { terminalPlugin } from './terminal/index.js';
import { registerErrorHandler } from './utils/error-handler.js';

const __dirname = dirname(fileURLToPath(import.meta.url));

Expand Down Expand Up @@ -154,6 +155,7 @@ export function createServer(options = {}) {
}

const fastify = Fastify(fastifyOptions);
registerErrorHandler(fastify);

// Add raw body parser for all content types
fastify.addContentTypeParser('*', { parseAs: 'buffer' }, (req, body, done) => {
Expand Down
24 changes: 24 additions & 0 deletions src/utils/error-handler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* Top-level Fastify error handler that logs the full stack for any 5xx.
*
* Without this, 500 responses carry only Fastify's default 91-byte body and
* leave no trace in logs — production debugging has to infer the exception
* from the response body alone (see #309 investigation for why that's bad).
*
* 4xx errors carry their own statusCode and don't need stack logging — they
* are expected client errors with self-explanatory messages.
*/
export function registerErrorHandler(fastify) {
fastify.setErrorHandler(function (err, request, reply) {
const statusCode = err.statusCode ?? 500;
if (statusCode >= 500) {
request.log.error({
err,
method: request.method,
url: request.url,
hostname: request.hostname
}, 'Unhandled 5xx error');
}
reply.send(err);
});
}
83 changes: 83 additions & 0 deletions test/error-handler.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/**
* Tests for the top-level error handler (#312).
*
* Verifies that:
* 1. Unhandled exceptions from route handlers produce a 500 whose body
* matches Fastify's default shape — the existing 91-byte response
* format must not regress (that's the baseline consumers rely on).
* 2. The error's stack is actually logged (with method/url/hostname
* context), so production debugging has a real trace.
* 3. 4xx errors are NOT stack-logged (they're expected client errors).
*
* Uses a minimal Fastify instance so the test exercises only the handler,
* not the full server's auth/routing stack.
*/

import { describe, it, before, after } from 'node:test';
import assert from 'node:assert';
import Fastify from 'fastify';
import { registerErrorHandler } from '../src/utils/error-handler.js';

class LogCapture {
constructor() { this.lines = []; }
write(chunk) {
try { this.lines.push(JSON.parse(chunk)); } catch { /* non-JSON */ }
return true;
}
errorLines() {
return this.lines.filter((l) => l.level >= 50);
}
clear() { this.lines.length = 0; }
}

describe('registerErrorHandler (#312)', () => {
let app;
const capture = new LogCapture();

before(async () => {
app = Fastify({
logger: { level: 'error', stream: capture },
disableRequestLogging: true
});
registerErrorHandler(app);
app.get('/throw500', async () => { throw new Error('synthetic 5xx for test'); });
app.get('/throw400', async () => {
const err = new Error('bad client input');
err.statusCode = 400;
throw err;
});
});

after(async () => { await app.close(); });

it('500 response body matches Fastify default shape (no regression)', async () => {
capture.clear();
const res = await app.inject({ method: 'GET', url: '/throw500' });
assert.strictEqual(res.statusCode, 500);
assert.deepStrictEqual(res.json(), {
statusCode: 500,
error: 'Internal Server Error',
message: 'synthetic 5xx for test'
});
});

it('500 logs include the stack and request context', async () => {
capture.clear();
await app.inject({ method: 'GET', url: '/throw500', headers: { host: 'example.test' } });
const line = capture.errorLines().find((l) => l.msg === 'Unhandled 5xx error');
assert.ok(line, `expected 'Unhandled 5xx error' log line, got: ${JSON.stringify(capture.errorLines())}`);
assert.strictEqual(line.method, 'GET');
assert.strictEqual(line.url, '/throw500');
assert.strictEqual(line.hostname, 'example.test');
assert.ok(line.err && line.err.stack, 'expected err.stack in log');
assert.ok(line.err.stack.includes('synthetic 5xx'), 'stack should identify the error');
});

it('4xx errors do not trigger the 5xx stack log', async () => {
capture.clear();
const res = await app.inject({ method: 'GET', url: '/throw400' });
assert.strictEqual(res.statusCode, 400);
const unhandled = capture.errorLines().filter((l) => l.msg === 'Unhandled 5xx error');
assert.strictEqual(unhandled.length, 0, '4xx must not produce a 5xx stack log');
});
});