Skip to content

Commit 5f49d5b

Browse files
committed
Address second round of review feedback
- Replace Number.isFinite with Number.isInteger in concurrentMap validation: floats like 1.5 now correctly throw RangeError (isInteger implies finite, covers NaN/Infinity/floats/negatives in a single check) - Add test: concurrency=1.5 throws RangeError - Pass maxRetries=0 to fetchWithRetry for raw file fetches so AbortSignal.timeout acts as a strict hard cap; retry delays in fetchWithRetry are not abort-aware and would otherwise let a timed-out request silently retry for up to 30 s See: #100
1 parent c00b14d commit 5f49d5b

3 files changed

Lines changed: 14 additions & 5 deletions

File tree

src/api-utils.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,4 +364,10 @@ describe("concurrentMap", () => {
364364
RangeError,
365365
);
366366
});
367+
368+
it("throws RangeError when concurrency is a float (e.g. 1.5)", async () => {
369+
await expect(concurrentMap([1], async (n) => n, { concurrency: 1.5 })).rejects.toThrow(
370+
RangeError,
371+
);
372+
});
367373
});

src/api-utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ export async function concurrentMap<T, R>(
125125
fn: (item: T, index: number) => Promise<R>,
126126
{ concurrency = 20 }: { concurrency?: number } = {},
127127
): Promise<R[]> {
128-
if (!Number.isFinite(concurrency) || concurrency < 1) {
128+
if (!Number.isInteger(concurrency) || concurrency < 1) {
129129
throw new RangeError(
130130
`concurrentMap: concurrency must be a positive integer, got ${concurrency}`,
131131
);

src/api.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -229,10 +229,13 @@ export async function fetchAllResults(
229229
urlsToFetch,
230230
async (htmlUrl) => {
231231
try {
232-
const res = await fetchWithRetry(toRawUrl(htmlUrl), {
233-
headers: { Authorization: `Bearer ${token}` },
234-
signal: AbortSignal.timeout(5_000),
235-
});
232+
// Disable retries (maxRetries=0) so the 5 s AbortSignal.timeout acts
233+
// as a hard cap — retry delays in fetchWithRetry are not abort-aware.
234+
const res = await fetchWithRetry(
235+
toRawUrl(htmlUrl),
236+
{ headers: { Authorization: `Bearer ${token}` }, signal: AbortSignal.timeout(5_000) },
237+
0,
238+
);
236239
if (res.ok) fileContentMap.set(htmlUrl, await res.text());
237240
} catch {
238241
// Fall back to fragment-relative line numbers (timeout or network error)

0 commit comments

Comments
 (0)