fetch: add retry/backoff and endpoint resilience#21
Conversation
![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: 982f66e1af
ℹ️ 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".
| fn overpass_endpoints(config: &NetworkConfig) -> Vec<String> { | ||
| if config.overpass_endpoints.is_empty() { | ||
| vec![config.overpass_url.clone()] | ||
| } else { | ||
| config.overpass_endpoints.clone() |
There was a problem hiding this comment.
overpass_url when callers keep the default endpoint list
NetworkConfig still exposes overpass_url as a public field, so downstream code can validly do let mut cfg = NetworkConfig::default(); cfg.overpass_url = mirror.into(); or NetworkConfig { overpass_url: ..., ..Default::default() }. After this change, overpass_endpoints() ignores that override whenever overpass_endpoints is non-empty, and Default now always pre-populates the list with overpass-api.de. That creates a silent regression for existing custom-endpoint users: requests continue going to the default host unless they learn to update both fields.
Useful? React with 👍 / 👎.
| return response | ||
| .bytes() | ||
| .await | ||
| .map(|bytes| bytes.to_vec()) | ||
| .map_err(|error| { | ||
| RoutingError::Network(format!( | ||
| "Overpass response body read failed from {} on attempt {}: {}", | ||
| endpoint, | ||
| attempt + 1, | ||
| error | ||
| )) | ||
| }); |
There was a problem hiding this comment.
Once an endpoint returns HTTP 200, this branch immediately returns the result of response.bytes().await. If the body download itself times out or the connection is reset mid-stream—which can happen on large Overpass payloads under the configured read_timeout—the function exits here and never reaches the retry loop or the secondary endpoints. In those cases the new resilience logic still fails on the first mirror instead of retrying/failing over.
Useful? React with 👍 / 👎.
1 participant
Summary
Overpass fetches still used a single endpoint and a single request attempt, so transient rate limits or endpoint outages failed the whole load with limited context.
This change adds configurable resilience to the live fetch path:
NetworkConfignow supports endpoint pools, retry count, and retry backoffValidation
Fixes #8