Skip to content

fix: off-by-one in retryOn — retries: 0 still retries once#14

Merged
StefanoGuerrini merged 1 commit into
ZenRows:mainfrom
jorgehermo9:fix/off-by-one-retries
Apr 21, 2026
Merged

fix: off-by-one in retryOn — retries: 0 still retries once#14
StefanoGuerrini merged 1 commit into
ZenRows:mainfrom
jorgehermo9:fix/off-by-one-retries

Conversation

@jorgehermo9

@jorgehermo9 jorgehermo9 commented Apr 6, 2026

Copy link
Copy Markdown
Contributor

Closes #15

Bug

The retryOn callback in the constructor uses attempt > retries to decide whether to stop retrying. However, fetch-retry starts counting attempts at 0, so with the default retries: 0:

  • attempt=0: 0 > 0false → enters retry logic → retries once
  • attempt=1: 1 > 0true → stops

This means retries: 0 (the default) actually performs 1 retry, making 2 fetch calls total. The README states "retries are not active by default", which contradicts this behavior.

Fix

Change attempt > retries to attempt >= retries in the retryOn callback (one character change in src/index.ts).

With >=, retries: 0 now correctly means zero retries:

  • attempt=0: 0 >= 0true → stops immediately → no retry

Tests

Added parameterized tests (tests/retry-count.test.ts) verifying exact fetch call counts:

retries Expected fetch calls Behavior
0 1 No retry (default)
1 2 1 retry
2 3 2 retries

All tests pass with the fix and fail without it.

The retryOn callback used `attempt > retries`, but fetch-retry starts
counting at attempt=0. With the default `retries: 0`, the check
`0 > 0` is false, so the SDK enters the retry logic and retries once.

Change to `attempt >= retries` so that `retries: 0` means zero retries,
matching the README which states "retries are not active by default".

Add parameterized tests verifying exact fetch call counts for
retries: 0, 1, and 2.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jorgehermo9

Copy link
Copy Markdown
Contributor Author

hey @AnderRV, is this repo still being maintained? We are customers, and we are facing this bug. We workaround it setting retries: -1 in our client instance. I see you have issues not available, so I created this PR. Feel free to close if this is unmaintained.

@StefanoGuerrini

Copy link
Copy Markdown
Contributor

hey @AnderRV, is this repo still being maintained? We are customers, and we are facing this bug. We workaround it setting retries: -1 in our client instance. I see you have issues not available, so I created this PR. Feel free to close if this is unmaintained.

Hi @jorgehermo9, thanks for the thorough write-up and the fix.

Yes, the repo is maintained, apologies that Issues were turned off, we'll re-enable them.
We're going to merge this as-is and ship it in the next release.

Thanks again for the contribution!

@StefanoGuerrini

Copy link
Copy Markdown
Contributor

@jorgehermo9 quick heads up: your fix is live on npm as zenrows@2.0.2. Release notes: https://github.com/ZenRows/zenrows-node-sdk/releases/tag/v2.0.2

Thanks again, much appreciated 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Default retries: 0 still performs one retry on 422/503/504

2 participants