Repository navigation
http: create response.overwriteHead() for things like stream piping error handling #52026
Description
Activity
it seems a very specific use case, @ShogunPanda WDYT?
I'm not sure this method would make sense, IMHO.
When you writewriteHeadand then start piping, the data starts flowing to the client. Overwriting the first line might not make sense as the response is already traveling through the socket.Obviously, if the response is small enough it might happen that the response is still buffered and thus overwriting the headers make sense.
But for the general case I wouldn't assume this and therefore I'm -1 on this.
I understand what you're saying @ShogunPanda, it would only make sense if the errors are guaranteed to be thrown before the writable even has any data (ENOENT type errors for example). It wouldn't be for cases when data is already being piped after first chunk because the original set response code and headers wouldn't want to be changed anyway.
I feel a completely new method would be redundant for this specific use case, yes. That's why I feel better about the edited part of the OP---reversing the added throw because of docs confusion from #36721 and updating documentation to provide better clarity about how it works. It wasn't broken before, I used it in production in 14 LTS.
If not acceptable by the community the solution will be to create my own custom piper and inject the writeHead logic in-between the readable and writable---which requires handling the stream's other errors to prevent memory leaks (not a problem but added minutia).
I do this with uploads. I send auth data in the first chunks and pause the stream. If auth passes I resume the stream stripping away the auth data and proceed to create a temp file handle, etc., (otherwise it destroys the request as anything else would be malicious). This saves server resources very well.
Yes, I guess docs should be updated similarly as you propose. Would you like to send a PR?
I've yet to make any PRs (GitHub newbie), I read the relevant docs for this repo and it seems straightforward. Perhaps I give it a week or so if someone wishes to share the torch. Otherwise I will setup the git environment locally and make the contribution.
Much appreciated for the responses & feedback <3
There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the never-stale
Issues and PRs exempt from automated stale handling. label or close this issue if it should be closed. If not, the issue will be automatically closed 6 months after the last non-automated comment.
For more information on how the project manages feature requests, please consult the feature request management document.There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.
For more information on how the project manages feature requests, please consult the feature request management document.
What is the problem this feature will solve?
Problem created after fix: #45508 to throw error on multiple
writeHeadcalls (I personally updated to 20 LTS from 14 LTS recently).This creates a new problem when wanting to pipe read streams to http response streams as you cannot overwrite the header response code upon a file read stream error event to reflect the file error (
ENOENT,EISDIR, etc..) to give appropriate http responses (404, 500, etc.).Example that will now break:
What is the feature you are proposing to solve the problem?
An identical function without the #45508 changes:
Proposing http
response.overwriteHead()for at least 20 LTS +Example:
What alternatives have you considered?
I would pull and go through the process of trying to add this feature myself if I knew the pulling process well. I'm new to GitHub I've always stayed solo in development so excuse my noob-ness. I understand it's much volunteered.
I've considered breaking apart the piping for fine grain control but that defeats the purpose of piping to begin with and for others unaware it may introduce memory leaks if they don't close both read and write streams on these kinds of error events properly.
(EDIT)
If calling
writeHeadmultiple times is not detrimental to anything other than the effect of overwriting (which needs confirming), then perhaps the documentation could better reflect that instead of throwing an error on multiple use to simplify this solution. (Issue genesis: #36721)Current docs:
"This method must only be called once on a message and it must be called before
response.end()is called..."Re-worded to something like:
"This function must be called before
response.end()is called.If
response.write()orresponse.end()are called before this, the implicit/mutable headers will be calculated first.This function will directly write the supplied header values onto the network channel and merge any headers supplied with
response.setHeader()beforehand. Headers supplied to this function will have priority overresponse.setHeader()headers.If
response.setHeader()is never called before this,response.getHeader()will not yield the expected results as internal caching is bypassed. Useresponse.setHeader()if progressive population, future retrieval or modification is desired.Caution multiple calls of this function on the same response, as these calls will overwrite the current header values and response code on the network channel..."