diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md
index 50303baa78207f..8c17d434c398f9 100644
--- a/doc/api/deprecations.md
+++ b/doc/api/deprecations.md
@@ -2548,6 +2548,33 @@ APIs that do not make sense to use in userland. File streams should always be
opened through their corresponding factory methods [`fs.createWriteStream()`][]
and [`fs.createReadStream()`][]) or by passing a file descriptor in options.
+
+### DEP0XXX: ClientRequest.abort() is the same ClientRequest.destroy()
+
+
+Type: Documentation-only
+
+[`ClientRequest.destroy()`][] is the same as [`ClientRequest.abort()`][].
+
+
+### DEP0XXX: ClientRequest.aborted is the same ClientRequest.destroyed
+
+
+Type: Documentation-only
+
+[`ClientRequest.destroyed`][] is the same as [`ClientRequest.aborted`][].
+
+
[`--pending-deprecation`]: cli.html#cli_pending_deprecation
[`--throw-deprecation`]: cli.html#cli_throw_deprecation
[`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size
@@ -2555,6 +2582,8 @@ and [`fs.createReadStream()`][]) or by passing a file descriptor in options.
[`Buffer.from(buffer)`]: buffer.html#buffer_class_method_buffer_from_buffer
[`Buffer.isBuffer()`]: buffer.html#buffer_class_method_buffer_isbuffer_obj
[`Cipher`]: crypto.html#crypto_class_cipher
+[`ClientRequest.abort()`]: http.html#http_request_abort
+[`ClientRequest.destroy()`]: stream.html#stream_readable_destroy_error
[`Decipher`]: crypto.html#crypto_class_decipher
[`EventEmitter.listenerCount(emitter, eventName)`]: events.html#events_eventemitter_listenercount_emitter_eventname
[`REPLServer.clearBufferedCommand()`]: repl.html#repl_replserver_clearbufferedcommand
diff --git a/doc/api/http.md b/doc/api/http.md
index 44bea87e3bc6ca..3e12ae773916b3 100644
--- a/doc/api/http.md
+++ b/doc/api/http.md
@@ -565,10 +565,14 @@ srv.listen(1337, '127.0.0.1', () => {
### request.abort()
+> Stability: 0 - Deprecated. Use [`request.destroy()`][] instead.
+
Marks the request as aborting. Calling this will cause remaining data
-in the response to be dropped and the socket to be destroyed.
+in the response to be dropped and the socket to be destroyed. After
+calling this method, no further errors will be emitted.
### request.aborted
+> Stability: 0 - Deprecated. Use [`request.destroyed`][] instead.
+
* {boolean}
The `request.aborted` property will be `true` if the request has
@@ -2319,43 +2325,24 @@ In the case of a connection error, the following events will be emitted:
* `'error'`
* `'close'`
-In the case of a premature connection close before the response is received,
-the following events will be emitted in the following order:
-
-* `'socket'`
-* `'error'` with an error with message `'Error: socket hang up'` and code
- `'ECONNRESET'`
-* `'close'`
-
-In the case of a premature connection close after the response is received,
-the following events will be emitted in the following order:
-
-* `'socket'`
-* `'response'`
- * `'data'` any number of times, on the `res` object
-* (connection closed here)
-* `'aborted'` on the `res` object
-* `'close'`
-* `'close'` on the `res` object
-
-If `req.abort()` is called before the connection succeeds, the following events
-will be emitted in the following order:
+If `req.destroy()` is called before the connection succeeds, the following
+events will be emitted in the following order:
* `'socket'`
-* (`req.abort()` called here)
+* (`req.destroy(err)` called here)
* `'abort'`
-* `'error'` with an error with message `'Error: socket hang up'` and code
- `'ECONNRESET'`
+* `'error'` if `err` was provided in `req.destroy(err)`.
* `'close'`
-If `req.abort()` is called after the response is received, the following events
-will be emitted in the following order:
+If `req.destroy()` is called after the response is received, the following
+events will be emitted in the following order:
* `'socket'`
* `'response'`
* `'data'` any number of times, on the `res` object
-* (`req.abort()` called here)
+* (`req.destroy(err)` called here)
* `'abort'`
+* `'error'` if `err` was provided in `req.destroy(err)`.
* `'aborted'` on the `res` object
* `'close'`
* `'close'` on the `res` object
@@ -2392,6 +2379,8 @@ not abort the request or do anything besides add a `'timeout'` event.
[`net.createConnection()`]: net.html#net_net_createconnection_options_connectlistener
[`new URL()`]: url.html#url_constructor_new_url_input_base
[`removeHeader(name)`]: #http_request_removeheader_name
+[`request.destroy()`]: stream.html#stream_readable_destroy_error
+[`request.destroyed`]: stream.html#stream_readable_destroyed
[`request.end()`]: #http_request_end_data_encoding_callback
[`request.flushHeaders()`]: #http_request_flushheaders
[`request.getHeader()`]: #http_request_getheader_name
diff --git a/lib/_http_client.js b/lib/_http_client.js
index 30a6366f355708..908cc84cefb8fe 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -23,6 +23,7 @@
const { Object } = primordials;
+const { destroy, kWritableState } = require('internal/streams/destroy');
const net = require('net');
const url = require('url');
const assert = require('internal/assert');
@@ -190,6 +191,9 @@ function ClientRequest(input, options, cb) {
this._ended = false;
this.res = null;
+ // The aborted property is for backwards compat and is not
+ // strictly the same as destroyed as it can be written
+ // to by the user.
this.aborted = false;
this.timeoutCb = null;
this.upgradeOrConnect = false;
@@ -197,7 +201,14 @@ function ClientRequest(input, options, cb) {
this.maxHeadersCount = null;
this.reusedSocket = false;
- let called = false;
+ // Used by destroyImpl.
+ this[kWritableState] = {
+ errorEmitted: false,
+ destroyed: false,
+ emitClose: true,
+ socketPending: true,
+ destroyCallback: null
+ };
if (this.agent) {
// If there is an agent we should default to Connection:keep-alive,
@@ -261,11 +272,17 @@ function ClientRequest(input, options, cb) {
}
const oncreate = (err, socket) => {
- if (called)
+ const state = this[kWritableState];
+ if (!state.socketPending)
return;
- called = true;
+ state.socketPending = false;
if (err) {
- process.nextTick(() => this.emit('error', err));
+ const { destroyCallback: cb } = state;
+ if (cb) {
+ cb(err);
+ } else {
+ this.destroy(err);
+ }
return;
}
this.onSocket(socket);
@@ -280,9 +297,10 @@ function ClientRequest(input, options, cb) {
this._last = true;
this.shouldKeepAlive = false;
if (typeof options.createConnection === 'function') {
+ const state = this[kWritableState];
const newSocket = options.createConnection(options, oncreate);
- if (newSocket && !called) {
- called = true;
+ if (newSocket && state.socketPending) {
+ state.socketPending = false;
this.onSocket(newSocket);
} else {
return;
@@ -298,6 +316,12 @@ function ClientRequest(input, options, cb) {
Object.setPrototypeOf(ClientRequest.prototype, OutgoingMessage.prototype);
Object.setPrototypeOf(ClientRequest, OutgoingMessage);
+Object.defineProperty(ClientRequest.prototype, 'destroyed', {
+ get() {
+ return this[kWritableState].destroyed;
+ }
+});
+
ClientRequest.prototype._finish = function _finish() {
DTRACE_HTTP_CLIENT_REQUEST(this, this.socket);
OutgoingMessage.prototype._finish.call(this);
@@ -311,28 +335,37 @@ ClientRequest.prototype._implicitHeader = function _implicitHeader() {
this[kOutHeaders]);
};
-ClientRequest.prototype.abort = function abort() {
- if (!this.aborted) {
- process.nextTick(emitAbortNT.bind(this));
- }
+ClientRequest.prototype.destroy = destroy;
+ClientRequest.prototype._destroy = function(err, cb) {
this.aborted = true;
+ process.nextTick(emitAbortNT, this);
// If we're aborting, we don't care about any more response data.
if (this.res) {
this.res._dump();
}
- // In the event that we don't have a socket, we will pop out of
- // the request queue through handling in onSocket.
- if (this.socket) {
+ if (this.upgradeOrConnect) {
+ // We're detached from socket.
+ cb(err);
+ } else if (this.socket) {
// in-progress
- this.socket.destroy();
+ this.socket.destroy(err, cb);
+ } else if (this[kWritableState].socketPending) {
+ // In the event that we don't have a socket, we will pop out of
+ // the request queue through handling in onSocket.
+ this[kWritableState].destroyCallback = (er) => cb(er || err);
+ } else {
+ cb(err);
}
};
+ClientRequest.prototype.abort = function abort() {
+ this.destroy();
+};
-function emitAbortNT() {
- this.emit('abort');
+function emitAbortNT(self) {
+ self.emit('abort');
}
function ondrain() {
@@ -363,24 +396,23 @@ function socketCloseListener() {
res.aborted = true;
res.emit('aborted');
}
- req.emit('close');
if (!res.aborted && res.readable) {
res.on('end', function() {
+ // We can only destroy req after 'end'. Otherwise we will dump the
+ // data.
+ req.destroy();
this.emit('close');
});
res.push(null);
} else {
+ req.destroy();
res.emit('close');
}
} else {
- if (!req.socket._hadError) {
- // This socket error fired before we started to
- // receive a response. The error needs to
- // fire on the request.
- req.socket._hadError = true;
- req.emit('error', connResetException('socket hang up'));
- }
- req.emit('close');
+ // This socket error fired before we started to
+ // receive a response. The error needs to
+ // fire on the request.
+ req.destroy(connResetException('socket hang up'));
}
// Too bad. That output wasn't getting written.
@@ -400,13 +432,6 @@ function socketErrorListener(err) {
const req = socket._httpMessage;
debug('SOCKET ERROR:', err.message, err.stack);
- if (req) {
- // 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);
- }
-
const parser = socket.parser;
if (parser) {
parser.finish();
@@ -416,7 +441,7 @@ function socketErrorListener(err) {
// Ensure that no further data will come out of the socket
socket.removeListener('data', socketOnData);
socket.removeListener('end', socketOnEnd);
- socket.destroy();
+ req.destroy(err);
}
function freeSocketErrorListener(err) {
@@ -431,17 +456,15 @@ function socketOnEnd() {
const req = this._httpMessage;
const parser = this.parser;
- 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.socket._hadError = true;
- req.emit('error', connResetException('socket hang up'));
- }
if (parser) {
parser.finish();
freeParser(parser, req, socket);
}
- socket.destroy();
+
+ // 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.
+ const err = !req.res ? connResetException('socket hang up') : null;
+ req.destroy(err);
}
function socketOnData(d) {
@@ -456,9 +479,7 @@ function socketOnData(d) {
prepareError(ret, parser, d);
debug('parse error', ret);
freeParser(parser, req, socket);
- socket.destroy();
- req.socket._hadError = true;
- req.emit('error', ret);
+ req.destroy(ret);
} else if (parser.incoming && parser.incoming.upgrade) {
// Upgrade (if status code 101) or CONNECT
const bytesParsed = ret;
@@ -490,10 +511,10 @@ function socketOnData(d) {
socket.readableFlowing = null;
req.emit(eventName, res, socket, bodyHead);
- req.emit('close');
+ req.destroy();
} else {
// Requested Upgrade or used CONNECT method, but have no handler.
- socket.destroy();
+ req.destroy();
}
} else if (parser.incoming && parser.incoming.complete &&
// When the status code is informational (100, 102-199),
@@ -582,7 +603,7 @@ function parserOnIncomingClient(res, shouldKeepAlive) {
// If the user did not listen for the 'response' event, then they
// can't possibly read the data, so we ._dump() it into the void
// so that the socket doesn't hang there in a paused state.
- if (req.aborted || !req.emit('response', res))
+ if (req.destroyed || !req.emit('response', res))
res._dump();
if (method === 'HEAD')
@@ -720,10 +741,11 @@ ClientRequest.prototype.onSocket = function onSocket(socket) {
};
function onSocketNT(req, socket) {
- if (req.aborted) {
+ if (req.destroyed) {
+ const { destroyCallback: cb } = req[kWritableState];
// If we were aborted while waiting for a socket, skip the whole thing.
if (!req.agent) {
- socket.destroy();
+ socket.destroy(null, cb);
} else {
req.emit('close');
socket.emit('free');
diff --git a/lib/internal/streams/destroy.js b/lib/internal/streams/destroy.js
index ded75b255934c4..7be8716753f323 100644
--- a/lib/internal/streams/destroy.js
+++ b/lib/internal/streams/destroy.js
@@ -1,12 +1,15 @@
'use strict';
+const kWritableState = Symbol('kWritableState');
+const kReadableState = Symbol('kReadableState');
+
function needError(stream, err) {
if (!err) {
return false;
}
- const r = stream._readableState;
- const w = stream._writableState;
+ const r = stream[kReadableState] || stream._readableState;
+ const w = stream[kWritableState] || stream._writableState;
if ((w && w.errorEmitted) || (r && r.errorEmitted)) {
return false;
@@ -25,8 +28,8 @@ function needError(stream, err) {
// Undocumented cb() API, needed for core, not for public API.
// The cb() will be invoked synchronously if _destroy is synchronous.
function destroy(err, cb) {
- const r = this._readableState;
- const w = this._writableState;
+ const r = this[kReadableState] || this._readableState;
+ const w = this[kWritableState] || this._writableState;
if (w && err) {
w.errored = true;
@@ -85,8 +88,8 @@ function emitErrorNT(self, err) {
}
function undestroy() {
- const r = this._readableState;
- const w = this._writableState;
+ const r = this[kReadableState] || this._readableState;
+ const w = this[kWritableState] || this._writableState;
if (r) {
r.destroyed = false;
@@ -115,8 +118,8 @@ function errorOrDestroy(stream, err) {
// the error to be emitted nextTick. In a future
// semver major update we should change the default to this.
- const r = stream._readableState;
- const w = stream._writableState;
+ const r = stream[kReadableState] || stream._readableState;
+ const w = stream[kWritableState] || stream._writableState;
if (w & err) {
w.errored = true;
@@ -132,5 +135,7 @@ function errorOrDestroy(stream, err) {
module.exports = {
destroy,
undestroy,
- errorOrDestroy
+ errorOrDestroy,
+ kWritableState,
+ kReadableState
};
diff --git a/test/parallel/test-http-agent-uninitialized-with-handle.js b/test/parallel/test-http-agent-uninitialized-with-handle.js
index 77f01771734c87..93765c858c6a77 100644
--- a/test/parallel/test-http-agent-uninitialized-with-handle.js
+++ b/test/parallel/test-http-agent-uninitialized-with-handle.js
@@ -12,6 +12,7 @@ const socket = new net.Socket();
socket._handle = {
ref() { },
readStart() { },
+ close() {}
};
const server = http.createServer(common.mustCall((req, res) => {
diff --git a/test/parallel/test-http-client-aborted.js b/test/parallel/test-http-client-aborted.js
new file mode 100644
index 00000000000000..5a9f68a93a0e3e
--- /dev/null
+++ b/test/parallel/test-http-client-aborted.js
@@ -0,0 +1,57 @@
+'use strict';
+
+const common = require('../common');
+const http = require('http');
+const assert = require('assert');
+const EventEmitter = require('events')
+
+{
+ const server = http.createServer(common.mustCall(function(req, res) {
+ req.on('aborted', common.mustCall(function() {
+ assert.strictEqual(this.aborted, true);
+ server.close();
+ }));
+ assert.strictEqual(req.aborted, false);
+ res.write('hello');
+ }));
+
+ server.listen(0, common.mustCall(() => {
+ const req = http.get({
+ port: server.address().port,
+ headers: { connection: 'keep-alive' }
+ }, common.mustCall((res) => {
+ req.destroy(new Error('kaboom'));
+ req.on('error', common.mustCall());
+ assert.strictEqual(req.destroyed, true);
+ assert.strictEqual(req.aborted, true);
+ }));
+ }));
+}
+
+{
+ const req = http.request({
+ createConnection: (_, callback) => {
+ process.nextTick(callback, new Error('kaboom'));
+ }
+ });
+ req.on('socket', common.mustNotCall());
+ req.destroy();
+ req.on('error', common.mustCall(common.expectsError({
+ message: 'kaboom'
+ })));
+}
+
+{
+ // Make sure socket is destroyed if req.destroy
+ // is called before socket is ready.
+ const dummySocket = new EventEmitter();
+ dummySocket.destroy = common.mustCall();
+ dummySocket.on('free', common.mustNotCall());
+ const req = http.request({
+ createConnection: common.mustCall((_, callback) => {
+ process.nextTick(callback, null, dummySocket);
+ })
+ });
+ req.on('socket', common.mustNotCall());
+ req.destroy();
+}
diff --git a/test/parallel/test-http-client-close-event.js b/test/parallel/test-http-client-close-event.js
index 7573931ac48ef6..4bd0832e4cfb6f 100644
--- a/test/parallel/test-http-client-close-event.js
+++ b/test/parallel/test-http-client-close-event.js
@@ -26,5 +26,7 @@ server.listen(0, common.mustCall(() => {
server.close();
}));
- req.destroy();
+ const err = new Error('socket hang up');
+ err.code = 'ECONNRESET';
+ req.destroy(err);
}));
diff --git a/test/parallel/test-http-client-set-timeout.js b/test/parallel/test-http-client-set-timeout.js
index 15aae7023bf3f1..9d267de6ac8dca 100644
--- a/test/parallel/test-http-client-set-timeout.js
+++ b/test/parallel/test-http-client-set-timeout.js
@@ -43,6 +43,8 @@ server.listen(0, mustCall(() => {
req.on('timeout', mustCall(() => {
strictEqual(req.socket.listenerCount('timeout'), 0);
- req.destroy();
+ const err = new Error('socket hang up');
+ err.code = 'ECONNRESET';
+ req.destroy(err);
}));
}));