Skip to content

Send limits when getting rate limited#4102

Merged
aibrahim-oai merged 9 commits intomainfrom
send-limits
Sep 23, 2025
Merged

Send limits when getting rate limited#4102
aibrahim-oai merged 9 commits intomainfrom
send-limits

Conversation

@aibrahim-oai
Copy link
Copy Markdown
Collaborator

Users need visibility on rate limits when they are rate limited.

}

if status == StatusCode::TOO_MANY_REQUESTS {
let rate_limit_snapshot = parse_rate_limit_snapshot(res.headers());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we put this and event firing above the check for errors?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but we send it in the error body. We don't have a channel in case of error to fire it.

secondary_used_percent: f64,
primary_used_percent: f64,
) -> Vec<String> {
if secondary_used_percent == 100.0 || primary_used_percent == 100.0 {
Copy link
Copy Markdown
Collaborator

@pakrym-oai pakrym-oai Sep 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no warnings for completely used limits?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already error on rate limit. Trying to prevent that:

image

Comment thread codex-rs/core/tests/suite/client.rs Outdated
let codex_fixture = builder
.build(&server)
.await
.expect("build codex conversation");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and anyhow error

Comment thread codex-rs/core/src/codex.rs Outdated
}

async fn send_token_count_event(&self, sub_id: &str) {
let token_event = self.get_token_count_event().await;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we merge in the get_token_count_event ?

@aibrahim-oai aibrahim-oai enabled auto-merge (squash) September 23, 2025 21:31
@aibrahim-oai aibrahim-oai merged commit 8227a5b into main Sep 23, 2025
19 checks passed
@aibrahim-oai aibrahim-oai deleted the send-limits branch September 23, 2025 22:56
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 23, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants