[glm-grade=A] fix(caddy-upstreams): swap errorResponse arg order to statusCode-first; add type validator (DC-062)
Routes/caddy-upstreams.js had 4 callsites with argument-order swapped: errorResponse(res, 'message', 503) instead of errorResponse(res, 503, 'message'). The canonical signature from src/utils/responses.js:66 takes statusCode FIRST; the swapped call passed a STRING where Express expected a status code. res.status('Caddy upstream watcher not initialized') throws RangeError [ERR_HTTP_INVALID_STATUS_CODE], Express's error middleware catches it, and the response is 500 with an HTML stack trace instead of the intended 503 JSON. Four `!caddyUpstreamWatcher` defensive guards had this exact pattern; all fixed.
Defense-in-depth (responses.js): errorResponse() now validates that statusCode is an integer in 100..599 and that message is a string BEFORE calling res.status(). Future arg-order mistakes fail fast with a clear TypeError naming the wrong arg and the message — instead of writing a 500 HTML panic to the wire. Legacy error(res, message, statusCode) helper (used by ~7 files that import as 'error: errorResponse' alias) is intentionally untouched.
Tests (__tests__/utils-responses-dc-062.test.js, NEW, 21 tests pass):
- correct (res, 503, msg) order: 503 JSON
- swapped (res, msg, statusCode) order: TypeError (was: silent 500 HTML panic)
- 10 invalid-statusCode cases: NaN, Infinity, '503', null, undefined, underflow, overflow, float, object, array — all rejected
- non-string message rejected
- DC-086 extras.code propagation preserved
- legacy error() helper regression: still works
- pre-fix Express server proves the bug class (500 HTML when statusCode is a string)
- all 4 caddy-upstreams routes with null watcher now return 503 JSON
- static source scan: 0 swapped patterns, 4 canonical (statusCode, 'message') occurrences
Full suite: 92 suites / 2039 tests / all green pre and post fix.
[glm-grade=A] from deleg_45e44614 (3 tool calls, 82s, MiniMax-M3 stand-in per Sami's 2026-08-17 authorization)
This commit is contained in:
@@ -62,8 +62,34 @@ function noContent(res) {
|
||||
*
|
||||
* DC-086: If extras.code is set, it's treated as a machine-readable error code
|
||||
* (e.g. 'DC-CONT-002'). If message looks like a DC code, it's auto-extracted.
|
||||
*
|
||||
* DC-062: Validate that `statusCode` is a valid HTTP status (integer in
|
||||
* 100..599) BEFORE calling res.status(). Without this guard, a caller who
|
||||
* passes (res, message, statusCode) instead of (res, statusCode, message)
|
||||
* ends up with res.status(<string>), which throws
|
||||
* RangeError [ERR_HTTP_INVALID_STATUS_CODE] — Express catches that and
|
||||
* writes a 500 with an HTML stack trace to the client, which is the worst
|
||||
* possible failure mode (looks like a server crash, breaks CSRF and
|
||||
* content-type expectations, leaks the stack). Failing fast with a clear
|
||||
* TypeError names the call site early in the request lifecycle.
|
||||
*/
|
||||
function errorResponse(res, statusCode, message, extras = {}) {
|
||||
if (
|
||||
typeof statusCode !== 'number'
|
||||
|| !Number.isFinite(statusCode)
|
||||
|| !Number.isInteger(statusCode)
|
||||
|| statusCode < 100
|
||||
|| statusCode > 599
|
||||
) {
|
||||
throw new TypeError(
|
||||
`errorResponse(res, statusCode, message, extras): statusCode must be an integer HTTP status (100..599); received ${JSON.stringify(statusCode)} (message=${JSON.stringify(message)})`
|
||||
);
|
||||
}
|
||||
if (typeof message !== 'string') {
|
||||
throw new TypeError(
|
||||
`errorResponse(res, statusCode, message, extras): message must be a string; received ${typeof message} ${JSON.stringify(message)}`
|
||||
);
|
||||
}
|
||||
const body = { success: false, error: message, ...extras };
|
||||
// DC-086: surface machine-readable code at top level for client handling
|
||||
if (extras.code) {
|
||||
|
||||
Reference in New Issue
Block a user