Skip to content

Commit 7e12862

Browse files
fix(http-fetcher): fall back to reloadInterval after retries exhausted (#4113)
As reported in #4109, the weather module retries much more frequently than expected after network errors. #4092 already fixed the main cause (duplicate fetchers), but the backoff logic in `HTTPFetcher` still has a gap: once retries are exhausted, `calculateBackoffDelay` keeps returning a short fixed delay (60s) instead of falling back to `reloadInterval`. The same problem existed for 5xx errors, where the delay grew to 8× the configured interval. Inspired by #4110 (thanks @CodeLine9), this PR makes both error paths fall back to `reloadInterval` after retries are exhausted. I also simplified the catch block, extracted a `#shortenUrl()` helper for log messages, and added tests for the backoff progression.
1 parent 3f2a030 commit 7e12862

2 files changed

Lines changed: 120 additions & 38 deletions

File tree

js/http_fetcher.js

Lines changed: 40 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,19 @@ class HTTPFetcher extends EventEmitter {
183183
return null;
184184
}
185185

186+
/**
187+
* Returns a shortened version of the URL for log messages.
188+
* @returns {string} Shortened URL
189+
*/
190+
#shortenUrl () {
191+
try {
192+
const urlObj = new URL(this.url);
193+
return `${urlObj.origin}${urlObj.pathname}${urlObj.search.length > 50 ? "?..." : urlObj.search}`;
194+
} catch {
195+
return this.url;
196+
}
197+
}
198+
186199
/**
187200
* Determines the retry delay for a non-ok response
188201
* @param {Response} response - The fetch Response object
@@ -198,28 +211,35 @@ class HTTPFetcher extends EventEmitter {
198211
errorType = "AUTH_FAILURE";
199212
delay = Math.max(this.reloadInterval * 5, THIRTY_MINUTES);
200213
message = `Authentication failed (${status}). Check your API key. Waiting ${Math.round(delay / 60000)} minutes before retry.`;
201-
Log.error(`${this.logContext}${this.url} - ${message}`);
214+
Log.error(`${this.logContext}${this.#shortenUrl()} - ${message}`);
202215
} else if (status === 429) {
203216
errorType = "RATE_LIMITED";
204217
const retryAfter = response.headers.get("retry-after");
205218
const parsed = retryAfter ? this.#parseRetryAfter(retryAfter) : null;
206219
delay = parsed !== null ? Math.max(parsed, this.reloadInterval) : Math.max(this.reloadInterval * 2, FIFTEEN_MINUTES);
207220
message = `Rate limited (429). Retrying in ${Math.round(delay / 60000)} minutes.`;
208-
Log.warn(`${this.logContext}${this.url} - ${message}`);
221+
Log.warn(`${this.logContext}${this.#shortenUrl()} - ${message}`);
209222
} else if (status >= 500) {
210223
errorType = "SERVER_ERROR";
211224
this.serverErrorCount = Math.min(this.serverErrorCount + 1, this.maxRetries);
212-
delay = this.reloadInterval * Math.pow(2, this.serverErrorCount);
213-
message = `Server error (${status}). Retry #${this.serverErrorCount} in ${Math.round(delay / 60000)} minutes.`;
214-
Log.error(`${this.logContext}${this.url} - ${message}`);
225+
if (this.serverErrorCount >= this.maxRetries) {
226+
delay = this.reloadInterval;
227+
message = `Server error (${status}). Max retries reached, retrying at configured interval (${Math.round(delay / 1000)}s).`;
228+
} else {
229+
delay = HTTPFetcher.calculateBackoffDelay(this.serverErrorCount, {
230+
maxDelay: this.reloadInterval
231+
});
232+
message = `Server error (${status}). Retry #${this.serverErrorCount} in ${Math.round(delay / 1000)}s.`;
233+
}
234+
Log.error(`${this.logContext}${this.#shortenUrl()} - ${message}`);
215235
} else if (status >= 400) {
216236
errorType = "CLIENT_ERROR";
217237
delay = Math.max(this.reloadInterval * 2, FIFTEEN_MINUTES);
218238
message = `Client error (${status}). Retrying in ${Math.round(delay / 60000)} minutes.`;
219-
Log.error(`${this.logContext}${this.url} - ${message}`);
239+
Log.error(`${this.logContext}${this.#shortenUrl()} - ${message}`);
220240
} else {
221241
message = `Unexpected HTTP status ${status}.`;
222-
Log.error(`${this.logContext}${this.url} - ${message}`);
242+
Log.error(`${this.logContext}${this.#shortenUrl()} - ${message}`);
223243
}
224244

225245
return {
@@ -293,28 +313,22 @@ class HTTPFetcher extends EventEmitter {
293313
const isTimeout = error.name === "AbortError";
294314
const message = isTimeout ? `Request timeout after ${this.timeout}ms` : `Network error: ${error.message}`;
295315

296-
// Apply exponential backoff for network errors
297316
this.networkErrorCount = Math.min(this.networkErrorCount + 1, this.maxRetries);
298-
const backoffDelay = HTTPFetcher.calculateBackoffDelay(this.networkErrorCount, {
299-
maxDelay: this.reloadInterval
300-
});
301-
nextDelay = backoffDelay;
302-
303-
// Truncate URL for cleaner logs
304-
let shortUrl = this.url;
305-
try {
306-
const urlObj = new URL(this.url);
307-
shortUrl = `${urlObj.origin}${urlObj.pathname}${urlObj.search.length > 50 ? "?..." : urlObj.search}`;
308-
} catch {
309-
// If URL parsing fails, use original URL
310-
}
317+
const exhausted = this.networkErrorCount >= this.maxRetries;
311318

312-
// Gradual log-level escalation: WARN for first 2 attempts, ERROR after
313-
const retryMessage = `Retry #${this.networkErrorCount} in ${Math.round(nextDelay / 1000)}s.`;
314-
if (this.networkErrorCount <= 2) {
315-
Log.warn(`${this.logContext}${shortUrl} - ${message} ${retryMessage}`);
319+
if (exhausted) {
320+
nextDelay = this.reloadInterval;
321+
Log.error(`${this.logContext}${this.#shortenUrl()} - ${message} Max retries reached, retrying at configured interval (${Math.round(nextDelay / 1000)}s).`);
316322
} else {
317-
Log.error(`${this.logContext}${shortUrl} - ${message} ${retryMessage}`);
323+
nextDelay = HTTPFetcher.calculateBackoffDelay(this.networkErrorCount, {
324+
maxDelay: this.reloadInterval
325+
});
326+
const retryMsg = `${this.logContext}${this.#shortenUrl()} - ${message} Retry #${this.networkErrorCount} in ${Math.round(nextDelay / 1000)}s.`;
327+
if (this.networkErrorCount <= 2) {
328+
Log.warn(retryMsg);
329+
} else {
330+
Log.error(retryMsg);
331+
}
318332
}
319333

320334
const errorInfo = this.#createErrorInfo(
@@ -324,18 +338,6 @@ class HTTPFetcher extends EventEmitter {
324338
nextDelay,
325339
error
326340
);
327-
328-
/**
329-
* Error event - fired when fetch fails
330-
* @event HTTPFetcher#error
331-
* @type {object}
332-
* @property {string} message - Error description
333-
* @property {number|null} statusCode - HTTP status or null for network errors
334-
* @property {number} retryDelay - Ms until next retry
335-
* @property {number} retryCount - Number of consecutive server errors
336-
* @property {string} url - The URL that was fetched
337-
* @property {Error|null} originalError - The original error
338-
*/
339341
this.emit("error", errorInfo);
340342
} finally {
341343
clearTimeout(timeoutId);

tests/unit/functions/http_fetcher_spec.js

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,3 +469,83 @@ describe("selfSignedCert dispatcher", () => {
469469
expect(options.dispatcher).toBeUndefined();
470470
});
471471
});
472+
473+
describe("Retry exhaustion fallback", () => {
474+
it("should fall back to reloadInterval after network retries exhausted", async () => {
475+
server.use(
476+
http.get(TEST_URL, () => {
477+
return HttpResponse.error();
478+
})
479+
);
480+
481+
fetcher = new HTTPFetcher(TEST_URL, { reloadInterval: 300000, maxRetries: 3 });
482+
483+
const errors = [];
484+
fetcher.on("error", (errorInfo) => errors.push(errorInfo));
485+
486+
// Trigger maxRetries + 1 fetches to reach exhaustion
487+
for (let i = 0; i < 4; i++) {
488+
await fetcher.fetch();
489+
}
490+
491+
// First retries should use backoff (< reloadInterval)
492+
expect(errors[0].retryAfter).toBe(15000);
493+
expect(errors[1].retryAfter).toBe(30000);
494+
// Third retry hits maxRetries, should fall back to reloadInterval
495+
expect(errors[2].retryAfter).toBe(300000);
496+
// Subsequent errors stay at reloadInterval
497+
expect(errors[3].retryAfter).toBe(300000);
498+
});
499+
500+
it("should fall back to reloadInterval after server error retries exhausted", async () => {
501+
server.use(
502+
http.get(TEST_URL, () => {
503+
return new HttpResponse(null, { status: 503 });
504+
})
505+
);
506+
507+
fetcher = new HTTPFetcher(TEST_URL, { reloadInterval: 300000, maxRetries: 3 });
508+
509+
const errors = [];
510+
fetcher.on("error", (errorInfo) => errors.push(errorInfo));
511+
512+
for (let i = 0; i < 4; i++) {
513+
await fetcher.fetch();
514+
}
515+
516+
// First retries should use backoff (< reloadInterval)
517+
expect(errors[0].retryAfter).toBe(15000);
518+
expect(errors[1].retryAfter).toBe(30000);
519+
// Third retry hits maxRetries, should fall back to reloadInterval
520+
expect(errors[2].retryAfter).toBe(300000);
521+
// Subsequent errors stay at reloadInterval
522+
expect(errors[3].retryAfter).toBe(300000);
523+
});
524+
525+
it("should reset network error count on success", async () => {
526+
let requestCount = 0;
527+
server.use(
528+
http.get(TEST_URL, () => {
529+
requestCount++;
530+
if (requestCount <= 2) return HttpResponse.error();
531+
return HttpResponse.text("ok");
532+
})
533+
);
534+
535+
fetcher = new HTTPFetcher(TEST_URL, { reloadInterval: 300000, maxRetries: 3 });
536+
537+
const errors = [];
538+
fetcher.on("error", (errorInfo) => errors.push(errorInfo));
539+
540+
// Two failures with backoff
541+
await fetcher.fetch();
542+
await fetcher.fetch();
543+
expect(errors).toHaveLength(2);
544+
expect(errors[0].retryAfter).toBe(15000);
545+
expect(errors[1].retryAfter).toBe(30000);
546+
547+
// Success resets counter
548+
await fetcher.fetch();
549+
expect(fetcher.networkErrorCount).toBe(0);
550+
});
551+
});

0 commit comments

Comments
 (0)