Skip to content

Commit

Permalink
http,stream: make virtual methods throw an error
Browse files Browse the repository at this point in the history
Make virtual methods throw an ERR_METHOD_NOT_IMPLEMENTED error instead
of emitting it.

The error is not recoverable and the only way to handle it is to
override the method.

PR-URL: #31912
Refs: #31818 (review)
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
lpinca committed Mar 7, 2020
1 parent 86ab4ee commit 6f0ec79
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 46 deletions.
2 changes: 1 addition & 1 deletion lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ OutgoingMessage.prototype.removeHeader = function removeHeader(name) {


OutgoingMessage.prototype._implicitHeader = function _implicitHeader() {
this.emit('error', new ERR_METHOD_NOT_IMPLEMENTED('_implicitHeader()'));
throw new ERR_METHOD_NOT_IMPLEMENTED('_implicitHeader()');
};

ObjectDefineProperty(OutgoingMessage.prototype, 'headersSent', {
Expand Down
2 changes: 1 addition & 1 deletion lib/_stream_readable.js
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ function maybeReadMore_(stream, state) {
// for virtual (non-string, non-buffer) streams, "length" is somewhat
// arbitrary, and perhaps not very meaningful.
Readable.prototype._read = function(n) {
errorOrDestroy(this, new ERR_METHOD_NOT_IMPLEMENTED('_read()'));
throw new ERR_METHOD_NOT_IMPLEMENTED('_read()');
};

Readable.prototype.pipe = function(dest, pipeOpts) {
Expand Down
2 changes: 1 addition & 1 deletion lib/_stream_transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ Transform.prototype.push = function(chunk, encoding) {
// an error, then that'll put the hurt on the whole operation. If you
// never call cb(), then you'll never get another chunk.
Transform.prototype._transform = function(chunk, encoding, cb) {
cb(new ERR_METHOD_NOT_IMPLEMENTED('_transform()'));
throw new ERR_METHOD_NOT_IMPLEMENTED('_transform()');
};

Transform.prototype._write = function(chunk, encoding, cb) {
Expand Down
2 changes: 1 addition & 1 deletion lib/_stream_writable.js
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ Writable.prototype._write = function(chunk, encoding, cb) {
if (this._writev) {
this._writev([{ chunk, encoding }], cb);
} else {
process.nextTick(cb, new ERR_METHOD_NOT_IMPLEMENTED('_write()'));
throw new ERR_METHOD_NOT_IMPLEMENTED('_write()');
}
};

Expand Down
19 changes: 11 additions & 8 deletions test/parallel/test-http-outgoing-proto.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
const common = require('../common');
require('../common');
const assert = require('assert');

const http = require('http');
Expand Down Expand Up @@ -62,13 +62,16 @@ assert.throws(() => {
{
const outgoingMessage = new OutgoingMessage();

outgoingMessage.on('error', common.expectsError({
code: 'ERR_METHOD_NOT_IMPLEMENTED',
name: 'Error',
message: 'The _implicitHeader() method is not implemented'
}));

outgoingMessage.write('');
assert.throws(
() => {
outgoingMessage.write('');
},
{
code: 'ERR_METHOD_NOT_IMPLEMENTED',
name: 'Error',
message: 'The _implicitHeader() method is not implemented'
}
);
}

assert(OutgoingMessage.prototype.write.call({ _header: 'test' }));
Expand Down
4 changes: 3 additions & 1 deletion test/parallel/test-stream-readable-async-iterators.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ async function tests() {
{
const AsyncIteratorPrototype = Object.getPrototypeOf(
Object.getPrototypeOf(async function* () {}).prototype);
const rs = new Readable({});
const rs = new Readable({
read() {}
});
assert.strictEqual(
Object.getPrototypeOf(Object.getPrototypeOf(rs[Symbol.asyncIterator]())),
AsyncIteratorPrototype);
Expand Down
20 changes: 12 additions & 8 deletions test/parallel/test-stream-readable-with-unimplemented-_read.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
'use strict';
const common = require('../common');
require('../common');

const assert = require('assert');
const { Readable } = require('stream');

const readable = new Readable();

readable.on('error', common.expectsError({
code: 'ERR_METHOD_NOT_IMPLEMENTED',
name: 'Error',
message: 'The _read() method is not implemented'
}));

readable.read();
assert.throws(
() => {
readable.read();
},
{
code: 'ERR_METHOD_NOT_IMPLEMENTED',
name: 'Error',
message: 'The _read() method is not implemented'
}
);
25 changes: 14 additions & 11 deletions test/parallel/test-stream-transform-constructor-set-methods.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
'use strict';
const common = require('../common');

const { strictEqual } = require('assert');
const assert = require('assert');
const { Transform } = require('stream');

const t = new Transform();

t.on('error', common.expectsError({
name: 'Error',
code: 'ERR_METHOD_NOT_IMPLEMENTED',
message: 'The _transform() method is not implemented'
}));

t.end(Buffer.from('blerg'));
assert.throws(
() => {
t.end(Buffer.from('blerg'));
},
{
name: 'Error',
code: 'ERR_METHOD_NOT_IMPLEMENTED',
message: 'The _transform() method is not implemented'
}
);

const _transform = common.mustCall((chunk, _, next) => {
next();
Expand All @@ -32,9 +35,9 @@ const t2 = new Transform({
final: _final
});

strictEqual(t2._transform, _transform);
strictEqual(t2._flush, _flush);
strictEqual(t2._final, _final);
assert.strictEqual(t2._transform, _transform);
assert.strictEqual(t2._flush, _flush);
assert.strictEqual(t2._final, _final);

t2.end(Buffer.from('blerg'));
t2.resume();
28 changes: 15 additions & 13 deletions test/parallel/test-stream-writable-constructor-set-methods.js
Original file line number Diff line number Diff line change
@@ -1,34 +1,36 @@
'use strict';
const common = require('../common');

const { strictEqual } = require('assert');
const assert = require('assert');
const { Writable } = require('stream');

const w = new Writable();

w.on('error', common.expectsError({
name: 'Error',
code: 'ERR_METHOD_NOT_IMPLEMENTED',
message: 'The _write() method is not implemented'
}));

const bufferBlerg = Buffer.from('blerg');
const w = new Writable();

w.end(bufferBlerg);
assert.throws(
() => {
w.end(bufferBlerg);
},
{
name: 'Error',
code: 'ERR_METHOD_NOT_IMPLEMENTED',
message: 'The _write() method is not implemented'
}
);

const _write = common.mustCall((chunk, _, next) => {
next();
});

const _writev = common.mustCall((chunks, next) => {
strictEqual(chunks.length, 2);
assert.strictEqual(chunks.length, 2);
next();
});

const w2 = new Writable({ write: _write, writev: _writev });

strictEqual(w2._write, _write);
strictEqual(w2._writev, _writev);
assert.strictEqual(w2._write, _write);
assert.strictEqual(w2._writev, _writev);

w2.write(bufferBlerg);

Expand Down
5 changes: 4 additions & 1 deletion test/parallel/test-tls-socket-allow-half-open-option.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ const tls = require('tls');
{
// The option is ignored when the `socket` argument is a generic
// `stream.Duplex`.
const duplex = new stream.Duplex({ allowHalfOpen: false });
const duplex = new stream.Duplex({
allowHalfOpen: false,
read() {}
});
const socket = new tls.TLSSocket(duplex, { allowHalfOpen: true });
assert.strictEqual(socket.allowHalfOpen, false);
}
Expand Down

0 comments on commit 6f0ec79

Please sign in to comment.