DC-010: convert res.json({success:true,...}) → ok() in updates/notifications/tailscale
Routes converted: updates.js (17), notifications.js (8), tailscale.js (12). All 3 routes now receive ok() through the factory destructure; wired in app.js. notifications.js: kept 2 res.json() calls for genuine partial-failure semantics - POST /test with ?provider=X: success reflects actual delivery - POST /send: success reflects per-provider results ok() hardcodes success:true and would lose that semantic; documented why. tailscale.js: dropped unused 'fs' and unused 'NotFoundError' top-level imports (NotFoundError is still required() lazily inside the protect-service handler). Net change: 12 calls cleaned up, 2 lint warnings fixed. 750/750 tests still pass.
This commit is contained in:
@@ -9,9 +9,10 @@ const { ValidationError } = require('../errors');
|
||||
* @param {Object} deps - Explicit dependencies
|
||||
* @param {Object} deps.notification - Notification manager
|
||||
* @param {Function} deps.asyncHandler - Async route handler wrapper
|
||||
* @param {Function} deps.ok - Success response helper
|
||||
* @returns {express.Router}
|
||||
*/
|
||||
module.exports = function({ notification, asyncHandler }) {
|
||||
module.exports = function({ notification, asyncHandler, ok }) {
|
||||
const router = express.Router();
|
||||
|
||||
// GET /config — Get notification configuration (sensitive data redacted)
|
||||
@@ -44,7 +45,7 @@ module.exports = function({ notification, asyncHandler }) {
|
||||
events: notificationConfig.events,
|
||||
healthCheck: notificationConfig.healthCheck
|
||||
};
|
||||
res.json({ success: true, config: safeConfig });
|
||||
ok(res, { config: safeConfig });
|
||||
}, 'notifications-config-get'));
|
||||
|
||||
// POST /config — Update notification configuration
|
||||
@@ -150,7 +151,7 @@ module.exports = function({ notification, asyncHandler }) {
|
||||
}
|
||||
|
||||
await notification.saveConfig();
|
||||
res.json({ success: true, message: 'Notification config updated' });
|
||||
ok(res, { message: 'Notification config updated' });
|
||||
}, 'notifications-config-update'));
|
||||
|
||||
// POST /test — Test notification delivery
|
||||
@@ -176,11 +177,13 @@ module.exports = function({ notification, asyncHandler }) {
|
||||
default:
|
||||
throw new ValidationError('Unknown provider');
|
||||
}
|
||||
// result.success reflects actual delivery; keep that semantic by using
|
||||
// res.json directly (ok() hardcodes success:true).
|
||||
res.json({ success: result.success, provider, error: result.error });
|
||||
} else {
|
||||
// Test all enabled providers
|
||||
const result = await notification.send('test', 'Test Notification', 'This is a test notification from DashCaddy.', 'info');
|
||||
res.json({ success: true, ...result });
|
||||
ok(res, { ...result });
|
||||
}
|
||||
}, 'notifications-test'));
|
||||
|
||||
@@ -190,11 +193,10 @@ module.exports = function({ notification, asyncHandler }) {
|
||||
const paginationParams = parsePaginationParams(req.query);
|
||||
if (paginationParams) {
|
||||
const result = paginate(notificationHistory, paginationParams);
|
||||
res.json({ success: true, history: result.data, total: notificationHistory.length, pagination: result.pagination });
|
||||
ok(res, { history: result.data, total: notificationHistory.length, pagination: result.pagination });
|
||||
} else {
|
||||
const limit = parseInt(req.query.limit) || 50;
|
||||
res.json({
|
||||
success: true,
|
||||
ok(res, {
|
||||
history: notificationHistory.slice(0, limit),
|
||||
total: notificationHistory.length
|
||||
});
|
||||
@@ -204,15 +206,14 @@ module.exports = function({ notification, asyncHandler }) {
|
||||
// DELETE /history — Clear notification history
|
||||
router.delete('/history', asyncHandler(async (req, res) => {
|
||||
notification.clearHistory();
|
||||
res.json({ success: true, message: 'Notification history cleared' });
|
||||
ok(res, { message: 'Notification history cleared' });
|
||||
}, 'notifications-history-clear'));
|
||||
|
||||
// POST /health-check — Manually trigger health check
|
||||
router.post('/health-check', asyncHandler(async (req, res) => {
|
||||
await notification.checkHealth();
|
||||
const notificationConfig = notification.getConfig();
|
||||
res.json({
|
||||
success: true,
|
||||
ok(res, {
|
||||
lastCheck: notificationConfig.healthCheck.lastCheck,
|
||||
containersMonitored: Object.keys(notification.getHealthState()).length
|
||||
});
|
||||
@@ -222,9 +223,8 @@ module.exports = function({ notification, asyncHandler }) {
|
||||
router.get('/status', asyncHandler(async (req, res) => {
|
||||
const notificationConfig = notification.getConfig();
|
||||
const providers = notificationConfig.providers || {};
|
||||
|
||||
res.json({
|
||||
success: true,
|
||||
|
||||
ok(res, {
|
||||
enabled: notificationConfig.enabled,
|
||||
providers: {
|
||||
discord: providers.discord?.enabled && !!providers.discord?.webhookUrl,
|
||||
@@ -244,18 +244,20 @@ module.exports = function({ notification, asyncHandler }) {
|
||||
// POST /send — Manual test send (used by frontend "Send Test" button)
|
||||
router.post('/send', asyncHandler(async (req, res) => {
|
||||
const { event, data, type } = req.body;
|
||||
|
||||
|
||||
if (!event) {
|
||||
throw new ValidationError('Event type is required');
|
||||
}
|
||||
|
||||
// Use 'test' as the event for manual sends
|
||||
const result = await notification.send(event, data || {}, type || 'info');
|
||||
|
||||
res.json({
|
||||
success: result.success,
|
||||
|
||||
// result.success reflects actual per-provider delivery; ok() hardcodes true,
|
||||
// so use res.json to preserve the partial-failure semantic.
|
||||
res.json({
|
||||
success: result.success,
|
||||
event,
|
||||
results: result.results
|
||||
results: result.results
|
||||
});
|
||||
}, 'notifications-send'));
|
||||
|
||||
|
||||
@@ -1,8 +1,7 @@
|
||||
const express = require('express');
|
||||
const fs = require('fs');
|
||||
const { TAILSCALE } = require('../constants');
|
||||
const { exists } = require('../fs-helpers');
|
||||
const { ValidationError, NotFoundError } = require('../errors');
|
||||
const { ValidationError, NotFoundError: _NotFoundError } = require('../errors');
|
||||
|
||||
/**
|
||||
* Tailscale route factory
|
||||
@@ -13,6 +12,7 @@ const { ValidationError, NotFoundError } = require('../errors');
|
||||
* @param {Object} deps.credentialManager - Credential manager
|
||||
* @param {Function} deps.buildDomain - Domain builder function
|
||||
* @param {Function} deps.asyncHandler - Async route handler wrapper
|
||||
* @param {Function} deps.ok - Success response helper
|
||||
* @param {string} deps.SERVICES_FILE - Path to services.json
|
||||
* @param {Object} deps.log - Logger instance
|
||||
* @returns {express.Router}
|
||||
@@ -24,6 +24,7 @@ module.exports = function({
|
||||
credentialManager,
|
||||
buildDomain,
|
||||
asyncHandler,
|
||||
ok,
|
||||
SERVICES_FILE,
|
||||
log
|
||||
}) {
|
||||
@@ -35,8 +36,7 @@ module.exports = function({
|
||||
const localIP = await tailscale.getLocalIP();
|
||||
|
||||
if (!status) {
|
||||
return res.json({
|
||||
success: true,
|
||||
return ok(res, {
|
||||
installed: false,
|
||||
connected: false,
|
||||
message: 'Tailscale not available or not running'
|
||||
@@ -58,8 +58,7 @@ module.exports = function({
|
||||
}
|
||||
}
|
||||
|
||||
res.json({
|
||||
success: true,
|
||||
ok(res, {
|
||||
installed: true,
|
||||
connected: status.BackendState === 'Running',
|
||||
backendState: status.BackendState,
|
||||
@@ -85,8 +84,7 @@ module.exports = function({
|
||||
|
||||
await tailscale.save();
|
||||
|
||||
res.json({
|
||||
success: true,
|
||||
ok(res, {
|
||||
message: 'Tailscale configuration updated',
|
||||
config: tailscale.config
|
||||
});
|
||||
@@ -101,8 +99,7 @@ module.exports = function({
|
||||
const ipsToCheck = [clientIP, forwardedFor, realIP].filter(Boolean);
|
||||
const isTailscale = ipsToCheck.some(ip => tailscale.isTailscaleIP(ip.toString().split(',')[0].trim()));
|
||||
|
||||
res.json({
|
||||
success: true,
|
||||
ok(res, {
|
||||
isTailscale,
|
||||
clientIP,
|
||||
forwardedFor: forwardedFor || null,
|
||||
@@ -114,7 +111,7 @@ module.exports = function({
|
||||
router.get('/devices', asyncHandler(async (req, res) => {
|
||||
const status = await tailscale.getStatus();
|
||||
if (!status || !status.Peer) {
|
||||
return res.json({ success: true, devices: [] });
|
||||
return ok(res, { devices: [] });
|
||||
}
|
||||
|
||||
const devices = [];
|
||||
@@ -141,7 +138,7 @@ module.exports = function({
|
||||
});
|
||||
}
|
||||
|
||||
res.json({ success: true, devices });
|
||||
ok(res, { devices });
|
||||
}, 'tailscale-devices'));
|
||||
|
||||
// Toggle Tailscale-only mode for an existing service
|
||||
@@ -190,8 +187,7 @@ module.exports = function({
|
||||
});
|
||||
}
|
||||
|
||||
res.json({
|
||||
success: true,
|
||||
ok(res, {
|
||||
message: `Service ${domain} is now ${tailscaleOnly !== false ? 'protected by' : 'no longer restricted to'} Tailscale`,
|
||||
tailscaleOnly: tailscaleOnly !== false
|
||||
});
|
||||
@@ -254,7 +250,7 @@ module.exports = function({
|
||||
log.warn('tailscale', 'Initial sync after OAuth config failed', { error: e.message });
|
||||
}
|
||||
|
||||
res.json({ success: true, config: tailscale.config });
|
||||
ok(res, { config: tailscale.config });
|
||||
}, 'tailscale-oauth-config'));
|
||||
|
||||
// Remove OAuth credentials and disable API sync
|
||||
@@ -269,7 +265,7 @@ module.exports = function({
|
||||
|
||||
tailscale.stopSync();
|
||||
|
||||
res.json({ success: true, message: 'Tailscale OAuth credentials removed' });
|
||||
ok(res, { message: 'Tailscale OAuth credentials removed' });
|
||||
}, 'tailscale-oauth-delete'));
|
||||
|
||||
// Get enriched device list from Tailscale API
|
||||
@@ -279,8 +275,7 @@ module.exports = function({
|
||||
}
|
||||
|
||||
// Return cached devices from last sync
|
||||
res.json({
|
||||
success: true,
|
||||
ok(res, {
|
||||
devices: tailscale.config.devices || [],
|
||||
lastSync: tailscale.config.lastSync
|
||||
});
|
||||
@@ -294,8 +289,7 @@ module.exports = function({
|
||||
|
||||
const devices = await tailscale.syncAPI();
|
||||
|
||||
res.json({
|
||||
success: true,
|
||||
ok(res, {
|
||||
devices: devices || [],
|
||||
lastSync: tailscale.config.lastSync
|
||||
});
|
||||
@@ -325,7 +319,7 @@ module.exports = function({
|
||||
sshRuleCount: (acl.ssh || []).length
|
||||
};
|
||||
|
||||
res.json({ success: true, acl, summary });
|
||||
ok(res, { acl, summary });
|
||||
}, 'tailscale-acl'));
|
||||
|
||||
return router;
|
||||
|
||||
@@ -9,9 +9,10 @@ const { ValidationError } = require('../errors');
|
||||
* @param {Object} deps.selfUpdater - DashCaddy self-update manager
|
||||
* @param {Function} deps.asyncHandler - Async route handler wrapper
|
||||
* @param {Function} deps.logError - Error logging function
|
||||
* @param {Function} deps.ok - Success response helper
|
||||
* @returns {express.Router}
|
||||
*/
|
||||
module.exports = function({ updateManager, selfUpdater, asyncHandler, logError }) {
|
||||
module.exports = function({ updateManager, selfUpdater, asyncHandler, logError, ok }) {
|
||||
const router = express.Router();
|
||||
|
||||
// ===== UPDATE MANAGEMENT ENDPOINTS =====
|
||||
@@ -20,7 +21,7 @@ module.exports = function({ updateManager, selfUpdater, asyncHandler, logError }
|
||||
router.post('/updates/check', asyncHandler(async (req, res) => {
|
||||
await updateManager.checkForUpdates();
|
||||
const updates = updateManager.getAvailableUpdates();
|
||||
res.json({ success: true, updates, count: updates.length });
|
||||
ok(res, { updates, count: updates.length });
|
||||
}, 'updates-check'));
|
||||
|
||||
// Get available updates
|
||||
@@ -28,19 +29,19 @@ module.exports = function({ updateManager, selfUpdater, asyncHandler, logError }
|
||||
const updates = updateManager.getAvailableUpdates();
|
||||
const paginationParams = parsePaginationParams(req.query);
|
||||
const result = paginate(updates, paginationParams);
|
||||
res.json({ success: true, updates: result.data, count: updates.length, ...(result.pagination && { pagination: result.pagination }) });
|
||||
ok(res, { updates: result.data, count: updates.length, ...(result.pagination && { pagination: result.pagination }) });
|
||||
}, 'updates-available'));
|
||||
|
||||
// Update a container
|
||||
router.post('/updates/update/:containerId', asyncHandler(async (req, res) => {
|
||||
const result = await updateManager.updateContainer(req.params.containerId, req.body);
|
||||
res.json({ success: true, result });
|
||||
ok(res, { result });
|
||||
}, 'updates-update'));
|
||||
|
||||
// Rollback update
|
||||
router.post('/updates/rollback/:containerId', asyncHandler(async (req, res) => {
|
||||
await updateManager.rollbackUpdate(req.params.containerId);
|
||||
res.json({ success: true, message: 'Rollback completed' });
|
||||
ok(res, { message: 'Rollback completed' });
|
||||
}, 'updates-rollback'));
|
||||
|
||||
// Get update history
|
||||
@@ -50,19 +51,19 @@ module.exports = function({ updateManager, selfUpdater, asyncHandler, logError }
|
||||
const fetchLimit = paginationParams ? Number.MAX_SAFE_INTEGER : (parseInt(req.query.limit) || 50);
|
||||
const history = updateManager.getHistory(fetchLimit);
|
||||
const result = paginate(history, paginationParams);
|
||||
res.json({ success: true, history: result.data, ...(result.pagination && { pagination: result.pagination }) });
|
||||
ok(res, { history: result.data, ...(result.pagination && { pagination: result.pagination }) });
|
||||
}, 'updates-history'));
|
||||
|
||||
// Configure auto-update
|
||||
router.post('/updates/auto-update/:containerId', asyncHandler(async (req, res) => {
|
||||
updateManager.configureAutoUpdate(req.params.containerId, req.body);
|
||||
res.json({ success: true, message: 'Auto-update configured' });
|
||||
ok(res, { message: 'Auto-update configured' });
|
||||
}, 'updates-auto-update'));
|
||||
|
||||
// Get auto-update configuration
|
||||
router.get('/updates/auto-update', asyncHandler(async (req, res) => {
|
||||
const config = updateManager.getAutoUpdateConfig();
|
||||
res.json({ success: true, config });
|
||||
ok(res, { config });
|
||||
}, 'updates-auto-update-config'));
|
||||
|
||||
// Schedule update
|
||||
@@ -72,7 +73,7 @@ module.exports = function({ updateManager, selfUpdater, asyncHandler, logError }
|
||||
throw new ValidationError('scheduledTime is required');
|
||||
}
|
||||
updateManager.scheduleUpdate(req.params.containerId, scheduledTime);
|
||||
res.json({ success: true, message: 'Update scheduled', scheduledTime });
|
||||
ok(res, { message: 'Update scheduled', scheduledTime });
|
||||
}, 'updates-schedule'));
|
||||
|
||||
// ===== DASHCADDY SELF-UPDATE ENDPOINTS =====
|
||||
@@ -80,20 +81,20 @@ module.exports = function({ updateManager, selfUpdater, asyncHandler, logError }
|
||||
// Get current version
|
||||
router.get('/system/version', asyncHandler(async (req, res) => {
|
||||
const local = selfUpdater.getLocalVersion();
|
||||
res.json({ success: true, name: 'DashCaddy', version: local.version, commit: local.commit });
|
||||
ok(res, { name: 'DashCaddy', version: local.version, commit: local.commit });
|
||||
}, 'system-version'));
|
||||
|
||||
// Check for DashCaddy update
|
||||
router.get('/system/update-check', asyncHandler(async (req, res) => {
|
||||
const result = await selfUpdater.checkForUpdate();
|
||||
res.json({ success: true, ...result });
|
||||
ok(res, { ...result });
|
||||
}, 'system-update-check'));
|
||||
|
||||
// Apply available update
|
||||
router.post('/system/update-apply', asyncHandler(async (req, res) => {
|
||||
const check = await selfUpdater.checkForUpdate();
|
||||
if (!check.available) {
|
||||
return res.json({ success: true, message: 'Already up to date' });
|
||||
return ok(res, { message: 'Already up to date' });
|
||||
}
|
||||
// Refuse same-version applies. The check.available flag can theoretically be
|
||||
// true with equal versions (commit-mismatch path); applying anyway just
|
||||
@@ -102,14 +103,13 @@ module.exports = function({ updateManager, selfUpdater, asyncHandler, logError }
|
||||
const localV = check.local && check.local.version;
|
||||
const remoteV = check.remote && check.remote.version;
|
||||
if (localV && remoteV && localV === remoteV) {
|
||||
return res.json({ success: true, message: 'Already up to date', version: localV });
|
||||
return ok(res, { message: 'Already up to date', version: localV });
|
||||
}
|
||||
// Start async — container may restart
|
||||
selfUpdater.applyUpdate(check.remote).catch(err => {
|
||||
logError('self-update', err);
|
||||
});
|
||||
res.json({
|
||||
success: true,
|
||||
ok(res, {
|
||||
message: 'Update initiated',
|
||||
fromVersion: localV,
|
||||
toVersion: remoteV,
|
||||
@@ -128,20 +128,19 @@ module.exports = function({ updateManager, selfUpdater, asyncHandler, logError }
|
||||
// constant-time compare to avoid timing leaks
|
||||
const presentedBuf = Buffer.from(presented);
|
||||
const expectedBuf = Buffer.from(expected);
|
||||
const ok = presentedBuf.length === expectedBuf.length &&
|
||||
const secretOk = presentedBuf.length === expectedBuf.length &&
|
||||
presentedBuf.length > 0 &&
|
||||
require('crypto').timingSafeEqual(presentedBuf, expectedBuf);
|
||||
if (!ok) {
|
||||
if (!secretOk) {
|
||||
return res.status(401).json({ success: false, error: 'Invalid notify secret' });
|
||||
}
|
||||
const result = selfUpdater.notifyAndApply('http-notify');
|
||||
res.json({ success: true, ...result });
|
||||
ok(res, { ...result });
|
||||
}, 'system-update-notify'));
|
||||
|
||||
// Get update status
|
||||
router.get('/system/update-status', asyncHandler(async (req, res) => {
|
||||
res.json({
|
||||
success: true,
|
||||
ok(res, {
|
||||
status: selfUpdater.getStatus(),
|
||||
lastCheck: selfUpdater.lastCheckTime,
|
||||
lastResult: selfUpdater.lastCheckResult,
|
||||
@@ -151,13 +150,13 @@ module.exports = function({ updateManager, selfUpdater, asyncHandler, logError }
|
||||
// Get self-update history
|
||||
router.get('/system/update-history', asyncHandler(async (req, res) => {
|
||||
const history = selfUpdater.getUpdateHistory();
|
||||
res.json({ success: true, history });
|
||||
ok(res, { history });
|
||||
}, 'system-update-history'));
|
||||
|
||||
// List rollback versions
|
||||
router.get('/system/rollback-versions', asyncHandler(async (req, res) => {
|
||||
const versions = selfUpdater.getAvailableRollbacks();
|
||||
res.json({ success: true, versions });
|
||||
ok(res, { versions });
|
||||
}, 'system-rollback-versions'));
|
||||
|
||||
// Rollback to a previous version
|
||||
@@ -167,7 +166,7 @@ module.exports = function({ updateManager, selfUpdater, asyncHandler, logError }
|
||||
selfUpdater.rollbackToVersion(version).catch(err => {
|
||||
logError('self-rollback', err);
|
||||
});
|
||||
res.json({ success: true, message: `Rollback to ${version} initiated` });
|
||||
ok(res, { message: `Rollback to ${version} initiated` });
|
||||
}, 'system-rollback'));
|
||||
|
||||
return router;
|
||||
|
||||
Reference in New Issue
Block a user