fix: avoid double-counting fetch failures #33
@@ -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)));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user