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

feat: add timeout middleware #1529

Merged
merged 4 commits into from
Apr 25, 2024
Merged

feat: add timeout middleware #1529

merged 4 commits into from
Apr 25, 2024

Conversation

J0
Copy link
Contributor

@J0 J0 commented Apr 11, 2024

A new middleware is introduced that enforces a strict timeout by using context.WithTimeout(). When the timeout is reached, a 504 JSON error with the request_timeout error code is sent. Anything that depends on the context is cancelled.

@coveralls
Copy link

coveralls commented Apr 12, 2024

Pull Request Test Coverage Report for Build 8829292642

Details

  • 67 of 70 (95.71%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 65.73%

Changes Missing Coverage Covered Lines Changed/Added Lines %
internal/api/errors.go 5 8 62.5%
Totals Coverage Status
Change from base Build 8800621941: 0.2%
Covered Lines: 8207
Relevant Lines: 12486

💛 - Coveralls

@hf hf force-pushed the j0/add_timeout_middleware branch from f3d692d to 4bd26b1 Compare April 24, 2024 15:39
@hf hf marked this pull request as ready for review April 24, 2024 15:40
@hf hf requested a review from a team as a code owner April 24, 2024 15:40
@hf hf force-pushed the j0/add_timeout_middleware branch 2 times, most recently from 9d4d500 to a79dab0 Compare April 24, 2024 15:57
@hf hf force-pushed the j0/add_timeout_middleware branch from a79dab0 to 840b30c Compare April 24, 2024 21:54
internal/api/saml.go Outdated Show resolved Hide resolved
internal/api/middleware.go Outdated Show resolved Hide resolved
@kangmingtay kangmingtay force-pushed the j0/add_timeout_middleware branch from b200f00 to 1ec8152 Compare April 25, 2024 08:18
@hf hf merged commit f96ff31 into master Apr 25, 2024
2 checks passed
@hf hf deleted the j0/add_timeout_middleware branch April 25, 2024 08:35
hf pushed a commit that referenced this pull request Apr 25, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.150.0](v2.149.0...v2.150.0)
(2024-04-25)


### Features

