Skip to content

Return typed ratelimit error including retry-after header #1671

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

0xmichalis
Copy link

Fixes #1668

src/api/api.ts Outdated
@@ -755,6 +756,19 @@ export class OpenSeaAPI {

const response = await req.send();
if (!response.ok()) {
if (response.statusCode === 599) {
Copy link
Author

Choose a reason for hiding this comment

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

I could also use the standard 429 status code here to make the clause future-proof.

Copy link
Collaborator

Choose a reason for hiding this comment

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

429 makes sense, not sure if it should be catching 599 for rate limit?

Copy link
Author

Choose a reason for hiding this comment

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

This is the status code currently returned from the API (likely a proxy under your control sitting in front of the actual API).

@ryanio
Copy link
Collaborator

ryanio commented Jun 12, 2025

thanks for this PR! it's much appreciated, and sorry it took me so long to review. i'm concerned it might break people's implementations though by throwing a different error if we release it in a patch version. what is the current error response when you get rate limited? i do think it's useful to return the retryAfter data, but maybe we can just add it to the end of the existing error message and it can be grepped? not ideal, but just one thought.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include retry-after info on 429 errors
2 participants