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

Support HTTP 103 Early Hints #46

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

Conversation

thomasjm
Copy link
Contributor

This PR follows on from snoyberg/http-client#524, to add support for HTTP 103 Early Hints to the reverse proxy.

The main change here is that we have to switch from WAI's responseStream response composer to responseRaw, because the nature of early hints means that we don't know the final HTTP status or headers up front, and the raw composer is the only one low-level enough to support sending (potentially multiple) HTTP 103 Early Hints sections before the main response.

This is slightly painful, because it

  • Increases the code complexity slightly--we have to handle HTTP 1.1 chunking ourselves, because normally responseStream would do it.
  • Requires a breaking change to the API, with defaultOnExc changing its signature. This is essentially because we have to stick the early hints callback on the request object before the call to responseOpen, and that callback needs to be able to start sending response data before responseOpen returns. As a result, I had to wrap the entire bracket call of waiProxyToSettings within the sendResponse call, which commits us to using responseRaw, which is why defaultOnExc has to work this way.

Copy link
Member

@snoyberg snoyberg left a comment

Choose a reason for hiding this comment

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

This is a level of code dupe I'm not comfortable with, at least as the maintainer of the project.

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