From 336fb18b44761eed9eb52c415ec8610d42d270f8 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
Backport-PR-URL: https://github.com/nodejs/node/pull/38386
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                 | 27 ++++++--
 test/parallel/test-http2-client-destroy.js | 76 +++++++++++++++++++++-
 3 files changed, 106 insertions(+), 6 deletions(-)

diff --git a/doc/api/http2.md b/doc/api/http2.md
index 3e4f66f8cd4c06..f033b50548a1e7 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: v10.10.0
     pr-url: https://github.com/nodejs/node/pull/22466
     description: HTTP/2 is now Stable. Previously, it had been Experimental.
@@ -819,6 +822,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}
 
@@ -855,6 +860,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 f8f88957de36be..3c11cf1adad2ca 100644
--- a/lib/internal/http2/core.js
+++ b/lib/internal/http2/core.js
@@ -94,12 +94,15 @@ const {
     ERR_OUT_OF_RANGE,
     ERR_SOCKET_CLOSED
   },
-  hideStackFrames
+  hideStackFrames,
+  AbortError
 } = require('internal/errors');
-const { validateNumber,
-        validateString,
-        validateUint32,
-        isUint32,
+const {
+  isUint32,
+  validateNumber,
+  validateString,
+  validateUint32,
+  validateAbortSignal,
 } = require('internal/validators');
 const fsPromisesInternal = require('internal/fs/promises');
 const { utcDate } = require('internal/http');
@@ -1666,6 +1669,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 = requestOnConnect.bind(stream, headersList, options);
     if (this.connecting) {
       if (this[kPendingRequestCalls] !== null) {
diff --git a/test/parallel/test-http2-client-destroy.js b/test/parallel/test-http2-client-destroy.js
index 12da903c6535a0..0336c7910fa6a6 100644
--- a/test/parallel/test-http2-client-destroy.js
+++ b/test/parallel/test-http2-client-destroy.js
@@ -1,4 +1,4 @@
-// Flags: --expose-internals
+// Flags: --expose-internals --experimental-abortcontroller
 
 'use strict';
 
@@ -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');
 
 {
@@ -165,3 +166,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()));
+  }));
+}