Skip to content

fix: preserve numRetries = 0 - #373

Merged
tharropoulos merged 1 commit into
typesense:masterfrom
johnemau:johnemau/fix-num-retries-zero
Sep 24, 2026
Merged

tharropoulos merged 1 commit into
typesense:masterfrom
johnemau:johnemau/fix-num-retries-zero

Conversation

@johnemau

Copy link
Copy Markdown
Contributor

Addresses #372

numRetries: 0 became 3 because || 3 replaced the ternary's falsy result. A timed-out request was attempted four times instead of once. Removing the wrapping parentheses preserves 0 and applies the fallback only to the default branch, which validate() guarantees is at least 1.

Validation

New unit tests cover these numRetries inputs:

Input Expected
0 0
1, 2, 10 same value
-1 node count (falls back to default)
undefined / omitted node count
undefined with nearestNode node count + 1

The zero case fails on master with expected 3 to be +0 and passes with the fix. Against a local Typesense 29.0 server, the full suite had 158 passed, 32 skipped, and 0 failed. eslint . reports 0 errors and 8 pre-existing warnings in untouched files; tsc --noEmit -p tsconfig.test.json is clean.

🤖 Generated with Claude Code

Generated with AI under the guidance of John.

The `|| 3` fallback wrapped the whole ternary, so an explicit
`numRetries: 0` was replaced with 3. Bind it to the default branch only.

Adds Configuration tests covering explicit zero, positive, negative, and
undefined numRetries, with and without nearestNode.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@johnemau
johnemau marked this pull request as ready for review September 24, 2026 15:19
Copilot AI lite review requested due to automatic review settings September 24, 2026 15:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The fix and regression coverage address the reported behavior with no unresolved issues.

Review effort: Lite
Findings: None

What changed in this PR

Fixes numRetries: 0 being converted to 3 and adds regression coverage.

Changes:

  • Preserves explicit zero retry counts.
  • Tests zero, positive, negative, undefined, and nearest-node cases.
File Summary
test/​Typesense/​Configuration.spec.ts Adds comprehensive retry-count regression tests.
src/​Typesense/​Configuration.ts Corrects retry fallback precedence.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@tharropoulos

Copy link
Copy Markdown
Collaborator

Thank you for catching this!

@tharropoulos
tharropoulos merged commit e1eff35 into typesense:master Sep 24, 2026
1 check passed
@johnemau

Copy link
Copy Markdown
Contributor Author

Thank you for catching this!

No problem and thank you for merging my fix so quickly @tharropoulos!

Do you have an idea when we can expect the next release?

P.S. Thank you for your work!!

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.

3 participants