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.

Sign up for GitHub

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

Jump to bottom

MSC4041: http header Retry-After for http code 429 #4041

Merged
merged 12 commits into from
Feb 19, 2024
Merged

MSC4041: http header Retry-After for http code 429 #4041

merged 12 commits into from
Feb 19, 2024

Conversation

ThomasHalwax
Copy link
Contributor

@ThomasHalwax ThomasHalwax commented Aug 10, 2023 •
edited by richvdh
Loading

Rendered

This proposes the usage of the http header Retry-After in case a home server sends a http response 429 (rate limiting).

The Retry-After header handling was implemented in

FCP tickyboxes

@ThomasHalwax ThomasHalwax mentioned this pull request Aug 10, 2023
Closed
@ThomasHalwax @KitsuneRal
Update proposals/4041-retry-after-header-rate-limiting.md
cce98eb 
Thanks for the advice! What a stupid mistake!

Co-authored-by: Alexey Rusakov <Kitsune-Ral@users.sf.net>
@turt2live turt2live changed the title http header Retry-After for http code 429 MSC4041: http header Retry-After for http code 429 Aug 10, 2023
@turt2live
Copy link
Member

@ThomasHalwax please sign off on your changes here for this MSC to be eligible for FCP in the future.

@turt2live turt2live added proposal A matrix spec change proposal client-server Client-Server API kind:maintenance MSC which clarifies/updates existing spec needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. labels Aug 10, 2023
@ThomasHalwax
Copy link
Contributor Author

ThomasHalwax commented Aug 11, 2023 •
edited
Loading

Signed-off-by: Thomas Halwax thomas.halwax@syncpoint.io

MTRNord
MTRNord approved these changes Aug 11, 2023
View reviewed changes
Copy link
Contributor

@MTRNord MTRNord left a comment

Choose a reason for hiding this comment

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

Lgtm apart from the one comment. Imho this makes sense and would ease the effort to implement the spec to some extend.

proposals/4041-retry-after-header-rate-limiting.md Show resolved Hide resolved
@ThomasHalwax ThomasHalwax marked this pull request as ready for review August 16, 2023 08:54
@Half-Shot Half-Shot mentioned this pull request Aug 19, 2023
Merged
3 tasks
@Half-Shot
Copy link
Contributor

Server side implementation in matrix-org/synapse#16136.

Unsure if we need a client impl, given its a standard header. If needed, maybe @ThomasHalwax can help there.

@ThomasHalwax
Copy link
Contributor Author

Server side implementation in matrix-org/synapse#16136.

Unsure if we need a client impl, given its a standard header. If needed, maybe @ThomasHalwax can help there.

It is a standard header but not all clients libs take care about it. The current matrix-js-sdk seems to ignore it which is OK since the current Matrix Spec does not mention it.

@anoadragon453
Copy link
Member

The Synapse and mautrix-go implementations listed in the PR description look sound. Removing the needs-implementation label.

@anoadragon453 anoadragon453 removed the needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. label Jan 4, 2024
@anoadragon453
Copy link
Member

anoadragon453 commented Jan 4, 2024 •
edited
Loading

The author has indicated this MSC is ready for acceptance. However, the remaining conversations should be resolved before a final comment period for acceptance can be proposed.

Please CC @matrix-org/spec-core-team (without code blocks) in said conversation(s) if you require guidance on resolving them, or ask for help in the Office of the Matrix Spec Core Team room. Thanks!

uhoreg
uhoreg reviewed Feb 6, 2024
View reviewed changes
proposals/4041-retry-after-header-rate-limiting.md

## Potential issues

In order to maintain backward compatibility with existing client libraries home servers shall use both the `Retry-After` header and the
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
In order to maintain backward compatibility with existing client libraries home servers shall use both the `Retry-After` header and the
In order to maintain backward compatibility with existing client libraries homeservers shall use both the `Retry-After` header and the

minor nitpick: homeserver is one word

Copy link
Member

Choose a reason for hiding this comment

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

There's a couple of other occurrences of the incorrect spelling as well.

Copy link
Member

Choose a reason for hiding this comment

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

Will apply manually after merging - can't edit the PR itself.

@mscbot
Copy link
Collaborator

mscbot commented Feb 13, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@mscbot mscbot added final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. and removed proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. labels Feb 13, 2024
@mscbot
Copy link
Collaborator

mscbot commented Feb 18, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

