From 2d394d882d7f9d0782adcb8add9aaecd2c1aaf4c Mon Sep 17 00:00:00 2001 From: Hermes Date: Wed, 10 Jun 2026 21:52:33 -0700 Subject: [PATCH] Standardize response shapes and fix dead fetchT timeout keys Three small cleanups for v1.14.0: 1. /caddy/cas now uses standard success envelope Was: { status: 'success', data: { cas: caList } } Now: { success: true, cas: caList } Updated frontend service-infrastructure.js to match. 2. /api/health/ca now uses standard envelope + meaningful HTTP codes Was: { status, message, daysUntilExpiration } with 200 on every error Now: { success, caStatus, message|error, daysUntilExpiration } with 200 / 404 / 500 as appropriate caStatus field preserves the original 'healthy'/'warning'/'critical'/'error' semantic so any future consumer of the CA-health state still has it. Tests updated to match. 3. Dead timeout: keys in fetchT opts are now a warning, not a silent strip src/utils/http.js:41 used to do without telling anyone. Callers that wrote fetchT(url, { timeout: 5000 }) got the default 5s timeout with no indication that their explicit value was ignored. Now it logs a warning naming the call site, then strips the key. Fixed 4 call sites that had stale timeout: keys: - src/context/caddy.js - src/context/dns.js - src/context/provider-dns.js - routes/dns.js (2 places) --- .../__tests__/routes/health.routes.test.js | 22 ++++++------- dashcaddy-api/package.json | 2 +- dashcaddy-api/routes/dns.js | 7 ++--- dashcaddy-api/routes/health.js | 31 ++++++++++--------- dashcaddy-api/routes/sites.js | 3 +- dashcaddy-api/src/context/caddy.js | 5 ++- dashcaddy-api/src/context/dns.js | 6 ++-- dashcaddy-api/src/context/provider-dns.js | 3 +- dashcaddy-api/src/utils/http.js | 10 +++++- status/js/core/service-infrastructure.js | 6 ++-- 10 files changed, 53 insertions(+), 42 deletions(-) diff --git a/dashcaddy-api/__tests__/routes/health.routes.test.js b/dashcaddy-api/__tests__/routes/health.routes.test.js index 558e381..a059da6 100644 --- a/dashcaddy-api/__tests__/routes/health.routes.test.js +++ b/dashcaddy-api/__tests__/routes/health.routes.test.js @@ -538,7 +538,7 @@ describe('Health Routes', () => { const { app } = createApp(); const res = await request(app).get('/api/health/ca'); expect(res.status).toBe(200); - expect(res.body.status).toBe('healthy'); + expect(res.body.caStatus).toBe('healthy'); expect(res.body.daysUntilExpiration).toBeGreaterThan(90); }); @@ -551,7 +551,7 @@ describe('Health Routes', () => { const { app } = createApp(); const res = await request(app).get('/api/health/ca'); expect(res.status).toBe(200); - expect(res.body.status).toBe('warning'); + expect(res.body.caStatus).toBe('warning'); expect(res.body.daysUntilExpiration).toBeLessThan(90); expect(res.body.daysUntilExpiration).toBeGreaterThanOrEqual(30); }); @@ -565,7 +565,7 @@ describe('Health Routes', () => { const { app } = createApp(); const res = await request(app).get('/api/health/ca'); expect(res.status).toBe(200); - expect(res.body.status).toBe('critical'); + expect(res.body.caStatus).toBe('critical'); expect(res.body.daysUntilExpiration).toBeLessThan(30); expect(res.body.daysUntilExpiration).toBeGreaterThanOrEqual(0); }); @@ -579,7 +579,7 @@ describe('Health Routes', () => { const { app } = createApp(); const res = await request(app).get('/api/health/ca'); expect(res.status).toBe(200); - expect(res.body.status).toBe('critical'); + expect(res.body.caStatus).toBe('critical'); expect(res.body.daysUntilExpiration).toBeLessThan(7); }); @@ -592,7 +592,7 @@ describe('Health Routes', () => { const { app } = createApp(); const res = await request(app).get('/api/health/ca'); expect(res.status).toBe(200); - expect(res.body.status).toBe('critical'); + expect(res.body.caStatus).toBe('critical'); expect(res.body.daysUntilExpiration).toBeLessThan(0); expect(res.body.message).toMatch(/EXPIRED/); }); @@ -601,9 +601,9 @@ describe('Health Routes', () => { exists.mockResolvedValue(false); const { app } = createApp(); const res = await request(app).get('/api/health/ca'); - expect(res.status).toBe(200); - expect(res.body.status).toBe('error'); - expect(res.body.message).toMatch(/not found/); + expect(res.status).toBe(404); + expect(res.body.caStatus).toBe('error'); + expect(res.body.error).toMatch(/not found/); expect(res.body.daysUntilExpiration).toBeNull(); }); @@ -612,9 +612,9 @@ describe('Health Routes', () => { execSync.mockImplementation(() => { throw new Error('openssl not found'); }); const { app } = createApp(); const res = await request(app).get('/api/health/ca'); - expect(res.status).toBe(200); - expect(res.body.status).toBe('error'); - expect(res.body.message).toBe('openssl not found'); + expect(res.status).toBe(500); + expect(res.body.caStatus).toBe('error'); + expect(res.body.error).toBe('openssl not found'); expect(res.body.daysUntilExpiration).toBeNull(); }); }); diff --git a/dashcaddy-api/package.json b/dashcaddy-api/package.json index 1db5bf8..4e992aa 100644 --- a/dashcaddy-api/package.json +++ b/dashcaddy-api/package.json @@ -1,6 +1,6 @@ { "name": "dashcaddy-api", - "version": "1.13.2", + "version": "1.13.3", "description": "DashCaddy API server - Dashboard backend for Docker, Caddy & DNS management", "main": "server.js", "scripts": { diff --git a/dashcaddy-api/routes/dns.js b/dashcaddy-api/routes/dns.js index fe4512b..b6f6f8a 100644 --- a/dashcaddy-api/routes/dns.js +++ b/dashcaddy-api/routes/dns.js @@ -383,9 +383,8 @@ module.exports = function({ const response = await fetchT(technitiumUrl, { method: 'GET', - headers: { 'Accept': 'text/plain' }, - timeout: 10000 - }); + headers: { 'Accept': 'text/plain' } + }, 10000); if (!response.ok) { const errorText = await response.text(); @@ -640,7 +639,7 @@ module.exports = function({ const dnsPort = siteConfig.dnsServerPort || '5380'; try { const url = `http://${serverInfo.ip}:${dnsPort}/api/admin/restart?token=${encodeURIComponent(tokenResult.token)}`; - const response = await fetchT(url, { method: 'POST', timeout: 5000 }); + const response = await fetchT(url, { method: 'POST' }, 5000); const result = await response.json(); if (result.status === 'ok') { success(res, { message: 'Restart initiated' }); diff --git a/dashcaddy-api/routes/health.js b/dashcaddy-api/routes/health.js index c59ac48..b57f8e2 100644 --- a/dashcaddy-api/routes/health.js +++ b/dashcaddy-api/routes/health.js @@ -273,9 +273,10 @@ module.exports = function({ try { // Check if certificate exists if (!await exists(rootCertPath)) { - return res.json({ - status: 'error', - message: 'Root CA certificate not found', + return res.status(404).json({ + success: false, + error: 'Root CA certificate not found', + caStatus: 'error', daysUntilExpiration: null }); } @@ -286,34 +287,36 @@ module.exports = function({ const daysUntilExpiration = Math.floor((expirationDate - new Date()) / (1000 * 60 * 60 * 24)); // Alert thresholds - let status = 'healthy'; + let caStatus = 'healthy'; let message = `CA certificate valid for ${daysUntilExpiration} days`; if (daysUntilExpiration < 0) { - status = 'critical'; + caStatus = 'critical'; message = `CA certificate EXPIRED ${Math.abs(daysUntilExpiration)} days ago!`; } else if (daysUntilExpiration < 7) { - status = 'critical'; + caStatus = 'critical'; message = `CA certificate expires in ${daysUntilExpiration} days!`; } else if (daysUntilExpiration < 30) { - status = 'critical'; + caStatus = 'critical'; message = `CA certificate expires in ${daysUntilExpiration} days!`; } else if (daysUntilExpiration < 90) { - status = 'warning'; + caStatus = 'warning'; message = `CA certificate expires in ${daysUntilExpiration} days`; } res.json({ - status: status, - message: message, - daysUntilExpiration: daysUntilExpiration, + success: true, + caStatus, + message, + daysUntilExpiration, expiresAt: notAfter }); } catch (error) { await logError('GET /api/health/ca', error); - res.json({ - status: 'error', - message: error.message, + res.status(500).json({ + success: false, + error: error.message, + caStatus: 'error', daysUntilExpiration: null }); } diff --git a/dashcaddy-api/routes/sites.js b/dashcaddy-api/routes/sites.js index e66eceb..c7e2e49 100644 --- a/dashcaddy-api/routes/sites.js +++ b/dashcaddy-api/routes/sites.js @@ -3,6 +3,7 @@ const fs = require('fs'); const { CADDY, REGEX, LIMITS } = require('../constants'); const { ValidationError, ConflictError, NotFoundError } = require('../errors'); const { validateURL } = require('../input-validator'); +const { ok } = require('../src/utils/responses'); /** * Sites route factory @@ -127,7 +128,7 @@ module.exports = function({ asyncHandler, caddy, dns, fetchT, buildDomain, addSe name: ca.name, displayName: ca.name !== (ca.id || ca.name) ? `${ca.name} (${ca.id || ca.name})` : ca.name })); - res.json({ status: 'success', data: { cas: caList } }); + ok(res, { cas: caList }); }, 'caddy-get-cas')); // Remove a site from Caddyfile diff --git a/dashcaddy-api/src/context/caddy.js b/dashcaddy-api/src/context/caddy.js index 04837ff..d64b24c 100644 --- a/dashcaddy-api/src/context/caddy.js +++ b/dashcaddy-api/src/context/caddy.js @@ -93,9 +93,8 @@ async function verifySiteAccessible(domain, fetchT, httpsAgent, log, maxAttempts try { const response = await fetchT(`https://${domain}/`, { method: 'HEAD', - agent: httpsAgent, - timeout: 5000 - }); + agent: httpsAgent + }, 5000); log.info('caddy', 'Site is accessible', { domain, status: response.status }); return true; diff --git a/dashcaddy-api/src/context/dns.js b/dashcaddy-api/src/context/dns.js index c91988a..5446b56 100644 --- a/dashcaddy-api/src/context/dns.js +++ b/dashcaddy-api/src/context/dns.js @@ -58,9 +58,9 @@ async function refreshDnsToken(username, password, server, fetchT, log) { headers: { 'Accept': 'application/json', 'Content-Type': 'application/x-www-form-urlencoded' - }, - timeout: 10000 - } + } + }, + 10000 ); const result = await response.json(); diff --git a/dashcaddy-api/src/context/provider-dns.js b/dashcaddy-api/src/context/provider-dns.js index 8753054..4fd39f0 100644 --- a/dashcaddy-api/src/context/provider-dns.js +++ b/dashcaddy-api/src/context/provider-dns.js @@ -96,7 +96,8 @@ function createProviderDnsContext(siteConfig, buildDomain, credentialManager, fe const params = new URLSearchParams({ user: username, pass: password, includeInfo: 'false' }); const response = await fetchT( `http://${server}:5380/api/user/login?${params.toString()}`, - { method: 'POST', headers: { 'Accept': 'application/json', 'Content-Type': 'application/x-www-form-urlencoded' }, timeout: 10000 } + { method: 'POST', headers: { 'Accept': 'application/json', 'Content-Type': 'application/x-www-form-urlencoded' } }, + 10000 ); const result = await response.json(); if (result.status === 'ok' && result.token) { diff --git a/dashcaddy-api/src/utils/http.js b/dashcaddy-api/src/utils/http.js index f82b8b2..b5275b7 100644 --- a/dashcaddy-api/src/utils/http.js +++ b/dashcaddy-api/src/utils/http.js @@ -38,7 +38,15 @@ function fetchT(url, opts = {}, timeoutMs = TIMEOUTS.HTTP_DEFAULT) { if (!opts.signal) { opts = { ...opts, signal: AbortSignal.timeout(timeoutMs) }; } - delete opts.timeout; + // The `timeout` key in fetch() opts is silently ignored by undici. Callers + // should use the third arg of fetchT() (timeoutMs) instead. If a caller + // passes `timeout: N` here, it's almost certainly a bug — we used to silently + // strip it, which masked the issue. Now we surface it in logs and strip it. + if ('timeout' in opts) { + console.warn(`[fetchT] opts.timeout=${opts.timeout} is ignored — pass timeoutMs as the 3rd arg of fetchT() instead. Called from: ${new Error().stack.split('\n').slice(2, 4).join(' <- ')}`); + const { timeout, ...rest } = opts; + opts = rest; + } return fetch(url, opts); } diff --git a/status/js/core/service-infrastructure.js b/status/js/core/service-infrastructure.js index 6b5b778..3dc8a34 100644 --- a/status/js/core/service-infrastructure.js +++ b/status/js/core/service-infrastructure.js @@ -14,15 +14,15 @@ const result = await response.json(); - if (result.status === 'success') { + if (result.success) { const select = document.getElementById('existing-ca-select'); select.innerHTML = ''; - if (result.data.cas.length === 0) { + if (result.cas.length === 0) { select.innerHTML = ''; } else { select.innerHTML = ''; - result.data.cas.forEach(ca => { + result.cas.forEach(ca => { const option = document.createElement('option'); if (typeof ca === 'object') { option.value = ca.id;