Skip to content

Commit

Permalink
stream: destroy wrapped streams on error
Browse files Browse the repository at this point in the history
Stream should be destroyed and update state accordingly when
the wrapped stream emits error.

Does some additional cleanup with future TODOs that might be worth
looking into.

PR-URL: #34102
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
ronag authored and MylesBorins committed Jul 16, 2020
1 parent 46d183c commit 41c80f6
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 5 deletions.
28 changes: 23 additions & 5 deletions lib/_stream_readable.js
Expand Up @@ -66,7 +66,6 @@ ObjectSetPrototypeOf(Readable.prototype, Stream.prototype);
ObjectSetPrototypeOf(Readable, Stream);

const { errorOrDestroy } = destroyImpl;
const kProxyEvents = ['error', 'close', 'destroy', 'pause', 'resume'];

function prependListener(emitter, event, fn) {
// Sadly this is not cacheable as some libraries bundle their own
Expand Down Expand Up @@ -1034,10 +1033,29 @@ Readable.prototype.wrap = function(stream) {
}
}

// Proxy certain important events.
for (const kProxyEvent of kProxyEvents) {
stream.on(kProxyEvent, this.emit.bind(this, kProxyEvent));
}
stream.on('error', (err) => {
errorOrDestroy(this, err);
});

stream.on('close', () => {
// TODO(ronag): Update readable state?
this.emit('close');
});

stream.on('destroy', () => {
// TODO(ronag): this.destroy()?
this.emit('destroy');
});

stream.on('pause', () => {
// TODO(ronag): this.pause()?
this.emit('pause');
});

stream.on('resume', () => {
// TODO(ronag): this.resume()?
this.emit('resume');
});

// When we try to consume some more bytes, simply unpause the
// underlying stream.
Expand Down
32 changes: 32 additions & 0 deletions test/parallel/test-stream2-readable-wrap-error.js
@@ -0,0 +1,32 @@
'use strict';
const common = require('../common');
const assert = require('assert');

const Readable = require('_stream_readable');
const EE = require('events').EventEmitter;

const oldStream = new EE();
oldStream.pause = () => {};
oldStream.resume = () => {};

{
const r = new Readable({ autoDestroy: true })
.wrap(oldStream)
.on('error', common.mustCall(() => {
assert.strictEqual(r._readableState.errorEmitted, true);
assert.strictEqual(r._readableState.errored, true);
assert.strictEqual(r.destroyed, true);
}));
oldStream.emit('error', new Error());
}

{
const r = new Readable({ autoDestroy: false })
.wrap(oldStream)
.on('error', common.mustCall(() => {
assert.strictEqual(r._readableState.errorEmitted, true);
assert.strictEqual(r._readableState.errored, true);
assert.strictEqual(r.destroyed, false);
}));
oldStream.emit('error', new Error());
}

0 comments on commit 41c80f6

Please sign in to comment.