@mscbot mscbot added finished-final-comment-period and removed disposition-merge final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. labels Feb 18, 2024
@turt2live turt2live merged commit 555b5d7 into matrix-org:main Feb 19, 2024
1 check passed
@turt2live turt2live added spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec and removed finished-final-comment-period labels Feb 19, 2024
turt2live added a commit that referenced this pull request Feb 19, 2024
@turt2live
home server -> homeserver
9eaf81f 
#4041 (comment)
@clokep clokep mentioned this pull request Feb 20, 2024
Closed
@Half-Shot Half-Shot mentioned this pull request Mar 4, 2024
Merged
3 tasks
@turt2live
Copy link
Member

Spec PR: matrix-org/matrix-spec#1737

@turt2live turt2live added spec-pr-in-review A proposal which has been PR'd against the spec and is in review and removed spec-pr-missing Proposal has been implemented and is being used in the wild but hasn't yet been added to the spec labels Mar 4, 2024
@richvdh richvdh mentioned this pull request Mar 13, 2024
Merged
@turt2live
Copy link
Member

Merged 🎉

@turt2live turt2live added merged A proposal whose PR has merged into the spec! and removed spec-pr-in-review A proposal which has been PR'd against the spec and is in review labels Mar 20, 2024
@zecakeh zecakeh mentioned this pull request Mar 22, 2024
Closed
28 tasks
@clokep clokep mentioned this pull request Mar 22, 2024
Closed
8 tasks
@zecakeh zecakeh mentioned this pull request Mar 24, 2024
Merged
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Apr 2, 2024
@Midar
Update chat/matrix-synapse to 1.104.0
75eeabd 
# Synapse 1.104.0 (2024-04-02)

### Bugfixes

