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

stream: make virtual methods errors consistent #18813

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();