macchiato-framework / macchiato-core

Ring style HTTP server abstraction for Node.js
MIT License
377 stars 35 forks source link

Fix error response when `:body` is `stream` #39

Closed nenadalm closed 4 years ago

nenadalm commented 4 years ago

Hi. First response code and headers are written: https://github.com/macchiato-framework/macchiato-core/blob/f21bddc621745696b85e5fa97870f6fe913fcf1a/src/macchiato/http.cljs#L102

then stream is piped into node response: https://github.com/macchiato-framework/macchiato-core/blob/f21bddc621745696b85e5fa97870f6fe913fcf1a/src/macchiato/http.cljs#L96

then if error occurs: https://github.com/macchiato-framework/macchiato-core/blob/f21bddc621745696b85e5fa97870f6fe913fcf1a/src/macchiato/http.cljs#L95 response code and headers are written again: https://github.com/macchiato-framework/macchiato-core/blob/f21bddc621745696b85e5fa97870f6fe913fcf1a/src/macchiato/http.cljs#L110

which causes nodejs crashing with: Error [ERR_HTTP_HEADERS_SENT]: Cannot set headers after they are sent to the client and streamed file seems to be downloaded properly in browser (what was sent until the error is in the content and there is no hint that something failed)

With this fix request is aborted and file shows Network error in browser which is probably how it should be.

yogthos commented 4 years ago

Thanks 👍

yogthos commented 4 years ago

Just pushed out a new version along with an updated template.

nenadalm commented 4 years ago

thanks.