Skip to content

Commit 140cd36

Browse files
fix(db,runs): loop-failure error_logs fallback + don't delete a shared run log
Two MEDIUM findings from the bug hunt: - Public failure-drawer empty for continuous-loop failures: loop runs persist per node ONLY to batch_results (not results/error_logs), so getNodeErrors — which queries error_logs⋈results with a results-table fallback — returned nothing for a loop-only failure, violating the failure-log MUST. Add a second fallback that synthesizes a failure entry from the most recent failed batch_results row (it stores error + error_code). Same shape/aliases as the other queries; failed-only (actual_mbps IS NULL OR error_code IS NOT NULL); no schema migration needed. - deleteRun could purge a raw audit-*.log still referenced by ANOTHER saved run (resume reuses one log file across passes). Guard the delete with sharedByOther = index.runs.some(r => r.auditLog === entry.auditLog) (index.runs is post-splice) so a shared log isn't deleted out from under another run's Live Log. Adversarially reviewed (runtime-verified against a live schema): columns exist via migrations, shape parity with the primary/results queries, failed-only filter, post-splice some() check correct. No TEST RUN paths touched. Test suite green (188/31/45/35).
1 parent d24c87b commit 140cd36

2 files changed

Lines changed: 41 additions & 2 deletions

File tree

core/db.js

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1154,7 +1154,41 @@ export function getNodeErrors(addr, { limit = 50, stage = null } = {}, which) {
11541154
ORDER BY r.tested_at DESC
11551155
LIMIT @limit
11561156
`).all(params);
1157-
return fallback;
1157+
if (fallback.length > 0) return fallback;
1158+
1159+
// Second fallback: continuous-loop (public) runs persist per node ONLY to
1160+
// batch_results — never to results/error_logs — so a node that failed only
1161+
// inside a loop has no row in either table queried above. batch_results does
1162+
// store error + error_code, so synthesize a failure entry from the most recent
1163+
// failed batch_results row. Without this the public failure-drawer is empty for
1164+
// loop-only failures, violating the failure-log MUST contract. batch_results
1165+
// has no `stage`, so a specific stage filter can't match here.
1166+
if (stage && stage !== 'unknown') return fallback; // empty — no stage in batch_results
1167+
const batchFallback = db.prepare(`
1168+
SELECT NULL AS id, NULL AS result_id,
1169+
'unknown' AS stage,
1170+
COALESCE(br.error_code, 'UNKNOWN') AS error_code,
1171+
COALESCE(br.error, '(no message captured)') AS error_message,
1172+
NULL AS log_snippet,
1173+
br.tested_at AS captured_at,
1174+
br.tested_at, br.actual_mbps,
1175+
br.node_address AS node_addr, br.moniker,
1176+
br.country AS country,
1177+
br.city AS city,
1178+
br.country_code AS country_code,
1179+
br.type AS service_type,
1180+
NULL AS advertised_mbps, NULL AS latency_ms,
1181+
NULL AS handshake_ok, NULL AS session_ok, NULL AS pass,
1182+
NULL AS sdk, NULL AS continent, NULL AS tester_os,
1183+
NULL AS run_id, NULL AS raw_json,
1184+
1 AS synthesized
1185+
FROM batch_results br
1186+
WHERE br.node_address = @addr
1187+
AND (br.actual_mbps IS NULL OR br.error_code IS NOT NULL)
1188+
ORDER BY br.tested_at DESC
1189+
LIMIT @limit
1190+
`).all(params);
1191+
return batchFallback;
11581192
}
11591193

11601194
/**

server.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -961,7 +961,12 @@ function deleteRun(num) {
961961
if (entry && entry.auditLog) {
962962
const rawLog = path.join(__dirname, 'results', entry.auditLog);
963963
const isActive = state.auditLogPath && path.basename(state.auditLogPath) === entry.auditLog;
964-
if (!isActive && _ex(rawLog)) {
964+
// Don't delete a log file still referenced by ANOTHER saved run — resume
965+
// reuses one audit-*.log across passes, so deleting this run's basename would
966+
// orphan another run's Live Log. index.runs here is already post-splice (this
967+
// entry removed), so .some() checks only the runs that remain.
968+
const sharedByOther = index.runs.some(r => r.auditLog === entry.auditLog);
969+
if (!isActive && !sharedByOther && _ex(rawLog)) {
965970
try { _rm(rawLog, { force: true }); }
966971
catch (err) { console.error(`[deleteRun] failed to remove log: ${err.message}`); }
967972
}

0 commit comments

Comments
 (0)