Skip to content

Commit 48bf59b

Browse files
MadaraUchihacodebytere
authored andcommitted
http2: add support for AbortSignal to http2Session.request
- Add support - Add test - Docs once PR is up PR-URL: #36070 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent 79b2ba6 commit 48bf59b

3 files changed

Lines changed: 100 additions & 1 deletion

File tree

‎doc/api/http2.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
<!-- YAML
33
added: v8.4.0
44
changes:
5+
- version: REPLACEME
6+
pr-url: https://github.com/nodejs/node/pull/36070
7+
description: It is possible to abort a request with an AbortSignal.
58
- version: v15.0.0
69
pr-url: https://github.com/nodejs/node/pull/34664
710
description: Requests with the `host` header (with or without
@@ -846,6 +849,8 @@ added: v8.4.0
846849
and `256` (inclusive).
847850
*`waitForTrailers` {boolean} When `true`, the `Http2Stream` will emit the
848851
`'wantTrailers'` event after the final `DATA` frame has been sent.
852+
*`signal` {AbortSignal} An AbortSignal that may be used to abort an ongoing
853+
request.
849854

850855
* Returns: {ClientHttp2Stream}
851856

@@ -882,6 +887,10 @@ close when the final `DATA` frame is transmitted. User code must call either
882887
`http2stream.sendTrailers()` or `http2stream.close()` to close the
883888
`Http2Stream`.
884889

890+
When `options.signal` is set with an `AbortSignal` and then `abort` on the
891+
corresponding `AbortController` is called, the request will emit an `'error'`
892+
event with an `AbortError` error.
893+
885894
The `:method` and `:path` pseudo-headers are not specified within `headers`,
886895
they respectively default to:
887896

‎lib/internal/http2/core.js‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,8 @@ const {
109109
ERR_OUT_OF_RANGE,
110110
ERR_SOCKET_CLOSED
111111
},
112-
hideStackFrames
112+
hideStackFrames,
113+
AbortError
113114
}=require('internal/errors');
114115
const{
115116
isUint32,
@@ -118,6 +119,7 @@ const {
118119
validateNumber,
119120
validateString,
120121
validateUint32,
122+
validateAbortSignal,
121123
}=require('internal/validators');
122124
constfsPromisesInternal=require('internal/fs/promises');
123125
const{ utcDate }=require('internal/http');
@@ -1721,6 +1723,20 @@ class ClientHttp2Session extends Http2Session {
17211723
if(options.waitForTrailers)
17221724
stream[kState].flags|=STREAM_FLAGS_HAS_TRAILERS;
17231725

1726+
const{ signal }=options;
1727+
if(signal){
1728+
validateAbortSignal(signal,'options.signal');
1729+
constaborter=()=>stream.destroy(newAbortError());
1730+
if(signal.aborted){
1731+
aborter();
1732+
}else{
1733+
signal.addEventListener('abort',aborter);
1734+
stream.once('close',()=>{
1735+
signal.removeEventListener('abort',aborter);
1736+
});
1737+
}
1738+
}
1739+
17241740
constonConnect=FunctionPrototypeBind(requestOnConnect,
17251741
stream,headersList,options);
17261742
if(this.connecting){

‎test/parallel/test-http2-client-destroy.js‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ if (!common.hasCrypto)
88
constassert=require('assert');
99
consth2=require('http2');
1010
const{ kSocket }=require('internal/http2/util');
11+
const{ kEvents }=require('internal/event_target');
1112
constCountdown=require('../common/countdown');
1213

1314
{
@@ -167,3 +168,76 @@ const Countdown = require('../common/countdown');
167168
req.on('close',common.mustCall(()=>server.close()));
168169
}));
169170
}
171+
172+
// Destroy with AbortSignal
173+
{
174+
constserver=h2.createServer();
175+
constcontroller=newAbortController();
176+
177+
server.on('stream',common.mustNotCall());
178+
server.listen(0,common.mustCall(()=>{
179+
constclient=h2.connect(`http://localhost:${server.address().port}`);
180+
client.on('close',common.mustCall());
181+
182+
const{ signal }=controller;
183+
assert.strictEqual(signal[kEvents].get('abort'),undefined);
184+
185+
client.on('error',common.mustCall(()=>{
186+
// After underlying stream dies, signal listener detached
187+
assert.strictEqual(signal[kEvents].get('abort'),undefined);
188+
}));
189+
190+
constreq=client.request({},{ signal });
191+
192+
req.on('error',common.mustCall((err)=>{
193+
assert.strictEqual(err.code,'ABORT_ERR');
194+
assert.strictEqual(err.name,'AbortError');
195+
}));
196+
req.on('close',common.mustCall(()=>server.close()));
197+
198+
assert.strictEqual(req.aborted,false);
199+
assert.strictEqual(req.destroyed,false);
200+
// Signal listener attached
201+
assert.strictEqual(signal[kEvents].get('abort').size,1);
202+
203+
controller.abort();
204+
205+
assert.strictEqual(req.aborted,false);
206+
assert.strictEqual(req.destroyed,true);
207+
}));
208+
}
209+
// Pass an already destroyed signal to abort immediately.
210+
{
211+
constserver=h2.createServer();
212+
constcontroller=newAbortController();
213+
214+
server.on('stream',common.mustNotCall());
215+
server.listen(0,common.mustCall(()=>{
216+
constclient=h2.connect(`http://localhost:${server.address().port}`);
217+
client.on('close',common.mustCall());
218+
219+
const{ signal }=controller;
220+
controller.abort();
221+
222+
assert.strictEqual(signal[kEvents].get('abort'),undefined);
223+
224+
client.on('error',common.mustCall(()=>{
225+
// After underlying stream dies, signal listener detached
226+
assert.strictEqual(signal[kEvents].get('abort'),undefined);
227+
}));
228+
229+
constreq=client.request({},{ signal });
230+
// Signal already aborted, so no event listener attached.
231+
assert.strictEqual(signal[kEvents].get('abort'),undefined);
232+
233+
assert.strictEqual(req.aborted,false);
234+
// Destroyed on same tick as request made
235+
assert.strictEqual(req.destroyed,true);
236+
237+
req.on('error',common.mustCall((err)=>{
238+
assert.strictEqual(err.code,'ABORT_ERR');
239+
assert.strictEqual(err.name,'AbortError');
240+
}));
241+
req.on('close',common.mustCall(()=>server.close()));
242+
}));
243+
}

0 commit comments

Comments
 (0)