fix: avoid double-counting fetch failures #33

Merged
MrSphay merged 2 commits from codex/issue-15-fetch-metrics into codex/production-intelligence-terminal 2026-05-17 15:59:29 +00:00
2 changed files with 87 additions and 11 deletions

View File

@@ -42,6 +42,7 @@ export async function safeFetch(url, opts = {}) {
let lastError; let lastError;
for (let i = 0; i <= retries; i++) { for (let i = 0; i <= retries; i++) {
const started = Date.now(); const started = Date.now();
let metricRecorded = false;
try { try {
const controller = new AbortController(); const controller = new AbortController();
const timer = setTimeout(() => controller.abort(), timeout); const timer = setTimeout(() => controller.abort(), timeout);
@@ -51,22 +52,29 @@ export async function safeFetch(url, opts = {}) {
}); });
clearTimeout(timer); clearTimeout(timer);
const status = res.status; const status = res.status;
if (!res.ok) {
const body = await res.text().catch(() => '');
recordFetchMetric({ url, source, ok: false, status, bytes: body.length, durationMs: Date.now() - started, error: `HTTP ${res.status}` });
throw new Error(`HTTP ${res.status}: ${body.slice(0, 200)}`);
}
const text = await res.text(); const text = await res.text();
recordFetchMetric({ url, source, ok: true, status, bytes: text.length, durationMs: Date.now() - started }); if (!res.ok) {
const error = `HTTP ${res.status}`;
recordFetchMetric({ url, source, ok: false, status, bytes: text.length, durationMs: Date.now() - started, error });
metricRecorded = true;
throw new Error(`${error}: ${text.slice(0, 200)}`);
}
const trimmed = text.trim(); const trimmed = text.trim();
const contentType = res.headers.get('content-type') || ''; const contentType = res.headers.get('content-type') || '';
if (contentType.includes('text/html') || trimmed.startsWith('<!DOCTYPE html') || trimmed.startsWith('<html')) { if (contentType.includes('text/html') || trimmed.startsWith('<!DOCTYPE html') || trimmed.startsWith('<html')) {
throw new Error(`Expected JSON but received HTML from ${new URL(url).host}`); const error = `Expected JSON but received HTML from ${new URL(url).host}`;
recordFetchMetric({ url, source, ok: false, status, bytes: text.length, durationMs: Date.now() - started, error });
metricRecorded = true;
throw new Error(error);
} }
recordFetchMetric({ url, source, ok: true, status, bytes: text.length, durationMs: Date.now() - started });
metricRecorded = true;
try { return JSON.parse(text); } catch { return { rawText: text.slice(0, 500) }; } try { return JSON.parse(text); } catch { return { rawText: text.slice(0, 500) }; }
} catch (e) { } catch (e) {
lastError = e; lastError = e;
recordFetchMetric({ url, source, ok: false, status: null, bytes: 0, durationMs: Date.now() - started, error: e.message }); if (!metricRecorded) {
recordFetchMetric({ url, source, ok: false, status: null, bytes: 0, durationMs: Date.now() - started, error: e.message });
}
// GDELT needs 5s between requests, others are fine with shorter delays // GDELT needs 5s between requests, others are fine with shorter delays
if (i < retries) await new Promise(r => setTimeout(r, 2000 * (i + 1))); if (i < retries) await new Promise(r => setTimeout(r, 2000 * (i + 1)));
} }
@@ -79,6 +87,7 @@ export async function safeFetchText(url, opts = {}) {
let lastError; let lastError;
for (let i = 0; i <= retries; i++) { for (let i = 0; i <= retries; i++) {
const started = Date.now(); const started = Date.now();
let metricRecorded = false;
try { try {
const controller = new AbortController(); const controller = new AbortController();
const timer = setTimeout(() => controller.abort(), timeout); const timer = setTimeout(() => controller.abort(), timeout);
@@ -89,11 +98,14 @@ export async function safeFetchText(url, opts = {}) {
clearTimeout(timer); clearTimeout(timer);
const text = await res.text(); const text = await res.text();
recordFetchMetric({ url, source, ok: res.ok, status: res.status, bytes: text.length, durationMs: Date.now() - started, error: res.ok ? null : `HTTP ${res.status}` }); recordFetchMetric({ url, source, ok: res.ok, status: res.status, bytes: text.length, durationMs: Date.now() - started, error: res.ok ? null : `HTTP ${res.status}` });
metricRecorded = true;
if (!res.ok) throw new Error(`HTTP ${res.status}: ${text.slice(0, 200)}`); if (!res.ok) throw new Error(`HTTP ${res.status}: ${text.slice(0, 200)}`);
return { text, status: res.status, bytes: text.length }; return { text, status: res.status, bytes: text.length };
} catch (e) { } catch (e) {
lastError = e; lastError = e;
recordFetchMetric({ url, source, ok: false, status: null, bytes: 0, durationMs: Date.now() - started, error: e.message }); if (!metricRecorded) {
recordFetchMetric({ url, source, ok: false, status: null, bytes: 0, durationMs: Date.now() - started, error: e.message });
}
if (i < retries) await new Promise(r => setTimeout(r, 2000 * (i + 1))); if (i < retries) await new Promise(r => setTimeout(r, 2000 * (i + 1)));
} }
} }

View File

@@ -5,6 +5,7 @@ import { safeFetch, safeFetchText, getFetchMetrics } from '../apis/utils/fetch.m
test('safeFetch reports HTML as degraded JSON response', async () => { test('safeFetch reports HTML as degraded JSON response', async () => {
const originalFetch = globalThis.fetch; const originalFetch = globalThis.fetch;
const source = 'unit-html-once';
globalThis.fetch = async () => ({ globalThis.fetch = async () => ({
ok: true, ok: true,
status: 200, status: 200,
@@ -12,9 +13,72 @@ test('safeFetch reports HTML as degraded JSON response', async () => {
text: async () => '<html>not json</html>', text: async () => '<html>not json</html>',
}); });
try { try {
const data = await safeFetch('https://example.test/json', { retries: 0, source: 'unit' }); const data = await safeFetch('https://example.test/json', { retries: 0, source });
assert.match(data.error, /Expected JSON/); assert.match(data.error, /Expected JSON/);
assert.ok(getFetchMetrics().bySource.unit.requests >= 1); const bucket = getFetchMetrics().bySource[source];
assert.equal(bucket.requests, 1);
assert.equal(bucket.ok, 0);
assert.equal(bucket.failed, 1);
assert.equal(bucket.lastStatus, 200);
} finally {
globalThis.fetch = originalFetch;
}
});
test('safeFetch records HTTP failure once with status and bytes', async () => {
const originalFetch = globalThis.fetch;
const source = 'unit-http-failure-once';
globalThis.fetch = async () => ({
ok: false,
status: 503,
headers: { get: () => 'application/json' },
text: async () => 'service unavailable',
});
try {
const data = await safeFetch('https://example.test/fail', { retries: 0, source });
assert.match(data.error, /HTTP 503/);
const bucket = getFetchMetrics().bySource[source];
assert.equal(bucket.requests, 1);
assert.equal(bucket.ok, 0);
assert.equal(bucket.failed, 1);
assert.equal(bucket.lastStatus, 503);
assert.equal(bucket.bytes, 'service unavailable'.length);
assert.match(bucket.lastError, /HTTP 503/);
} finally {
globalThis.fetch = originalFetch;
}
});
test('safeFetch retry metrics count one record per attempt', async () => {
const originalFetch = globalThis.fetch;
const source = 'unit-retry-attempts';
let calls = 0;
globalThis.fetch = async () => {
calls += 1;
if (calls === 1) {
return {
ok: false,
status: 502,
headers: { get: () => 'application/json' },
text: async () => 'bad gateway',
};
}
return {
ok: true,
status: 200,
headers: { get: () => 'application/json' },
text: async () => '{"ok":true}',
};
};
try {
const data = await safeFetch('https://example.test/retry', { retries: 1, source });
assert.equal(data.ok, true);
assert.equal(calls, 2);
const bucket = getFetchMetrics().bySource[source];
assert.equal(bucket.requests, 2);
assert.equal(bucket.ok, 1);
assert.equal(bucket.failed, 1);
assert.equal(bucket.lastStatus, 200);
} finally { } finally {
globalThis.fetch = originalFetch; globalThis.fetch = originalFetch;
} }