- Fix regression when using OIDC provider. Introduced in v1.104.0rc1. ([\#17031](element-hq/synapse#17031))


# Synapse 1.104.0rc1 (2024-03-26)

### Features

- Add an OIDC config to specify extra parameters for the authorization grant URL. IT can be useful to pass an ACR value for example. ([\#16971](element-hq/synapse#16971))
- Add support for OIDC provider returning JWT. ([\#16972](element-hq/synapse#16972), [\#17031](element-hq/synapse#17031))

### Bugfixes

- Fix a bug which meant that, under certain circumstances, we might never retry sending events or to-device messages over federation after a failure. ([\#16925](element-hq/synapse#16925))
- Fix various long-standing bugs which could cause incorrect state to be returned from `/sync` in certain situations. ([\#16949](element-hq/synapse#16949))
- Fix case in which `m.fully_read` marker would not get updated. Contributed by @SpiritCroc. ([\#16990](element-hq/synapse#16990))
- Fix bug which did not retract a user's pending knocks at rooms when their account was deactivated. Contributed by @hanadi92. ([\#17010](element-hq/synapse#17010))

### Updates to the Docker image

- Updated `start.py` to generate config using the correct user ID when running as root (fixes [\#16824](element-hq/synapse#16824), [\#15202](element-hq/synapse#15202)). ([\#16978](element-hq/synapse#16978))

### Improved Documentation

- Add a query to force a refresh of a remote user's device list to the "Useful SQL for Admins" documentation page. ([\#16892](element-hq/synapse#16892))
- Minor grammatical corrections to the upgrade documentation. ([\#16965](element-hq/synapse#16965))
- Fix the sort order for the documentation version picker, so that newer releases appear above older ones. ([\#16966](element-hq/synapse#16966))
- Remove recommendation for a specific poetry version from contributing guide. ([\#17002](element-hq/synapse#17002))

### Internal Changes

- Improve lock performance when a lot of locks are all waiting for a single lock to be released. ([\#16840](element-hq/synapse#16840))
- Update power level default for public rooms. ([\#16907](element-hq/synapse#16907))
- Improve event validation. ([\#16908](element-hq/synapse#16908))
- Multi-worker-docker-container: disable log buffering. ([\#16919](element-hq/synapse#16919))
- Refactor state delta calculation in `/sync` handler. ([\#16929](element-hq/synapse#16929))
- Clarify docs for some room state functions. ([\#16950](element-hq/synapse#16950))
- Specify IP subnets in canonical form. ([\#16953](element-hq/synapse#16953))
- As done for SAML mapping provider, let's pass the module API to the OIDC one so the mapper can do more logic in its code. ([\#16974](element-hq/synapse#16974))
- Allow containers building on top of Synapse's Complement container is use the included PostgreSQL cluster. ([\#16985](element-hq/synapse#16985))
- Raise poetry-core version cap to 1.9.0. ([\#16986](element-hq/synapse#16986))
- Patch the db conn pool sooner in tests. ([\#17017](element-hq/synapse#17017))



### Updates to locked dependencies

* Bump anyhow from 1.0.80 to 1.0.81. ([\#17009](element-hq/synapse#17009))
* Bump black from 23.10.1 to 24.2.0. ([\#16936](element-hq/synapse#16936))
* Bump cryptography from 41.0.7 to 42.0.5. ([\#16958](element-hq/synapse#16958))
* Bump dawidd6/action-download-artifact from 3.1.1 to 3.1.2. ([\#16960](element-hq/synapse#16960))
* Bump dawidd6/action-download-artifact from 3.1.2 to 3.1.4. ([\#17008](element-hq/synapse#17008))
* Bump jinja2 from 3.1.2 to 3.1.3. ([\#17005](element-hq/synapse#17005))
* Bump log from 0.4.20 to 0.4.21. ([\#16977](element-hq/synapse#16977))
* Bump mypy from 1.5.1 to 1.8.0. ([\#16901](element-hq/synapse#16901))
* Bump netaddr from 0.9.0 to 1.2.1. ([\#17006](element-hq/synapse#17006))
* Bump pydantic from 2.6.0 to 2.6.4. ([\#17004](element-hq/synapse#17004))
* Bump pyo3 from 0.20.2 to 0.20.3. ([\#16962](element-hq/synapse#16962))
* Bump ruff from 0.1.14 to 0.3.2. ([\#16994](element-hq/synapse#16994))
* Bump serde from 1.0.196 to 1.0.197. ([\#16963](element-hq/synapse#16963))
* Bump serde_json from 1.0.113 to 1.0.114. ([\#16961](element-hq/synapse#16961))
* Bump types-jsonschema from 4.21.0.20240118 to 4.21.0.20240311. ([\#17007](element-hq/synapse#17007))
* Bump types-psycopg2 from 2.9.21.16 to 2.9.21.20240311. ([\#16995](element-hq/synapse#16995))
* Bump types-pyopenssl from 23.3.0.0 to 24.0.0.20240311. ([\#17003](element-hq/synapse#17003))

# Synapse 1.103.0 (2024-03-19)

No significant changes since 1.103.0rc1.




# Synapse 1.103.0rc1 (2024-03-12)

### Features

- Add a new [List Accounts v3](https://element-hq.github.io/synapse/v1.103/admin_api/user_admin_api.html#list-accounts-v3) Admin API with improved deactivated user filtering capabilities. ([\#16874](element-hq/synapse#16874))
- Include `Retry-After` header by default per [MSC4041](matrix-org/matrix-spec-proposals#4041). Contributed by @clokep. ([\#16947](element-hq/synapse#16947))

### Bugfixes

- Fix joining remote rooms when a module uses the `on_new_event` callback. This callback may now pass partial state events instead of the full state for remote rooms. Introduced in v1.76.0. ([\#16973](element-hq/synapse#16973))
- Fix performance issue when joining very large rooms that can cause the server to lock up. Introduced in v1.100.0. Contributed by @ggogel. ([\#16968](element-hq/synapse#16968))

### Improved Documentation

- Add HAProxy example for single port operation to reverse proxy documentation. Contributed by Georg Pfuetzenreuter (@tacerus). ([\#16768](element-hq/synapse#16768))
- Improve the documentation around running Complement tests with new configuration parameters. ([\#16946](element-hq/synapse#16946))
- Add docs on upgrading from a very old version. ([\#16951](element-hq/synapse#16951))


### Updates to locked dependencies

* Bump JasonEtco/create-an-issue from 2.9.1 to 2.9.2. ([\#16934](element-hq/synapse#16934))
* Bump anyhow from 1.0.79 to 1.0.80. ([\#16935](element-hq/synapse#16935))
* Bump dawidd6/action-download-artifact from 3.0.0 to 3.1.1. ([\#16933](element-hq/synapse#16933))
* Bump furo from 2023.9.10 to 2024.1.29. ([\#16939](element-hq/synapse#16939))
* Bump pyopenssl from 23.3.0 to 24.0.0. ([\#16937](element-hq/synapse#16937))
* Bump types-netaddr from 0.10.0.20240106 to 1.2.0.20240219. ([\#16938](element-hq/synapse#16938))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clokep clokep clokep left review comments

@dbkr dbkr dbkr left review comments

@uhoreg uhoreg uhoreg left review comments

@turt2live turt2live turt2live approved these changes

@ara4n ara4n ara4n left review comments

@anoadragon453 anoadragon453 anoadragon453 left review comments

@Half-Shot Half-Shot Half-Shot left review comments

@KitsuneRal KitsuneRal KitsuneRal left review comments

@MTRNord MTRNord MTRNord approved these changes

Assignees
No one assigned
Labels
client-server Client-Server API kind:maintenance MSC which clarifies/updates existing spec merged A proposal whose PR has merged into the spec! proposal A matrix spec change proposal
Projects
Spec Core Team Backlog
Status: Done to some definition
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.