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

Use a fresh connection on every request #380

Merged
merged 2 commits into from
Sep 4, 2024
Merged

Use a fresh connection on every request #380

merged 2 commits into from
Sep 4, 2024

Conversation

grego118
Copy link
Contributor

@grego118 grego118 commented Sep 4, 2024

We shouldn't be using a shared requests.Session() object across all requests that a client makes. If the client (and therefore the session) is shared across multiple threads or greenlets we can encounter a race condition that causes the following error:

('Connection aborted.', RemoteDisconnected('Remote end closed connection without response'))

This happens because the session maintains a connection pool for each host (keep-alive). If a connection is closed by the server around the same time a thread tries to make a new request using that connection, then the new request will be made on the already closed connection.

This PR simply removes the session member and lets the requests package make a new connection on every request.

@grego118 grego118 merged commit ba77c0b into main Sep 4, 2024
4 checks passed
@grego118 grego118 deleted the noSharedSession branch September 4, 2024 20:49
@AaronDDM AaronDDM mentioned this pull request Sep 4, 2024
AaronDDM added a commit that referenced this pull request Sep 4, 2024
* Fix typo on Clean Messages (Fix typo on Clean Messages (#375)
* Remove use of TestCommand (Remove use of TestCommand (#377)
* Add Folder Webhooks (Add Folder Webhooks - Python SDK (#374)
* Fix request session being reused across multiple requests (#380)
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.

2 participants