Skip to content

Commit

Permalink
fix: only emit close on last request
Browse files Browse the repository at this point in the history
emitting `close` early can affect the internal readable stream state.
previously, this was emitted on chunks and reconnects.
  • Loading branch information
fent committed Nov 14, 2020
1 parent 997efdd commit db3f16f
Showing 1 changed file with 9 additions and 1 deletion.
10 changes: 9 additions & 1 deletion lib/index.js
Expand Up @@ -51,11 +51,19 @@ const createStream = options => {
const pipeAndSetEvents = (req, stream, end) => {
// Forward events from the request to the stream.
[
'abort', 'request', 'response', 'error', 'close', 'redirect', 'retry', 'reconnect',
'abort', 'request', 'response', 'error', 'redirect', 'retry', 'reconnect',
].forEach(event => {
req.prependListener(event, stream.emit.bind(stream, event));
});
req.pipe(stream, { end });

// Emit `close` only when stream is already destroyed, in case there are multiple requests
// due to playlists, chunking, and reconnects.
req.on('close', () => {
if (stream.destroyed) {
stream.emit('close');
}
});
};


Expand Down

2 comments on commit db3f16f

@slavikus
Copy link

Choose a reason for hiding this comment

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

The changes in this commit seems to cause crashes in 60% of executions on my app when calling downloadFromInfo with this error:

0|web      | Error: socket hang up
0|web      |     at connResetException (internal/errors.js:614:14)
0|web      |     at TLSSocket.socketCloseListener (_http_client.js:441:25)
0|web      |     at TLSSocket.emit (events.js:327:22)
0|web      |     at net.js:670:12
0|web      |     at TCP.done (_tls_wrap.js:562:7)

@fent
Copy link
Owner Author

@fent fent commented on db3f16f Nov 21, 2020

Choose a reason for hiding this comment

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

this was fixed in the last few releases. will be updated on ytdl-core soon

Please sign in to comment.