Skip to content

Commit

Permalink
stream: make virtual methods errors consistent
Browse files Browse the repository at this point in the history
Use the same error code and always emit the error instead of
throwing it.
  • Loading branch information
lpinca committed Mar 11, 2018
1 parent 599337f commit f8bd8ec
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 61 deletions.
2 changes: 1 addition & 1 deletion lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,7 @@ OutgoingMessage.prototype.removeHeader = function removeHeader(name) {


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

Object.defineProperty(OutgoingMessage.prototype, 'headersSent', {
Expand Down
4 changes: 2 additions & 2 deletions lib/_stream_readable.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const { getHighWaterMark } = require('internal/streams/state');
const {
ERR_INVALID_ARG_TYPE,
ERR_STREAM_PUSH_AFTER_EOF,
ERR_STREAM_READ_NOT_IMPLEMENTED,
ERR_METHOD_NOT_IMPLEMENTED,
ERR_STREAM_UNSHIFT_AFTER_END_EVENT
} = require('internal/errors').codes;
const ReadableAsyncIterator = require('internal/streams/async_iterator');
Expand Down Expand Up @@ -568,7 +568,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) {
this.emit('error', new ERR_STREAM_READ_NOT_IMPLEMENTED());
this.emit('error', 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 @@ -162,7 +162,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) {
throw new ERR_METHOD_NOT_IMPLEMENTED('_transform');
cb(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 @@ -553,7 +553,7 @@ function clearBuffer(stream, state) {
}

Writable.prototype._write = function(chunk, encoding, cb) {
cb(new ERR_METHOD_NOT_IMPLEMENTED('_write'));
cb(new ERR_METHOD_NOT_IMPLEMENTED('_write()'));
};

Writable.prototype._writev = null;
Expand Down
1 change: 0 additions & 1 deletion lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -849,7 +849,6 @@ E('ERR_STREAM_CANNOT_PIPE', 'Cannot pipe, not readable', Error);
E('ERR_STREAM_DESTROYED', 'Cannot call %s after a stream was destroyed', Error);
E('ERR_STREAM_NULL_VALUES', 'May not write null values to stream', TypeError);
E('ERR_STREAM_PUSH_AFTER_EOF', 'stream.push() after EOF', Error);
E('ERR_STREAM_READ_NOT_IMPLEMENTED', '_read() is not implemented', Error);
E('ERR_STREAM_UNSHIFT_AFTER_END_EVENT',
'stream.unshift() after end event', Error);
E('ERR_STREAM_WRAP', 'Stream has StringDecoder set or is in objectMode', Error);
Expand Down
25 changes: 10 additions & 15 deletions test/parallel/test-http-outgoing-proto.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,6 @@ const OutgoingMessage = http.OutgoingMessage;
const ClientRequest = http.ClientRequest;
const ServerResponse = http.ServerResponse;

common.expectsError(
OutgoingMessage.prototype._implicitHeader,
{
code: 'ERR_METHOD_NOT_IMPLEMENTED',
type: Error,
message: 'The _implicitHeader() method is not implemented'
}
);
assert.strictEqual(
typeof ClientRequest.prototype._implicitHeader, 'function');
assert.strictEqual(
Expand Down Expand Up @@ -67,14 +59,17 @@ common.expectsError(() => {
});

// write
common.expectsError(() => {
{
const outgoingMessage = new OutgoingMessage();
outgoingMessage.write();
}, {
code: 'ERR_METHOD_NOT_IMPLEMENTED',
type: Error,
message: 'The _implicitHeader() method is not implemented'
});

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

outgoingMessage.write('');
}

assert(OutgoingMessage.prototype.write.call({ _header: 'test' }));

Expand Down
15 changes: 9 additions & 6 deletions test/parallel/test-stream-readable-with-unimplemented-_read.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
'use strict';
const common = require('../common');
const stream = require('stream');

const readable = new stream.Readable();
const { Readable } = require('stream');

common.expectsError(() => readable.read(), {
code: 'ERR_STREAM_READ_NOT_IMPLEMENTED',
const readable = new Readable();

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

readable.read();
32 changes: 16 additions & 16 deletions test/parallel/test-stream-transform-constructor-set-methods.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ const common = require('../common');
const { strictEqual } = require('assert');
const { Transform } = require('stream');

const t = new Transform();

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

t.end(Buffer.from('blerg'));

const _transform = common.mustCall((chunk, _, next) => {
next();
});
Expand All @@ -16,25 +26,15 @@ const _flush = common.mustCall((next) => {
next();
});

const t = new Transform({
const t2 = new Transform({
transform: _transform,
flush: _flush,
final: _final
});

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

t.end(Buffer.from('blerg'));
t.resume();

const t2 = new Transform({});

common.expectsError(() => {
t2.end(Buffer.from('blerg'));
}, {
type: Error,
code: 'ERR_METHOD_NOT_IMPLEMENTED',
message: 'The _transform method is not implemented'
});
t2.end(Buffer.from('blerg'));
t2.resume();
36 changes: 18 additions & 18 deletions test/parallel/test-stream-writable-constructor-set-methods.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ const common = require('../common');
const { strictEqual } = require('assert');
const { Writable } = require('stream');

const w = new Writable();

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

w.end(Buffer.from('blerg'));

const _write = common.mustCall((chunk, _, next) => {
next();
});
Expand All @@ -13,25 +23,15 @@ const _writev = common.mustCall((chunks, next) => {
next();
});

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

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

w.write(Buffer.from('blerg'));
w2.write(Buffer.from('blerg'));

w.cork();
w.write(Buffer.from('blerg'));
w.write(Buffer.from('blerg'));

w.end();

const w2 = new Writable();

w2.on('error', common.expectsError({
type: Error,
code: 'ERR_METHOD_NOT_IMPLEMENTED',
message: 'The _write method is not implemented'
}));
w2.cork();
w2.write(Buffer.from('blerg'));
w2.write(Buffer.from('blerg'));

w2.end(Buffer.from('blerg'));
w2.end();

0 comments on commit f8bd8ec

Please sign in to comment.