fix(http): enforce documented retry contracts - #55
Conversation
|
Warning Review limit reached
Next review available in: 10 minutes Limit details: You’ve used all 10 included reviews currently available. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Comment |
Greptile SummaryThis PR narrows built-in transient HTTP status handling to 500–599, 408, and 429, and adds explicit tests and documentation for the HTTP resilience contracts.
Confidence Score: 4/5The production change appears safe to merge, with only a non-blocking risk of a scheduler-dependent test timeout. The status-classification implementation and documented contracts are consistent, but the new attempt-timeout test can miss asynchronously registered fake timers because it substitutes short real-time polling for deterministic synchronization. Files Needing Attention: tests/Kevlar.Tests/HttpContractTests.cs
|
| Filename | Overview |
|---|---|
| src/Kevlar.Extensions.Http/HttpShield.cs | Correctly restricts built-in transient status classification to the documented HTTP ranges. |
| tests/Kevlar.Tests/HttpContractTests.cs | Adds broad HTTP contract coverage, with one scheduler-sensitive fake-time wait that can make CI flaky. |
| docs/docs/http.md | Accurately clarifies response ownership, request replay limitations, and shield sharing across handler lifetimes. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[HTTP request] --> B[30s total timeout]
B --> C[Retry up to 3 times]
C --> D[Circuit breaker]
D --> E[10s attempt timeout]
E --> F[HTTP handler]
F --> G{Transient outcome?}
G -->|500-599, 408, 429 or handled exception| C
G -->|No or retries exhausted| H[Caller-owned final response]
Reviews (1): Last reviewed commit: "fix(http): bound transient status codes" | Re-trigger Greptile
Lock HTTP retry, disposal, replay, cancellation, registration, and Standard pipeline contracts with deterministic tests. Clarify ownership and one-shot stream semantics in the docs.
2e5c920 to
e292ef5
Compare
Summary
Closes #26
Validation