Skip to content

Commit

Permalink
http: don't double-fire the req error event
Browse files Browse the repository at this point in the history
req.socket._hadError should be set before emitting the error event.

PR-URL: #14659
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
  • Loading branch information
fengmk2 authored and tniessen committed Aug 16, 2017
1 parent e96b949 commit 620ba41
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 4 deletions.
8 changes: 4 additions & 4 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,8 @@ 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());
req.socket._hadError = true;
req.emit('error', createHangUpError());
}

// Too bad. That output wasn't getting written.
Expand All @@ -397,10 +397,10 @@ function socketErrorListener(err) {
debug('SOCKET ERROR:', err.message, err.stack);

if (req) {
req.emit('error', 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;
req.emit('error', err);
}

// Handle any pending data
Expand Down Expand Up @@ -433,8 +433,8 @@ 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());
req.socket._hadError = true;
req.emit('error', createHangUpError());
}
if (parser) {
parser.finish();
Expand All @@ -455,8 +455,8 @@ function socketOnData(d) {
debug('parse error', ret);
freeParser(parser, req, socket);
socket.destroy();
req.emit('error', ret);
req.socket._hadError = true;
req.emit('error', ret);
} else if (parser.incoming && parser.incoming.upgrade) {
// Upgrade or CONNECT
var bytesParsed = ret;
Expand Down
16 changes: 16 additions & 0 deletions test/parallel/test-http-client-req-error-dont-double-fire.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
'use strict';
const assert = require('assert');
const http = require('http');
const common = require('../common');

// not exists host
const host = '*'.repeat(256);
const req = http.get({ host });
const err = new Error('mock unexpected code error');
req.on('error', common.mustCall(() => {
throw err;
}));

process.on('uncaughtException', common.mustCall((e) => {
assert.strictEqual(e, err);
}));

1 comment on commit 620ba41

@tniessen
Copy link
Member

Choose a reason for hiding this comment

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

This is unrelated to the specified PR-URL, it should have been #14333.

Please sign in to comment.