fix(collector): retry Gamma closed discovery after 500 - #626
Conversation
📝 WalkthroughWalkthroughClosed-lane Gamma discovery now retries tagged HTTP 500 responses and performs one untagged fallback request. Integration tests verify request bounds, parameter removal, target filtering, and fail-closed behavior. ChangesClosed-lane Gamma discovery
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Collector
participant GammaServer
participant TargetFilter
Collector->>GammaServer: Send tagged closed-lane request
GammaServer-->>Collector: Return HTTP 500
Collector->>GammaServer: Retry tagged request
GammaServer-->>Collector: Return HTTP 500
Collector->>GammaServer: Send untagged fallback request
GammaServer-->>Collector: Return market data
Collector->>TargetFilter: Apply supported-market checks
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2eb81a7d53
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| && error | ||
| .downcast_ref::<reqwest::Error>() | ||
| .and_then(reqwest::Error::status) | ||
| == Some(reqwest::StatusCode::INTERNAL_SERVER_ERROR) => |
There was a problem hiding this comment.
When the first closed-lane response is HTTP 500 with a malformed or over-budget textual Retry-After, get_json returns the contextualized reqwest::Error immediately from the retry-delay check rather than exhausting HTTP_GET_ATTEMPTS; this status-only guard still downcasts that source error and launches the untagged query. That bypasses the existing bounded-retry/fail-closed behavior despite the change contract limiting fallback to exhausted retries, so distinguish terminal retry exhaustion from an unhonorable Retry-After and cover the latter as a counterexample.
AGENTS.md reference: AGENTS.md:L113-L116
Useful? React with 👍 / 👎.
| let fallback_params = params | ||
| .iter() | ||
| .filter(|(key, _)| key != "tag_id") | ||
| .cloned() |
There was a problem hiding this comment.
When the closed tagged call exhausts with HTTP 500, this removes the only category-level filter before querying Gamma, while the remaining target_market check is not crypto-only: infer_symbol performs unrestricted substring matching (for example, RESOLVE contains the SOL alias) and accepts any binary market lasting exactly 5 or 15 minutes. Consequently, an unrelated untagged short-duration market can be recorded and polled as SOLUSDT whenever the fallback runs, contaminating the reference tape; retain an equivalent category or canonical market-pattern check before admitting the broader results.
AGENTS.md reference: AGENTS.md:L70-L74
Useful? React with 👍 / 👎.
1 participant
Change contract
Retry a Gamma discovery request without
tag_id=21only when the closed lane exhausts its existing bounded HTTP retries with HTTP 500. Keep the same date window, cursor, target-market admission checks, and fail-closed behavior for every other error.Acceptance evidence
Out of scope
Dependency / merge order
No stacked dependency. Base is
mainat612d0f9a57130d9f708f055d625b83b6839a1816, which already contains merged PR #620.Focused validation
cargo test -p hft-collector --lib --locked— 280 passed, 2 ignoredcargo clippy -p hft-collector --all-targets --features collector-binance --no-deps --locked -- -D warningsdeployment/aliyun/test-polymarket-raw-ops-control-plane.shgit diff --checkRollout / rollback impact
No automatic deployment. After merge, bind the new immutable candidate in the runtime control contract, establish a fresh reference-health baseline, and run the existing Gate before any cutover. Roll back by retaining the current runtime or reverting this single commit; no tape or OSS data is deleted by this change.
Issue relationship
Closes #625