Skip to content

emit error events consistently in next tick #5251

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

emit error events consistently in next tick #5251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 4 additions & 8 deletions lib/_http_agent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ const util = require('util');
const EventEmitter = require('events');
const debug = util.debuglog('http');

const emitError = require('internal/util').emitError;
Copy link
Member

@benjamingr benjamingr Mar 26, 2016

Choose a reason for hiding this comment

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

This is probably better, but we might as well use:

const {emitError} = require('internal/util');

Just wondering where we stand on those.

Copy link
Contributor

@mscdex mscdex Mar 26, 2016

Choose a reason for hiding this comment

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

Personally I find that particular ES6 syntax confusing because it always looks like an object literal to me.


// New Agent code.

// The largest departure from the previous implementation is that
Expand Down Expand Up @@ -143,10 +145,7 @@ Agent.prototype.addRequest = function(req, options) {
// If we are under maxSockets create a new one.
this.createSocket(req, options, function(err, newSocket) {
if (err) {
process.nextTick(function() {
req.emit('error', err);
});
return;
return process.nextTick(emitError, req, err);
}
req.onSocket(newSocket);
});
Expand Down Expand Up @@ -252,10 +251,7 @@ Agent.prototype.removeSocket = function(s, options) {
// If we have pending requests and a socket gets closed make a new one
this.createSocket(req, options, function(err, newSocket) {
if (err) {
process.nextTick(function() {
req.emit('error', err);
});
return;
return process.nextTick(emitError, req, err);
}
newSocket.emit('free');
});
Expand Down
14 changes: 6 additions & 8 deletions lib/_http_client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const OutgoingMessage = require('_http_outgoing').OutgoingMessage;
const Agent = require('_http_agent');
const Buffer = require('buffer').Buffer;

const emitError = require('internal/util').emitError;

function ClientRequest(options, cb) {
var self = this;
Expand Down Expand Up @@ -168,10 +169,7 @@ function ClientRequest(options, cb) {
return;
called = true;
if (err) {
process.nextTick(function() {
self.emit('error', err);
});
return;
return process.nextTick(emitError, self, err);
}
self.onSocket(socket);
self._deferToConnect(null, null, function() {
Expand Down Expand Up @@ -268,7 +266,7 @@ function socketCloseListener() {
// This socket error fired before we started to
// receive a response. The error needs to
// fire on the request.
req.emit('error', createHangUpError());
process.nextTick(emitError, req, createHangUpError());
req.socket._hadError = true;
}

Expand All @@ -292,7 +290,7 @@ function socketErrorListener(err) {
debug('SOCKET ERROR:', err.message, err.stack);

if (req) {
req.emit('error', err);
process.nextTick(emitError, req, err);
// For Safety. Some additional errors might fire later on
// and we need to make sure we don't double-fire the error event.
req.socket._hadError = true;
Expand Down Expand Up @@ -328,7 +326,7 @@ function socketOnEnd() {
if (!req.res && !req.socket._hadError) {
// If we don't have a response then we know that the socket
// ended prematurely and we need to emit an error on the request.
req.emit('error', createHangUpError());
process.nextTick(emitError, req, createHangUpError());
req.socket._hadError = true;
}
if (parser) {
Expand All @@ -350,7 +348,7 @@ function socketOnData(d) {
debug('parse error');
freeParser(parser, req, socket);
socket.destroy();
req.emit('error', ret);
process.nextTick(emitError, req, ret);
req.socket._hadError = true;
} else if (parser.incoming && parser.incoming.upgrade) {
// Upgrade or CONNECT
Expand Down
7 changes: 4 additions & 3 deletions lib/_tls_legacy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const debug = util.debuglog('tls-legacy');
const Buffer = require('buffer').Buffer;
const Timer = process.binding('timer_wrap').Timer;
const Connection = process.binding('crypto').Connection;
const emitError = require('internal/util').emitError;

function SlabBuffer() {
this.create();
Expand Down Expand Up @@ -833,14 +834,14 @@ SecurePair.prototype.error = function(returnOnly) {
err = connReset;
}
this.destroy();
if (!returnOnly) this.emit('error', err);
if (!returnOnly) process.nextTick(emitError, this, err);
} else if (this._isServer &&
this._rejectUnauthorized &&
/peer did not return a certificate/.test(err.message)) {
// Not really an error.
this.destroy();
} else {
if (!returnOnly) this.cleartext.emit('error', err);
if (!returnOnly) process.nextTick(emitError, this.cleartext, err);
}
return err;
};
Expand Down Expand Up @@ -878,7 +879,7 @@ exports.pipe = function pipe(pair, socket) {

function onerror(e) {
if (cleartext._controlReleased) {
cleartext.emit('error', e);
process.nextTick(emitError, cleartext, e);
}
}

Expand Down
6 changes: 4 additions & 2 deletions lib/_tls_wrap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ const tls_wrap = process.binding('tls_wrap');
const TCP = process.binding('tcp_wrap').TCP;
const Pipe = process.binding('pipe_wrap').Pipe;

const emitError = require('internal/util').emitError;

function onhandshakestart() {
debug('onhandshakestart');

Expand Down Expand Up @@ -536,7 +538,7 @@ TLSSocket.prototype._handleTimeout = function() {
TLSSocket.prototype._emitTLSError = function(err) {
var e = this._tlsError(err);
if (e)
this.emit('error', e);
process.nextTick(emitError, this, e);
};

TLSSocket.prototype._tlsError = function(err) {
Expand Down Expand Up @@ -1047,7 +1049,7 @@ exports.connect = function(/* [port, host], options, cb */) {
if (ekeyinfo.type === 'DH' && ekeyinfo.size < options.minDHSize) {
var err = new Error('DH parameter size ' + ekeyinfo.size +
' is less than ' + options.minDHSize);
socket.emit('error', err);
process.nextTick(emitError, socket, err);
socket.destroy();
return;
}
Expand Down
6 changes: 3 additions & 3 deletions lib/cluster.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const SCHED_NONE = 1;
const SCHED_RR = 2;

const uv = process.binding('uv');

const emitError = require('internal/util').emitError;
const cluster = new EventEmitter();
module.exports = cluster;
cluster.Worker = Worker;
Expand All @@ -33,8 +33,8 @@ function Worker(options) {

if (options.process) {
this.process = options.process;
this.process.on('error', (code, signal) =>
this.emit('error', code, signal)
this.process.on('error',
(code, signal) => process.nextTick(emitError, this, code, signal)
);
this.process.on('message', (message, handle) =>
this.emit('message', message, handle)
Expand Down
21 changes: 9 additions & 12 deletions lib/dgram.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const constants = require('constants');

const UDP = process.binding('udp_wrap').UDP;
const SendWrap = process.binding('udp_wrap').SendWrap;
const emitError = require('internal/util').emitError;

const BIND_STATE_UNBOUND = 0;
const BIND_STATE_BINDING = 1;
Expand Down Expand Up @@ -177,8 +178,7 @@ Socket.prototype.bind = function(port_ /*, address, callback*/) {
self._handle.lookup(address, function(err, ip) {
if (err) {
self._bindState = BIND_STATE_UNBOUND;
self.emit('error', err);
return;
return process.nextTick(emitError, self, err);
}

if (!cluster)
Expand All @@ -191,10 +191,9 @@ Socket.prototype.bind = function(port_ /*, address, callback*/) {
if (cluster.isWorker && !exclusive) {
function onHandle(err, handle) {
if (err) {
var ex = exceptionWithHostPort(err, 'bind', ip, port);
self.emit('error', ex);
self._bindState = BIND_STATE_UNBOUND;
return;
const er = exceptionWithHostPort(err, 'bind', ip, port);
return process.nextTick(emitError, self, er);
}

if (!self._handle)
Expand All @@ -218,11 +217,10 @@ Socket.prototype.bind = function(port_ /*, address, callback*/) {

const err = self._handle.bind(ip, port || 0, flags);
if (err) {
var ex = exceptionWithHostPort(err, 'bind', ip, port);
self.emit('error', ex);
self._bindState = BIND_STATE_UNBOUND;
// Todo: close?
return;
self._bindState = BIND_STATE_UNBOUND;
const er = exceptionWithHostPort(err, 'bind', ip, port);
return process.nextTick(emitError, self, er);
}

startListening(self);
Expand Down Expand Up @@ -362,8 +360,7 @@ function doSend(ex, self, ip, buffer, address, port, callback) {
return;
}

self.emit('error', ex);
return;
return process.nextTick(emitError, self, ex);
} else if (!self._handle) {
return;
}
Expand Down Expand Up @@ -526,7 +523,7 @@ Socket.prototype._stopReceiving = function() {
function onMessage(nread, handle, buf, rinfo) {
var self = handle.owner;
if (nread < 0) {
return self.emit('error', errnoException(nread, 'recvmsg'));
return process.nextTick(emitError, self, errnoException(nread, 'recvmsg'));
}
rinfo.size = buf.length; // compatibility
self.emit('message', buf, rinfo);
Expand Down
15 changes: 7 additions & 8 deletions lib/fs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const isWindows = process.platform === 'win32';

const DEBUG = process.env.NODE_DEBUG && /fs/.test(process.env.NODE_DEBUG);
const errnoException = util._errnoException;
const emitError = require('internal/util').emitError;

var printDeprecation;
try {
Expand Down Expand Up @@ -1387,7 +1388,7 @@ function FSWatcher() {
errnoException(status,
`Error watching file ${filename} for changes:`);
error.filename = filename;
self.emit('error', error);
process.nextTick(emitError, self, error);
} else {
self.emit('change', event, filename);
}
Expand Down Expand Up @@ -1852,8 +1853,7 @@ ReadStream.prototype.open = function() {
if (self.autoClose) {
self.destroy();
}
self.emit('error', er);
return;
return process.nextTick(emitError, self, er);
}

self.fd = fd;
Expand Down Expand Up @@ -1907,7 +1907,7 @@ ReadStream.prototype._read = function(n) {
if (self.autoClose) {
self.destroy();
}
self.emit('error', er);
return process.nextTick(emitError, self, er);
} else {
var b = null;
if (bytesRead > 0)
Expand Down Expand Up @@ -1944,7 +1944,7 @@ ReadStream.prototype.close = function(cb) {
function close(fd) {
fs.close(fd || self.fd, function(er) {
if (er)
self.emit('error', er);
process.nextTick(emitError, self, er);
else
self.emit('close');
});
Expand Down Expand Up @@ -2018,8 +2018,7 @@ WriteStream.prototype.open = function() {
if (this.autoClose) {
this.destroy();
}
this.emit('error', er);
return;
return process.nextTick(emitError, this, er);
}

this.fd = fd;
Expand All @@ -2030,7 +2029,7 @@ WriteStream.prototype.open = function() {

WriteStream.prototype._write = function(data, encoding, cb) {
if (!(data instanceof Buffer))
return this.emit('error', new Error('Invalid data'));
return process.nextTick(emitError, this, new Error('Invalid data'));

if (typeof this.fd !== 'number')
return this.once('open', function() {
Expand Down
2 changes: 1 addition & 1 deletion lib/http.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ Client.prototype.request = function(method, path, headers) {
options.agent = self.agent;
var c = new ClientRequest(options);
c.on('error', function(e) {
self.emit('error', e);
process.nextTick(internalUtil.emitError, self, e);
});
// The old Client interface emitted 'end' on socket end.
// This doesn't map to how we want things to operate in the future
Expand Down
14 changes: 8 additions & 6 deletions lib/internal/child_process.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ const errnoException = util._errnoException;
const SocketListSend = SocketList.SocketListSend;
const SocketListReceive = SocketList.SocketListReceive;

const emitError = require('internal/util').emitError;

module.exports = {
ChildProcess,
setupChannel,
Expand Down Expand Up @@ -199,7 +201,7 @@ function ChildProcess() {
err.path = self.spawnfile;

err.spawnargs = self.spawnargs.slice(1);
self.emit('error', err);
process.nextTick(emitError, self, err);
} else {
self.emit('exit', self.exitCode, self.signalCode);
}
Expand Down Expand Up @@ -378,7 +380,7 @@ ChildProcess.prototype.kill = function(sig) {
throw errnoException(err, 'kill');
} else {
/* Other error, almost certainly EPERM. */
this.emit('error', errnoException(err, 'kill'));
process.nextTick(emitError, this, errnoException(err, 'kill'));
}
}

Expand Down Expand Up @@ -524,7 +526,7 @@ function setupChannel(target, channel) {
if (typeof callback === 'function') {
process.nextTick(callback, ex);
} else {
this.emit('error', ex); // FIXME(bnoordhuis) Defer to next tick.
process.nextTick(emitError, this, ex);
}
return false;
};
Expand Down Expand Up @@ -635,7 +637,7 @@ function setupChannel(target, channel) {
if (typeof callback === 'function') {
process.nextTick(callback, ex);
} else {
this.emit('error', ex); // FIXME(bnoordhuis) Defer to next tick.
process.nextTick(emitError, this, ex);
}
}
}
Expand All @@ -657,8 +659,8 @@ function setupChannel(target, channel) {

target.disconnect = function() {
if (!this.connected) {
this.emit('error', new Error('IPC channel is already disconnected'));
return;
const err = new Error('IPC channel is already disconnected');
return process.nextTick(emitError, this, err);
}

// Do not allow any new messages to be written.
Expand Down
8 changes: 8 additions & 0 deletions lib/internal/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,11 @@ exports.assertCrypto = function(exports) {
if (noCrypto)
throw new Error('Node.js is not compiled with openssl crypto support');
};

exports.emitError = function emitError(emitter, err, arg) {
if (arguments.length === 3) {
emitter.emit('error', err, arg);
} else {
emitter.emit('error', err);
}
};
Loading