http2: add support for AbortSignal to http2Session.request

- Add support - Add test - Docs once PR is up PR-URL: nodejs#36070 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
targos · Apr 24, 2021 · 8ab4159 · 8ab4159
1 parent e7d3fed
commit 8ab4159
Show file tree

Hide file tree

Showing 3 changed files with 106 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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()));
+  }));
+}