From 524b7134e81db83552d58c5f4aad5a388081de8c Mon Sep 17 00:00:00 2001
From: Benjamin Gruenbaum <benjamingr@gmail.com>
Date: Mon, 9 Nov 2020 12:51:14 +0200
Subject: [PATCH] http: add support for abortsignal to http.request

PR-URL: https://github.com/nodejs/node/pull/36048
Backport-PR-URL: https://github.com/nodejs/node/pull/38386
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Ricky Zhou <0x19951125@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 doc/api/http.md                               |  9 +++++++
 lib/.eslintrc.yaml                            |  2 +-
 lib/_http_client.js                           | 16 +++++++++--
 lib/internal/errors.js                        | 11 ++++++++
 .../test-http-client-abort-destroy.js         | 27 +++++++++++++++++--
 5 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/doc/api/http.md b/doc/api/http.md
index a48ec63381c0c3..a17c7246c52c3e 100644
--- a/doc/api/http.md
+++ b/doc/api/http.md
@@ -2326,6 +2326,9 @@ This can be overridden for servers and client requests by passing the
 <!-- YAML
 added: v0.3.6
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/36048
+    description: It is possible to abort a request with an AbortSignal.
   - version:
      - v13.8.0
      - v12.15.0
@@ -2393,6 +2396,8 @@ changes:
      or `port` is specified, those specify a TCP Socket).
   * `timeout` {number}: A number specifying the socket timeout in milliseconds.
     This will set the timeout before the socket is connected.
+  * `signal` {AbortSignal}: An AbortSignal that may be used to abort an ongoing
+    request.
 * `callback` {Function}
 * Returns: {http.ClientRequest}
 
@@ -2580,6 +2585,10 @@ events will be emitted in the following order:
 Setting the `timeout` option or using the `setTimeout()` function will
 not abort the request or do anything besides add a `'timeout'` event.
 
+Passing an `AbortSignal` and then calling `abort` on the corresponding
+`AbortController` will behave the same way as calling `.destroy()` on the
+request itself.
+
 ## `http.validateHeaderName(name)`
 <!-- YAML
 added: v14.3.0
diff --git a/lib/.eslintrc.yaml b/lib/.eslintrc.yaml
index 20e01898695507..6bbdafab632fdd 100644
--- a/lib/.eslintrc.yaml
+++ b/lib/.eslintrc.yaml
@@ -95,7 +95,7 @@ rules:
     - selector: "ThrowStatement > CallExpression[callee.name=/Error$/]"
       message: "Use new keyword when throwing an Error."
     # Config specific to lib
-    - selector: "NewExpression[callee.name=/Error$/]:not([callee.name=/^(AssertionError|NghttpError)$/])"
+    - selector: "NewExpression[callee.name=/Error$/]:not([callee.name=/^(AssertionError|NghttpError|AbortError)$/])"
       message: "Use an error exported by the internal/errors module."
     - selector: "CallExpression[callee.object.name='Error'][callee.property.name='captureStackTrace']"
       message: "Please use `require('internal/errors').hideStackFrames()` instead."
diff --git a/lib/_http_client.js b/lib/_http_client.js
index 7b1232f33f0a9b..0ff5ab23727959 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -51,7 +51,7 @@ const { Buffer } = require('buffer');
 const { defaultTriggerAsyncIdScope } = require('internal/async_hooks');
 const { URL, urlToOptions, searchParamsSymbol } = require('internal/url');
 const { kOutHeaders, kNeedDrain } = require('internal/http');
-const { connResetException, codes } = require('internal/errors');
+const { AbortError, connResetException, codes } = require('internal/errors');
 const {
   ERR_HTTP_HEADERS_SENT,
   ERR_INVALID_ARG_TYPE,
@@ -59,7 +59,10 @@ const {
   ERR_INVALID_PROTOCOL,
   ERR_UNESCAPED_CHARACTERS
 } = codes;
-const { validateInteger } = require('internal/validators');
+const {
+  validateInteger,
+  validateAbortSignal,
+} = require('internal/validators');
 const { getTimerDuration } = require('internal/timers');
 const {
   DTRACE_HTTP_CLIENT_REQUEST,
@@ -169,6 +172,15 @@ function ClientRequest(input, options, cb) {
   if (options.timeout !== undefined)
     this.timeout = getTimerDuration(options.timeout, 'timeout');
 
+  const signal = options.signal;
+  if (signal) {
+    validateAbortSignal(signal, 'options.signal');
+    const listener = (e) => this.destroy(new AbortError());
+    signal.addEventListener('abort', listener);
+    this.once('close', () => {
+      signal.removeEventListener('abort', listener);
+    });
+  }
   let method = options.method;
   const methodIsString = (typeof method === 'string');
   if (method !== null && method !== undefined && !methodIsString) {
diff --git a/lib/internal/errors.js b/lib/internal/errors.js
index b0c8c0a130259e..0ee39432e38ae1 100644
--- a/lib/internal/errors.js
+++ b/lib/internal/errors.js
@@ -704,6 +704,16 @@ const fatalExceptionStackEnhancers = {
   }
 };
 
+// Node uses an AbortError that isn't exactly the same as the DOMException
+// to make usage of the error in userland and readable-stream easier.
+// It is a regular error with `.code` and `.name`.
+class AbortError extends Error {
+  constructor() {
+    super('The operation was aborted');
+    this.code = 'ABORT_ERR';
+    this.name = 'AbortError';
+  }
+}
 module.exports = {
   addCodeToName, // Exported for NghttpError
   codes,
@@ -718,6 +728,7 @@ module.exports = {
   uvException,
   uvExceptionWithHostPort,
   SystemError,
+  AbortError,
   // This is exported only to facilitate testing.
   E,
   kNoOverride,
diff --git a/test/parallel/test-http-client-abort-destroy.js b/test/parallel/test-http-client-abort-destroy.js
index 6db2ea5682e922..f46d10d367106e 100644
--- a/test/parallel/test-http-client-abort-destroy.js
+++ b/test/parallel/test-http-client-abort-destroy.js
@@ -1,3 +1,4 @@
+// Flags: --experimental-abortcontroller
 'use strict';
 const common = require('../common');
 const http = require('http');
@@ -52,8 +53,7 @@ const assert = require('assert');
 {
   // destroy
 
-  const server = http.createServer(common.mustNotCall((req, res) => {
-  }));
+  const server = http.createServer(common.mustNotCall());
 
   server.listen(0, common.mustCall(() => {
     const options = { port: server.address().port };
@@ -69,3 +69,26 @@ const assert = require('assert');
     assert.strictEqual(req.destroyed, true);
   }));
 }
+
+
+{
+  // Destroy with AbortSignal
+
+  const server = http.createServer(common.mustNotCall());
+  const controller = new AbortController();
+
+  server.listen(0, common.mustCall(() => {
+    const options = { port: server.address().port, signal: controller.signal };
+    const req = http.get(options, common.mustNotCall());
+    req.on('error', common.mustCall((err) => {
+      assert.strictEqual(err.code, 'ABORT_ERR');
+      assert.strictEqual(err.name, 'AbortError');
+      server.close();
+    }));
+    assert.strictEqual(req.aborted, false);
+    assert.strictEqual(req.destroyed, false);
+    controller.abort();
+    assert.strictEqual(req.aborted, false);
+    assert.strictEqual(req.destroyed, true);
+  }));
+}