Skip to content

Commit 2ee1dcf

Browse files
committed
fix: robustify searchTimeEntriesPage retries
- Wrap the transport fetch and the JSON parse in try/catch so network failures (DNS/socket) and parse errors now retry with backoff, not just HTTP statuses. HTTP-error throws (402 with context, generic non-OK) stay outside the try so they propagate immediately (Copilot). - Separate 402 retry budget from the 429/network retry budget. 429 needs a larger budget (real rate-limit backoff); 402 is usually a genuine premium gate so should retry at most once. maxAttempts=3, max402Retries=1 (Copilot). - Drop the `delete filters[k]` loop in `toggl_search_time_entries`; JSON.stringify already omits undefined-valued properties, so the loop was dead weight and its comment was also inaccurate (Copilot).
1 parent 87037fa commit 2ee1dcf

File tree

2 files changed

+34
-11
lines changed

2 files changed

+34
-11
lines changed

src/index.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -567,10 +567,6 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => {
567567
rounding_minutes: args?.rounding_minutes as number | undefined,
568568
page_size: args?.page_size as number | undefined,
569569
};
570-
// Strip undefined keys so Reports API doesn't see nulls where we mean "unset".
571-
for (const k of Object.keys(filters) as (keyof TimeEntrySearchFilters)[]) {
572-
if (filters[k] === undefined) delete filters[k];
573-
}
574570

575571
const entries = await api.searchTimeEntries(workspaceId as number, filters, {
576572
maxPages: (args?.max_pages as number | undefined) ?? 20,

src/toggl-api.ts

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -284,12 +284,28 @@ export class TogglAPI {
284284
const body = JSON.stringify(filters);
285285
// 402 from the Reports API is issued both for genuinely premium-gated
286286
// features (e.g. the `billable` filter on a Free workspace) AND as a
287-
// transient server-side glitch that clears on retry. Retry once; if it
288-
// still 402s, surface a message that covers both possibilities.
289-
const maxAttempts = 2;
287+
// transient server-side glitch that clears on retry. A true 402 is not
288+
// worth many retries (real premium gate, wasted latency), but transport
289+
// failures and 429 rate limits need a larger budget — so they are
290+
// tracked on separate counters.
291+
const maxAttempts = 3; // network/429 budget
292+
const max402Retries = 1; // 402-specific retry budget
293+
let attempts402 = 0;
290294

291295
for (let attempt = 0; attempt < maxAttempts; attempt++) {
292-
const response = await fetch(url, { method: 'POST', headers: this.headers, body });
296+
// Only the transport fetch and JSON parse are wrapped so that
297+
// deliberate HTTP-error throws below propagate out of the loop.
298+
let response: Awaited<ReturnType<typeof fetch>>;
299+
try {
300+
response = await fetch(url, { method: 'POST', headers: this.headers, body });
301+
} catch (error) {
302+
if (attempt >= maxAttempts - 1) throw error;
303+
const delay = (attempt + 1) * 1000;
304+
const message = error instanceof Error ? error.message : String(error);
305+
console.error(`Reports API request failed (attempt ${attempt + 1}/${maxAttempts}): ${message}. Retrying after ${delay}ms...`);
306+
await new Promise(r => setTimeout(r, delay));
307+
continue;
308+
}
293309

294310
if (response.status === 429) {
295311
const retryAfterRaw = response.headers.get('Retry-After');
@@ -300,8 +316,9 @@ export class TogglAPI {
300316
continue;
301317
}
302318

303-
if (response.status === 402 && attempt < maxAttempts - 1) {
304-
console.error(`Reports API returned 402 (attempt ${attempt + 1}/${maxAttempts}); retrying after 500ms...`);
319+
if (response.status === 402 && attempts402 < max402Retries) {
320+
attempts402++;
321+
console.error(`Reports API returned 402 (402 retry ${attempts402}/${max402Retries}); retrying after 500ms...`);
305322
await new Promise(r => setTimeout(r, 500));
306323
continue;
307324
}
@@ -319,7 +336,17 @@ export class TogglAPI {
319336
throw new Error(`Reports API error (${response.status}): ${text}`);
320337
}
321338

322-
const rows = (await response.json()) as ReportsSearchRow[];
339+
let rows: ReportsSearchRow[];
340+
try {
341+
rows = (await response.json()) as ReportsSearchRow[];
342+
} catch (error) {
343+
if (attempt >= maxAttempts - 1) throw error;
344+
const delay = (attempt + 1) * 1000;
345+
const message = error instanceof Error ? error.message : String(error);
346+
console.error(`Reports API response parse failed (attempt ${attempt + 1}/${maxAttempts}): ${message}. Retrying after ${delay}ms...`);
347+
await new Promise(r => setTimeout(r, delay));
348+
continue;
349+
}
323350
const nextIdHeader = response.headers.get('X-Next-ID');
324351
const nextRowHeader = response.headers.get('X-Next-Row-Number');
325352
return {

0 commit comments

Comments
 (0)