Skip to content

Move HTTP status handling out of handlers#2499

Open
rfontanarosa wants to merge 8 commits intomasterfrom
rfontanarosa/1920/code-health-emit-ok-http-status-in-callbacks-instead-of-onhttpsrequestasync-1
Open

Move HTTP status handling out of handlers#2499
rfontanarosa wants to merge 8 commits intomasterfrom
rfontanarosa/1920/code-health-emit-ok-http-status-in-callbacks-instead-of-onhttpsrequestasync-1

Conversation

@rfontanarosa
Copy link
Copy Markdown
Collaborator

closes #1920

this change makes HTTPS handlers transport-agnostic for status codes

Copy link
Copy Markdown
Collaborator

@gino-m gino-m left a comment

Choose a reason for hiding this comment

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

Not to bike shed too much on the design, but if the handlers are defined at the level of abstraction of the HTTP request/response, why should they set output in the response, but not the error codes?

I know earlier I had said I was hoping to abstract the HTTP stream away, but it seems with streaming, esp. with frameworks like busboy, that becomes difficult or impossible. If that's the case, taking a half measure and setting some things directly on the response in the handlers, and some things outside actually increases overall complexity.

If the core problem is that the handler might forget to set the response code and end the connection, is it possible for the wrapper to check whether the stream has been closed and to do it if necessary?

…-status-in-callbacks-instead-of-onhttpsrequestasync-1
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.

[Code health] Emit OK HTTP status in callbacks instead of onHttpsRequestAsync

2 participants