From 48bf59bb8be9e2ff4719fbf34f96b797411431fb Mon Sep 17 00:00:00 2001
From: Madara Uchiha <me@madara.ninja>
Date: Tue, 10 Nov 2020 19:06:41 +0200
Subject: [PATCH] http2: add support for AbortSignal to http2Session.request

- Add support
- Add test
- Docs once PR is up

PR-URL: https://github.com/nodejs/node/pull/36070
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
---
 doc/api/http2.md                           |  9 +++
 lib/internal/http2/core.js                 | 18 +++++-
 test/parallel/test-http2-client-destroy.js | 74 ++++++++++++++++++++++
 3 files changed, 100 insertions(+), 1 deletion(-)

diff --git a/doc/api/http2.md b/doc/api/http2.md
index 9fc447b657b368..8a676d24de73f1 100644
--- a/doc/api/http2.md
+++ b/doc/api/http2.md
@@ -2,6 +2,9 @@
 <!-- YAML
 added: v8.4.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/36070
+    description: It is possible to abort a request with an AbortSignal.
   - version: v15.0.0
     pr-url: https://github.com/nodejs/node/pull/34664
     description: Requests with the `host` header (with or without
@@ -846,6 +849,8 @@ added: v8.4.0
     and `256` (inclusive).
   * `waitForTrailers` {boolean} When `true`, the `Http2Stream` will emit the
     `'wantTrailers'` event after the final `DATA` frame has been sent.
+  * `signal` {AbortSignal} An AbortSignal that may be used to abort an ongoing
+    request.
 
 * Returns: {ClientHttp2Stream}
 
@@ -882,6 +887,10 @@ close when the final `DATA` frame is transmitted. User code must call either
 `http2stream.sendTrailers()` or `http2stream.close()` to close the
 `Http2Stream`.
 
+When `options.signal` is set with an `AbortSignal` and then `abort` on the
+corresponding `AbortController` is called, the request will emit an `'error'`
+event with an `AbortError` error.
+
 The `:method` and `:path` pseudo-headers are not specified within `headers`,
 they respectively default to:
 
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
index 2ea8eacd0c10a2..f685329574efe4 100644
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@@ -109,7 +109,8 @@ const {
     ERR_OUT_OF_RANGE,
     ERR_SOCKET_CLOSED
   },
-  hideStackFrames
+  hideStackFrames,
+  AbortError
 } = require('internal/errors');
 const {
   isUint32,
@@ -118,6 +119,7 @@ const {
   validateNumber,
   validateString,
   validateUint32,
+  validateAbortSignal,
 } = require('internal/validators');
 const fsPromisesInternal = require('internal/fs/promises');
 const { utcDate } = require('internal/http');
@@ -1721,6 +1723,20 @@ class ClientHttp2Session extends Http2Session {
     if (options.waitForTrailers)
       stream[kState].flags |= STREAM_FLAGS_HAS_TRAILERS;
 
+    const { signal } = options;
+    if (signal) {
+      validateAbortSignal(signal, 'options.signal');
+      const aborter = () => stream.destroy(new AbortError());
+      if (signal.aborted) {
+        aborter();
+      } else {
+        signal.addEventListener('abort', aborter);
+        stream.once('close', () => {
+          signal.removeEventListener('abort', aborter);
+        });
+      }
+    }
+
     const onConnect = FunctionPrototypeBind(requestOnConnect,
                                             stream, headersList, options);
     if (this.connecting) {
diff --git a/test/parallel/test-http2-client-destroy.js b/test/parallel/test-http2-client-destroy.js
index c91e88079db6bb..f257cf6f0006fa 100644
--- a/test/parallel/test-http2-client-destroy.js
+++ b/test/parallel/test-http2-client-destroy.js
@@ -8,6 +8,7 @@ if (!common.hasCrypto)
 const assert = require('assert');
 const h2 = require('http2');
 const { kSocket } = require('internal/http2/util');
+const { kEvents } = require('internal/event_target');
 const Countdown = require('../common/countdown');
 
 {
@@ -167,3 +168,76 @@ const Countdown = require('../common/countdown');
     req.on('close', common.mustCall(() => server.close()));
   }));
 }
+
+// Destroy with AbortSignal
+{
+  const server = h2.createServer();
+  const controller = new AbortController();
+
+  server.on('stream', common.mustNotCall());
+  server.listen(0, common.mustCall(() => {
+    const client = h2.connect(`http://localhost:${server.address().port}`);
+    client.on('close', common.mustCall());
+
+    const { signal } = controller;
+    assert.strictEqual(signal[kEvents].get('abort'), undefined);
+
+    client.on('error', common.mustCall(() => {
+      // After underlying stream dies, signal listener detached
+      assert.strictEqual(signal[kEvents].get('abort'), undefined);
+    }));
+
+    const req = client.request({}, { signal });
+
+    req.on('error', common.mustCall((err) => {
+      assert.strictEqual(err.code, 'ABORT_ERR');
+      assert.strictEqual(err.name, 'AbortError');
+    }));
+    req.on('close', common.mustCall(() => server.close()));
+
+    assert.strictEqual(req.aborted, false);
+    assert.strictEqual(req.destroyed, false);
+    // Signal listener attached
+    assert.strictEqual(signal[kEvents].get('abort').size, 1);
+
+    controller.abort();
+
+    assert.strictEqual(req.aborted, false);
+    assert.strictEqual(req.destroyed, true);
+  }));
+}
+// Pass an already destroyed signal to abort immediately.
+{
+  const server = h2.createServer();
+  const controller = new AbortController();
+
+  server.on('stream', common.mustNotCall());
+  server.listen(0, common.mustCall(() => {
+    const client = h2.connect(`http://localhost:${server.address().port}`);
+    client.on('close', common.mustCall());
+
+    const { signal } = controller;
+    controller.abort();
+
+    assert.strictEqual(signal[kEvents].get('abort'), undefined);
+
+    client.on('error', common.mustCall(() => {
+      // After underlying stream dies, signal listener detached
+      assert.strictEqual(signal[kEvents].get('abort'), undefined);
+    }));
+
+    const req = client.request({}, { signal });
+    // Signal already aborted, so no event listener attached.
+    assert.strictEqual(signal[kEvents].get('abort'), undefined);
+
+    assert.strictEqual(req.aborted, false);
+    // Destroyed on same tick as request made
+    assert.strictEqual(req.destroyed, true);
+
+    req.on('error', common.mustCall((err) => {
+      assert.strictEqual(err.code, 'ABORT_ERR');
+      assert.strictEqual(err.name, 'AbortError');
+    }));
+    req.on('close', common.mustCall(() => server.close()));
+  }));
+}