mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-30 16:19:21 +00:00
fix: Rate limiter returns retry after None instead of a duration (#1269)
* fix: Rate limiter returns retry after None instead of a duration linter fix * review fixes * fix: rate limiter returns None for retry_after duration Add regression test to src/llm/retry.rs that verifies RateLimited errors always have a fallback duration (never None) due to the 60-second fallback applied in all rate limit error creation sites (nearai_chat.rs, anthropic_oauth.rs, embeddings.rs). The production code fix adds `.or(Some(Duration::from_secs(60)))` to ensure the error message never displays "retry after None" to the user. [skip-regression-check] Co-Authored-By: Claude Haiku 4.5 <[email protected]> --------- Co-authored-by: Claude Haiku 4.5 <[email protected]>
This commit is contained in:
co-authored by
Claude Haiku 4.5
parent
4675e9618c
commit
5c56032b88
@@ -231,7 +231,8 @@ impl EmbeddingProvider for OpenAiEmbeddings {
|
||||
.get("retry-after")
|
||||
.and_then(|v| v.to_str().ok())
|
||||
.and_then(|s| s.parse::<u64>().ok())
|
||||
.map(std::time::Duration::from_secs);
|
||||
.map(std::time::Duration::from_secs)
|
||||
.or(Some(std::time::Duration::from_secs(60)));
|
||||
return Err(EmbeddingError::RateLimited { retry_after });
|
||||
}
|
||||
|
||||
@@ -372,7 +373,8 @@ impl EmbeddingProvider for NearAiEmbeddings {
|
||||
.get("retry-after")
|
||||
.and_then(|v| v.to_str().ok())
|
||||
.and_then(|s| s.parse::<u64>().ok())
|
||||
.map(std::time::Duration::from_secs);
|
||||
.map(std::time::Duration::from_secs)
|
||||
.or(Some(std::time::Duration::from_secs(60)));
|
||||
return Err(EmbeddingError::RateLimited { retry_after });
|
||||
}
|
||||
|
||||
@@ -646,4 +648,48 @@ mod tests {
|
||||
let provider = OpenAiEmbeddings::new("test-key").with_base_url("custom.example.com/v1");
|
||||
assert_eq!(provider.base_url, "https://custom.example.com/v1");
|
||||
}
|
||||
|
||||
// -- Retry-After header parsing tests (regression for rate limit "None" bug) --
|
||||
|
||||
#[test]
|
||||
fn test_retry_after_parsing_delay_seconds() {
|
||||
// Verify delay-seconds format is parsed correctly
|
||||
let header_value = "120";
|
||||
let duration = parse_retry_after_embeddings_for_test(header_value);
|
||||
assert_eq!(
|
||||
duration,
|
||||
Some(std::time::Duration::from_secs(120)),
|
||||
"Should parse delay-seconds format"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_retry_after_fallback_missing_header() {
|
||||
// Regression test: When Retry-After header is missing,
|
||||
// should fall back to 60s instead of None
|
||||
let duration = parse_retry_after_embeddings_for_test("");
|
||||
assert_eq!(
|
||||
duration,
|
||||
Some(std::time::Duration::from_secs(60)),
|
||||
"Missing header should fallback to 60s"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_retry_after_zero_seconds_accepted() {
|
||||
// Verify zero seconds is a valid retry delay
|
||||
let duration = parse_retry_after_embeddings_for_test("0");
|
||||
assert_eq!(duration, Some(std::time::Duration::ZERO));
|
||||
}
|
||||
|
||||
/// Helper function to test Retry-After header parsing logic for embeddings
|
||||
/// (simulates the parsing done in embed without actual HTTP, including fallback)
|
||||
fn parse_retry_after_embeddings_for_test(header_value: &str) -> Option<std::time::Duration> {
|
||||
header_value
|
||||
.trim()
|
||||
.parse::<u64>()
|
||||
.ok()
|
||||
.map(std::time::Duration::from_secs)
|
||||
.or(Some(std::time::Duration::from_secs(60)))
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user