Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add http header Retry-After to error 429 response #15840

Closed
ThomasHalwax opened this issue Jun 26, 2023 · 2 comments · Fixed by #16136
Closed

Add http header Retry-After to error 429 response #15840

ThomasHalwax opened this issue Jun 26, 2023 · 2 comments · Fixed by #16136
Labels
O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Blocks non-critical functionality, workarounds exist. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. Z-Spec-Blocked This change is blocked on specification (e.g. an MSC).

Comments

@ThomasHalwax
Copy link

ThomasHalwax commented Jun 26, 2023 •
edited
Loading

Description:

I'm using Ky to make calls to the Matrix API. Whenever I receive a rate-limited error with status code 429 I'm forced to parse the body of the error in order to retrieve the value for retry_after_ms before doing the next request.

Ky has a built in retry mechanism and honors the http header Retry-After that represents either an absolute date/time or a delay in seconds.

Can you please add the appropriate http header to 429 errors? I'm using Synapse v1.82 in a dockerized setup.

Thanks,
Thomas
@clokep clokep added S-Minor Blocks non-critical functionality, workarounds exist. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. O-Occasional Affects or can be seen by some users regularly or most users rarely labels Jun 27, 2023
@MadLittleMods MadLittleMods added Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution and removed Z-Help-Wanted We know exactly how to fix this issue, and would be grateful for any contribution labels Jun 28, 2023
@MadLittleMods
Copy link
Contributor

It looks like this was suggested before in #7138 where it was suggested that this needs to be spec'ed out or in a MSC before we can move forward. Seems like a nice short MSC to write so we can use the standardized header if you're interested.

Also mentioned in matrix-org/matrix-spec#44 (comment)

@MadLittleMods MadLittleMods added the Z-Spec-Blocked This change is blocked on specification (e.g. an MSC). label Jun 28, 2023
@ThomasHalwax
Copy link
Author

@MadLittleMods I have written a MSC proposal Use http header Retry-After to enable library-assisted retry handling to introduce the http header Retry-After. Would you please read it? It is my first MSC and I would be very happy about feedback.

Thanks!

@clokep clokep mentioned this issue Aug 21, 2023
Merged
3 tasks
DMRobertson pushed a commit that referenced this issue Aug 29, 2023
Merge remote-tracking branch 'origin/develop' into dmr/ratelimit-name…
7c681a1 
…-in-err

In particular this conflicted with #15840
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Blocks non-critical functionality, workarounds exist. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. Z-Spec-Blocked This change is blocked on specification (e.g. an MSC).
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add Retry-After to M_LIMIT_EXCEEDED error responses matrix-org/synapse
3 participants
@clokep @MadLittleMods @ThomasHalwax