Harden retry/timeout classification on structured HTTP status and bra…

[jonathanMLDev]

Summary

Retry and timeout classification no longer depends primarily on error-message regex. Transient failures are detected from structured HTTP status, Pinecone SDK error names, and a branded AppTimeoutError before falling back to network-ish message patterns.

• Add AppTimeoutError (thrown by withTimeout) and strengthen isAppTimeoutError via instanceof, cause-chain walk, and legacy message-prefix fallback
• Add forEachErrorInChain, getHttpStatus, and isRetryableHttpStatus to extract status from status/statusCode, Pinecone error names, and name-gated SDK message patterns
• Reorder defaultShouldRetry: reject app timeouts first, then structured status → PineconeConnectionError → message-regex last resort
• Expand unit and hot-path tests for reworded 429, SDK-shaped PineconeUnmappedHttpError, AppTimeoutError non-retry, and classifyToolCatchError → TIMEOUT

Classification-only change: retry counts, backoff defaults, and requestTimeoutMs wiring are unchanged.

Closes #230

Test plan

• npm run ci (typecheck, lint, format, build, benchmark smoke, test:coverage)
• retry.test.ts — structured { status: 429 }, PineconeUnmappedHttpError + Status: 429, AppTimeoutError not retried via defaultShouldRetry
• search.test.ts / rerank.test.ts — reworded 429 retried through runWithPolicy; search timeout rejects AppTimeoutError
• tool-error.test.ts / query-tool.context.test.ts — AppTimeoutError maps to MCP TIMEOUT
• Existing message-based retry cases remain green (network fallback preserved)

Summary by CodeRabbit

• Bug Fixes
  • Improved retry behavior for structured HTTP errors (including rate-limit responses) even when the message text doesn’t contain the status code.
  • More reliable app-level timeout detection: timeouts are no longer retried, even when errors are wrapped or chained.
  • Tool timeout classification is now consistent, with recovery marked as retryable and guidance aligned to timeout/retry actions.
• Tests
  • Added/updated coverage for structured 429 retries, non-retryable app timeouts, and TIMEOUT tool-error classification for both legacy and branded timeout errors.

[coderabbitai]

Warning

Review limit reached

@jonathanMLDev, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 35 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c792b6bd-8e7f-4f81-9847-f47a436483e5

📥 Commits

Reviewing files that changed from the base of the PR and between 4fa6ca7 and 4659791.

📒 Files selected for processing (7)

• src/core/pinecone-client.test.ts
• src/core/pinecone/indexes.test.ts
• src/core/pinecone/rerank.test.ts
• src/core/pinecone/search.test.ts
• src/core/pinecone/test-helpers.ts
• src/core/server/retry.test.ts
• src/core/server/retry.ts

📝 Walkthrough

Walkthrough

Retry and timeout handling now uses structured HTTP status extraction and branded application timeout errors. Pinecone search and rerank tests cover structured 429 retries, while server and query-tool tests verify timeout classification.

Changes

Retry and timeout classification

Layer / File(s)	Summary
Error-chain and HTTP status inspection src/core/server/retry.ts	Adds bounded cause-chain traversal, HTTP status extraction, Pinecone status mappings, retryable status checks, and branded timeout detection.
Retry predicates and timeout rejection src/core/server/retry.ts, src/core/server/retry.test.ts	Prioritizes application timeout exclusion and structured 429/5xx retry checks; withTimeout now rejects with AppTimeoutError.
Pinecone and tool timeout validation src/core/pinecone/*test.ts, src/core/server/tool-error.test.ts, src/core/server/tools/query-tool.context.test.ts	Tests structured 429 retries and branded or legacy timeout classification across Pinecone and tool handlers.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant PineconeOperation
  participant transientShouldRetry
  participant defaultShouldRetry
  participant AppTimeoutError
  PineconeOperation->>transientShouldRetry: evaluate thrown error
  transientShouldRetry->>defaultShouldRetry: evaluate non-timeout error
  defaultShouldRetry-->>transientShouldRetry: retry structured 429 or 5xx
  transientShouldRetry-->>PineconeOperation: retry or propagate
  AppTimeoutError-->>transientShouldRetry: do not retry application timeout

Loading

Possibly related PRs

• cppalliance/pinecone-read-only-mcp-typescript#225: Updates the shared retry/timeout machinery and Pinecone retry tests around structured HTTP status failures.

Suggested reviewers: wpak-ai, auramindnest, leostar0412

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: hardening retry and timeout classification using structured HTTP status.
Linked Issues check	✅ Passed	The PR aligns with #230 by prioritizing structured 429/5xx retry, adding branded app-timeout handling, preserving fallback behavior, and covering hot-path tests.
Out of Scope Changes check	✅ Passed	The changes stay focused on retry and timeout classification plus related tests, with no clear unrelated scope added.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/core/server/retry.ts`:
- Around line 163-181: Update defaultShouldRetry so any defined HTTP status
returns the result of isRetryableHttpStatus immediately, including non-retryable
statuses; only run hasTransientPineconeErrorName and message-regex fallbacks
when getHttpStatus returns undefined. Add a regression test in retry.test.ts
covering a structured non-retryable status with a retryable-looking message and
assert it returns false.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1140755d-1399-4265-95dc-376b250e7ba7

📥 Commits

Reviewing files that changed from the base of the PR and between 1bdb56a and cca072f.

📒 Files selected for processing (6)

• src/core/pinecone/rerank.test.ts
• src/core/pinecone/search.test.ts
• src/core/server/retry.test.ts
• src/core/server/retry.ts
• src/core/server/tool-error.test.ts
• src/core/server/tools/query-tool.context.test.ts

[codecov]

Codecov Report

❌ Patch coverage is 98.64865% with 1 line in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@1bdb56a). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
src/core/server/retry.ts	98.50%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #235   +/-   ##
=======================================
  Coverage        ?   86.34%           
=======================================
  Files           ?       48           
  Lines           ?     2599           
  Branches        ?      875           
=======================================
  Hits            ?     2244           
  Misses          ?      353           
  Partials        ?        2           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jonathanMLDev]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.