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)
This commit is contained in:
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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": {
|
||||
|
||||
@@ -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' });
|
||||
|
||||
@@ -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
|
||||
});
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user