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

response event don't prevent the program to output #223

Closed
piranna opened this issue Aug 16, 2016 · 5 comments
Closed

response event don't prevent the program to output #223

piranna opened this issue Aug 16, 2016 · 5 comments
Labels
bug Something does not work as it should ✭ help wanted ✭
Milestone

Comments

@piranna
Copy link

piranna commented Aug 16, 2016

The next code exists inmediatly without waiting to fully download the file:

got.stream.get(GCC_URL)
.on('response', function(res)
{
  res.pipe(fs.createWriteStream('out.tgz'))
})

Check it with some big file like GCC or linux kernel, it will only write just some kilobytes and exit in just a couple of seconds. On the other hand, the next "equivalent" code works flawlessly:

got.stream.get(GCC_URL).pipe(fs.createWriteStream('out.tgz'))

I think that if a response event is registered it should wait until the response stream is fully consumed.

@piranna
Copy link
Author

piranna commented Aug 17, 2016

I've found this also happens when using the request event.

@floatdrop floatdrop added the bug Something does not work as it should label Aug 18, 2016
@floatdrop
Copy link
Contributor

This is because input and output streams, that returned from got is not piped anywhere and get stuck'd with filled buffer. In this situation node prefers to exit (even if there is stream, that can consume data).

@piranna
Copy link
Author

piranna commented Aug 18, 2016

This is because input and output streams, that returned from got is not
piped anywhere and get stuck'd with filled buffer. In this situation node
prefers to exit (even if there is stream, that can consume data).

The point is that app exit without error at all, and I'm using async.js and
callbacks are not being called too. Is that the expected behaviour?
Shouldn't exit with a "buffers filled" exception or something? Should I
open an issue on Node.js?

On the other hand, I think this is a valid use case, or is it intended to
use only the returned proxy and the 'request' and 'response' events are
mostly internal details? If so, I would deprecate them and use them
internally to fill the proxy object with the request and response methods.

I'm asking this because I'm doing a get request and need to wait to the
'request' event to get a reference to the request object so later I can be
able to call .abort() to interrupt it, so I think it would enought (and
simple) by adding a .abort() method and others alike to the proxy object.

@floatdrop
Copy link
Contributor

floatdrop commented May 7, 2017

I guess we can copy res properties and emit output stream here instead.

@sindresorhus sindresorhus added this to the 9.0.0 milestone Jul 7, 2018
@szmarczak
Copy link
Collaborator

szmarczak commented Jul 15, 2018

This can be fixed in a three ways:

  1. enable flowing mode (recommended)
got.stream.get(GCC_URL)
.on('response', function(res)
{
  res.pipe(fs.createWriteStream('out.tgz'))
}).resume()
  1. send the main stream to /dev/null (not recommended)
got.stream.get(GCC_URL)
.on('response', function(res)
{
  res.pipe(fs.createWriteStream('out.tgz'))
}).pipe(fs.createWriteStream('/dev/null'))
  1. Increase output's highWaterMark. (needed only if you want to process it later)

Once the total size of the internal read buffer reaches the threshold specified by highWaterMark, the stream will temporarily stop reading data from the underlying resource until the data currently buffered can be consumed (that is, the stream will stop calling the internal readable._read() method that is used to fill the read buffer).

	const input = new PassThrough();
	const output = new PassThrough({highWaterMark: bytes});
	const proxy = duplexer3(input, output);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something does not work as it should ✭ help wanted ✭
Projects
None yet
Development

No branches or pull requests

4 participants