* add support for Azure CIAM login
([#1541](#1541))
([1cb4f96](1cb4f96))
* add timeout middleware
([#1529](#1529))
([f96ff31](f96ff31))
* allow for postgres and http functions on each extensibility point
([#1528](#1528))
([348a1da](348a1da))
* merge provider metadata on link account
([#1552](#1552))
([bd8b5c4](bd8b5c4))
* send over user in SendSMS Hook instead of UserID
([#1551](#1551))
([d4d743c](d4d743c))


### Bug Fixes

* return error if session id does not exist
([#1538](#1538))
([91e9eca](91e9eca))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
hf added a commit that referenced this pull request May 22, 2024
#1529 introduced timeout middleware, but it appears from working in the
wild it has some race conditions that are not particularly helpful.

This PR rewrites the implementation to get rid of race conditions, at
the expense of slightly higher RAM usage. It follows the implementation
of `http.TimeoutHandler` closely.

---------

Co-authored-by: Kang Ming <kang.ming1996@gmail.com>
uxodb pushed a commit to uxodb/auth that referenced this pull request Nov 13, 2024
A new middleware is introduced that enforces a strict timeout by using
`context.WithTimeout()`. When the timeout is reached, a 504 JSON error
with the `request_timeout` error code is sent. Anything that depends on
the context is cancelled.

---------

Co-authored-by: Kang Ming <kang.ming1996@gmail.com>
uxodb pushed a commit to uxodb/auth that referenced this pull request Nov 13, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.150.0](supabase/auth@v2.149.0...v2.150.0)
(2024-04-25)


### Features

* add support for Azure CIAM login
([supabase#1541](supabase#1541))
([1cb4f96](supabase@1cb4f96))
* add timeout middleware
([supabase#1529](supabase#1529))
([f96ff31](supabase@f96ff31))
* allow for postgres and http functions on each extensibility point
([supabase#1528](supabase#1528))
([348a1da](supabase@348a1da))
* merge provider metadata on link account
([supabase#1552](supabase#1552))
([bd8b5c4](supabase@bd8b5c4))
* send over user in SendSMS Hook instead of UserID
([supabase#1551](supabase#1551))
([d4d743c](supabase@d4d743c))


### Bug Fixes

* return error if session id does not exist
([supabase#1538](supabase#1538))
([91e9eca](supabase@91e9eca))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
uxodb pushed a commit to uxodb/auth that referenced this pull request Nov 13, 2024
supabase#1529 introduced timeout middleware, but it appears from working in the
wild it has some race conditions that are not particularly helpful.

This PR rewrites the implementation to get rid of race conditions, at
the expense of slightly higher RAM usage. It follows the implementation
of `http.TimeoutHandler` closely.

---------

Co-authored-by: Kang Ming <kang.ming1996@gmail.com>
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 13, 2024
A new middleware is introduced that enforces a strict timeout by using
`context.WithTimeout()`. When the timeout is reached, a 504 JSON error
with the `request_timeout` error code is sent. Anything that depends on
the context is cancelled.

---------

Co-authored-by: Kang Ming <kang.ming1996@gmail.com>
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 13, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.150.0](supabase/auth@v2.149.0...v2.150.0)
(2024-04-25)


### Features

* add support for Azure CIAM login
([supabase#1541](supabase#1541))
([1cb4f96](supabase@1cb4f96))
* add timeout middleware
([supabase#1529](supabase#1529))
([f96ff31](supabase@f96ff31))
* allow for postgres and http functions on each extensibility point
([supabase#1528](supabase#1528))
([348a1da](supabase@348a1da))
* merge provider metadata on link account
([supabase#1552](supabase#1552))
([bd8b5c4](supabase@bd8b5c4))
* send over user in SendSMS Hook instead of UserID
([supabase#1551](supabase#1551))
([d4d743c](supabase@d4d743c))


### Bug Fixes

* return error if session id does not exist
([supabase#1538](supabase#1538))
([91e9eca](supabase@91e9eca))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 13, 2024
supabase#1529 introduced timeout middleware, but it appears from working in the
wild it has some race conditions that are not particularly helpful.

This PR rewrites the implementation to get rid of race conditions, at
the expense of slightly higher RAM usage. It follows the implementation
of `http.TimeoutHandler` closely.

---------

Co-authored-by: Kang Ming <kang.ming1996@gmail.com>
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 15, 2024
A new middleware is introduced that enforces a strict timeout by using
`context.WithTimeout()`. When the timeout is reached, a 504 JSON error
with the `request_timeout` error code is sent. Anything that depends on
the context is cancelled.

---------

Co-authored-by: Kang Ming <kang.ming1996@gmail.com>
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 15, 2024
🤖 I have created a release *beep* *boop*
---


##
[2.150.0](supabase/auth@v2.149.0...v2.150.0)
(2024-04-25)


### Features

* add support for Azure CIAM login
([supabase#1541](supabase#1541))
([1cb4f96](supabase@1cb4f96))
* add timeout middleware
([supabase#1529](supabase#1529))
([f96ff31](supabase@f96ff31))
* allow for postgres and http functions on each extensibility point
([supabase#1528](supabase#1528))
([348a1da](supabase@348a1da))
* merge provider metadata on link account
([supabase#1552](supabase#1552))
([bd8b5c4](supabase@bd8b5c4))
* send over user in SendSMS Hook instead of UserID
([supabase#1551](supabase#1551))
([d4d743c](supabase@d4d743c))


### Bug Fixes

* return error if session id does not exist
([supabase#1538](supabase#1538))
([91e9eca](supabase@91e9eca))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
LashaJini pushed a commit to LashaJini/auth that referenced this pull request Nov 15, 2024
supabase#1529 introduced timeout middleware, but it appears from working in the
wild it has some race conditions that are not particularly helpful.

This PR rewrites the implementation to get rid of race conditions, at
the expense of slightly higher RAM usage. It follows the implementation
of `http.TimeoutHandler` closely.

---------

Co-authored-by: Kang Ming <kang.ming1996@gmail.com>
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.

4 participants