Skip to content

Correctly handle Connection: close with streaming #598

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

Merged
merged 2 commits into from
Jun 17, 2022

Conversation

Lukasa
Copy link
Collaborator

@Lukasa Lukasa commented Jun 17, 2022

Motivation

When users stream their bodies they may still want to send Connection:
close headers and terminate the connection early. This should work
properly.

Unfortunately it became clear that we didn't correctly pass the
information that the connection needed to be closed. As a result, we'd
inappropriately re-use the connection, potentially causing unnecessary
HTTP errors.

Modifications

Signal whether the connection needs to be closed when the final
connection action is to send .end.

Results

We behave better with streaming uploads.

Motivation

When users stream their bodies they may still want to send Connection:
close headers and terminate the connection early. This should work
properly.

Unfortunately it became clear that we didn't correctly pass the
information that the connection needed to be closed. As a result, we'd
inappropriately re-use the connection, potentially causing unnecessary
HTTP errors.

Modifications

Signal whether the connection needs to be closed when the final
connection action is to send .end.

Results

We behave better with streaming uploads.
@Lukasa Lukasa added the 🔨 semver/patch No public API change. label Jun 17, 2022
@Lukasa Lukasa requested review from glbrntt and dnadoba June 17, 2022 10:54
Copy link
Collaborator

@glbrntt glbrntt left a comment

Choose a reason for hiding this comment

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

LGTM

@Lukasa Lukasa merged commit ac34f6d into swift-server:main Jun 17, 2022
@Lukasa Lukasa deleted the cb-close-with-streaming branch June 17, 2022 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants