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

client: Create a new HTTP session for each request #10

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

underyx
Copy link

@underyx underyx commented Jul 17, 2018

The globally shared connection resets after some time. When it's reset, the web worker will optimistically try to use it anyway, and fail with a connection error.

WIth many web workers running behind a load balancer, this means that a huge portion of login requests would randomly land on a broken session and it was common for people to need to retry 3-4 times until their login attempt finally worked.

You can see here how within the sentry project's own code, sessions are created on demand instead of being shared globally: https://github.com/getsentry/sentry/blob/27cc0fed47732dec907668a4529ca39bb384bf5a/src/sentry/http.py#L271

underyx added 2 commits July 17, 2018 16:19
The globally shared connection resets after some time.
When it's reset, the web worker will optimistically try to use it anyway,
and fail with a connection error.

WIth many web workers running behind a load balancer,
this means that a huge portion of login requests would randomly land on a broken session
and it was common for people to need to retry 3-4 times
until their login attempt finally worked.

You can see here how within the sentry project's own code,
sessions are created on demand instead of being shared globally:
https://github.com/getsentry/sentry/blob/27cc0fed47732dec907668a4529ca39bb384bf5a/src/sentry/http.py#L271
It's not available for instance for the following RequestException:

ConnectionError(ProtocolError('Connection aborted.', error("(104, 'ECONNRESET')",)),)
@underyx
Copy link
Author

underyx commented Jul 30, 2018

@SkyLothar are you still active? Any chance to get this merged and released officially?

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.

1 participant