Skip to content
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

fix(http): Explain why we responded with 429 #1389

Merged
merged 3 commits into from
Aug 9, 2022

Conversation

untitaker
Copy link
Member

@untitaker untitaker commented Aug 8, 2022

fix getsentry/sentry#37103

Oftentimes we are not in control of the HTTP response (load balancers and relay may reject the request for a variety of reasons, and it would probably be quite a bit of effort to add docs links to all of those return paths), but if we
are, we can provide a more helpful error message.

The context here is that a customer was confused about whether the
network errors in Chrome's dev console would have any impact on their
application. While we can't influence the color of what is shown in the
dev console, we can provide documentation links in request/response
details. This will still be quite hidden but better than nothing.

Other ideas were to stop emitting 429s, but we could only do that for
browser, mobile and other "client" SDKs. For server-side SDKs we need
them to see a 429 so they stop sending data. And even then that change
would come with high risk of multiplying inbound traffic by a large
factor.

fix getsentry/sentry#37103

Oftentimes we are not in control of the rate limit response, but if we
are, we can provide a more helpful error message.

The context here is that a customer was confused about whether the
network errors in Chrome's dev console would have any impact on their
application. While we can't influence the color of what is shown in the
dev console, we can provide documentation links in request/response
details. This will still be quite hidden but better than nothing.

Other ideas were to stop emitting 429s, but we could only do that for
browser, mobile and other "client" SDKs. For server-side SDKs we _need_
them to see a 429 so they stop sending data. And even then that change
would come with high risk of multiplying inbound traffic by a large
factor.
@untitaker untitaker requested a review from a team August 8, 2022 13:09
relay-server/src/endpoints/common.rs Outdated Show resolved Hide resolved
@untitaker untitaker merged commit 4240551 into master Aug 9, 2022
@untitaker untitaker deleted the fix/add-documentation-link branch August 9, 2022 18:29
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.

Provide a status text containing the reason for HTTP 429
4 participants