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

Always attempt to set the Connection: close response header #429

Merged
merged 3 commits into from Feb 4, 2024

Conversation

zanieb
Copy link
Contributor

@zanieb zanieb commented Dec 21, 2023

While working on #428, I pondered the idea that closing the connection should cause client-side complaints of broken connections. It looks like Task.close_on_finish is sometimes set to true alongside a Connection: close header and other times not. If headers have not been written yet, it makes sense to always include Connection: close when close_on_finish is set. This pull request adds a utility that sets both of these values together. This results in better connection handling for clients.

There are a couple caveats:

  • If Connection already has a directive in the response headers, we will not set it
  • If headers have already been sent to the client, there is of course nothing we can do

There is also not particularly simple way to ensure that people call this method correctly unless we want to introduce a property for this variable — but I'm not really into properties with side-effects.

I believe this change does not invalidate #428 but it does also resolve #427.

@digitalresistor digitalresistor merged commit f737755 into Pylons:main Feb 4, 2024
27 checks passed
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.

Broken connections with many concurrent HEAD requests
2 participants