send icon indicating copy to clipboard operation
send copied to clipboard

Add ability to transform file stream before sending it

Open manifoldhiker opened this issue 7 years ago • 7 comments

I have the case, where I need to do some transformations on the file before writing it to the response. The way I do it now is by passing the array of transformation streams with options.

Could it be useful for others? If yes, maybe there are better ways of doing this?

manifoldhiker avatar Oct 09 '18 14:10 manifoldhiker

I know there was some previous PR / issue on this, and I think it is desired. I think there are some issues to work out in this implementation, mainly that any alteration in one of those transform streams that alters the length will mean the Content-Length header will be wrong and cause a transport error.

dougwilson avatar Oct 09 '18 15:10 dougwilson

Ok, I will try to fix this issues

manifoldhiker avatar Oct 09 '18 15:10 manifoldhiker

Hi @dougwilson ! I am figuring out how to apply transformations on the file (like compressing or adding ID3 tag to mp3 file) and set proper Content-Length to the response. Could you advise anything about it? As I understood, headers could be sent before the transfer of the response body. That means that:

a) We need to apply a deterministic transformation (when we know exactly how it will affect the size) to set the right header before it's being sent b) In the case of undeterministic transformations, we need to load full file into memory first, do mutation, calculate its size and then send

Am I thinking correclty?

manifoldhiker avatar Oct 10 '18 09:10 manifoldhiker

I'm going to see if I can find the previous conversation(s) on this, which may help. You are thinking correctly, but of course the downside to reading in the entire file into memory is that it's very memory-intensive and becomes much easier to overflow the server's memory with lots of requests, exhausting the Node.js process memory until it just dies. That's the main reason we are using streams right now. We've had bug reports about the handling of files >2GB a few times, so there are definitely large file usage it seems.

dougwilson avatar Oct 11 '18 04:10 dougwilson

Hello @dougwilson ! Any update on this?

manifoldhiker avatar Oct 23 '18 06:10 manifoldhiker

No, sorry.

dougwilson avatar Oct 23 '18 11:10 dougwilson

The conversation I'm thinking of is in a previous PR for this feature: #69

dougwilson avatar Oct 28 '18 00:10 dougwilson

I am going to take some liberty here and close this in favor of the much more deeply discussed https://github.com/pillarjs/send/pull/195 so that we can consolidate any conversation going forward. We are not going to try and land this feature soon, as we are focusing on the v5 push for express and this is not a breaking change, but I dont want to leave too many open PRs which are unlikely to see progress, especially when they are duplicates in many ways.

wesleytodd avatar May 17 '24 23:05 wesleytodd