Skip to content

Commit a699105

Browse files
Uzlopakmetcoder95
andauthored
chore(H2): onboard H2 into Undici queueing system (#3707) (#3724)
(cherry picked from commit d6c44f3) Co-authored-by: Carlos Fuentes <[email protected]>
1 parent 39c5974 commit a699105

File tree

4 files changed

+224
-181
lines changed

4 files changed

+224
-181
lines changed

lib/dispatcher/client-h2.js

+56-19
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ const {
2424
kOnError,
2525
kMaxConcurrentStreams,
2626
kHTTP2Session,
27-
kResume
27+
kResume,
28+
kSize,
29+
kHTTPContext
2830
} = require('../core/symbols.js')
2931

3032
const kOpenStreams = Symbol('open streams')
@@ -160,11 +162,10 @@ async function connectH2 (client, socket) {
160162
version: 'h2',
161163
defaultPipelining: Infinity,
162164
write (...args) {
163-
// TODO (fix): return
164-
writeH2(client, ...args)
165+
return writeH2(client, ...args)
165166
},
166167
resume () {
167-
168+
resumeH2(client)
168169
},
169170
destroy (err, callback) {
170171
if (closed) {
@@ -183,6 +184,20 @@ async function connectH2 (client, socket) {
183184
}
184185
}
185186

187+
function resumeH2 (client) {
188+
const socket = client[kSocket]
189+
190+
if (socket?.destroyed === false) {
191+
if (client[kSize] === 0 && client[kMaxConcurrentStreams] === 0) {
192+
socket.unref()
193+
client[kHTTP2Session].unref()
194+
} else {
195+
socket.ref()
196+
client[kHTTP2Session].ref()
197+
}
198+
}
199+
}
200+
186201
function onHttp2SessionError (err) {
187202
assert(err.code !== 'ERR_TLS_CERT_ALTNAME_INVALID')
188203

@@ -210,17 +225,32 @@ function onHttp2SessionEnd () {
210225
* along with the socket right away
211226
*/
212227
function onHTTP2GoAway (code) {
213-
const err = new RequestAbortedError(`HTTP/2: "GOAWAY" frame received with code ${code}`)
228+
// We cannot recover, so best to close the session and the socket
229+
const err = this[kError] || new SocketError(`HTTP/2: "GOAWAY" frame received with code ${code}`, util.getSocketInfo(this))
230+
const client = this[kClient]
214231

215-
// We need to trigger the close cycle right away
216-
// We need to destroy the session and the socket
217-
// Requests should be failed with the error after the current one is handled
218-
this[kSocket][kError] = err
219-
this[kClient][kOnError](err)
232+
client[kSocket] = null
233+
client[kHTTPContext] = null
220234

221-
this.unref()
235+
if (this[kHTTP2Session] != null) {
236+
this[kHTTP2Session].destroy(err)
237+
this[kHTTP2Session] = null
238+
}
222239

223240
util.destroy(this[kSocket], err)
241+
242+
// Fail head of pipeline.
243+
const request = client[kQueue][client[kRunningIdx]]
244+
client[kQueue][client[kRunningIdx]++] = null
245+
util.errorRequest(client, request, err)
246+
247+
client[kPendingIdx] = client[kRunningIdx]
248+
249+
assert(client[kRunning] === 0)
250+
251+
client.emit('disconnect', client[kUrl], [client], err)
252+
253+
client[kResume]()
224254
}
225255

226256
// https://www.rfc-editor.org/rfc/rfc7230#section-3.3.2
@@ -237,10 +267,6 @@ function writeH2 (client, request) {
237267
return false
238268
}
239269

240-
if (request.aborted) {
241-
return false
242-
}
243-
244270
const headers = {}
245271
for (let n = 0; n < reqHeaders.length; n += 2) {
246272
const key = reqHeaders[n + 0]
@@ -283,6 +309,8 @@ function writeH2 (client, request) {
283309
// We do not destroy the socket as we can continue using the session
284310
// the stream get's destroyed and the session remains to create new streams
285311
util.destroy(body, err)
312+
client[kQueue][client[kRunningIdx]++] = null
313+
client[kResume]()
286314
}
287315

288316
try {
@@ -293,6 +321,10 @@ function writeH2 (client, request) {
293321
util.errorRequest(client, request, err)
294322
}
295323

324+
if (request.aborted) {
325+
return false
326+
}
327+
296328
if (method === 'CONNECT') {
297329
session.ref()
298330
// We are already connected, streams are pending, first request
@@ -304,10 +336,12 @@ function writeH2 (client, request) {
304336
if (stream.id && !stream.pending) {
305337
request.onUpgrade(null, null, stream)
306338
++session[kOpenStreams]
339+
client[kQueue][client[kRunningIdx]++] = null
307340
} else {
308341
stream.once('ready', () => {
309342
request.onUpgrade(null, null, stream)
310343
++session[kOpenStreams]
344+
client[kQueue][client[kRunningIdx]++] = null
311345
})
312346
}
313347

@@ -428,17 +462,20 @@ function writeH2 (client, request) {
428462
// Present specially when using pipeline or stream
429463
if (stream.state?.state == null || stream.state.state < 6) {
430464
request.onComplete([])
431-
return
432465
}
433466

434-
// Stream is closed or half-closed-remote (6), decrement counter and cleanup
435-
// It does not have sense to continue working with the stream as we do not
436-
// have yet RST_STREAM support on client-side
437467
if (session[kOpenStreams] === 0) {
468+
// Stream is closed or half-closed-remote (6), decrement counter and cleanup
469+
// It does not have sense to continue working with the stream as we do not
470+
// have yet RST_STREAM support on client-side
471+
438472
session.unref()
439473
}
440474

441475
abort(new InformationalError('HTTP/2: stream half-closed (remote)'))
476+
client[kQueue][client[kRunningIdx]++] = null
477+
client[kPendingIdx] = client[kRunningIdx]
478+
client[kResume]()
442479
})
443480

444481
stream.once('close', () => {

package.json

+3
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@
7878
"test:fuzzing": "node test/fuzzing/fuzzing.test.js",
7979
"test:fetch": "npm run build:node && npm run test:fetch:nobuild",
8080
"test:fetch:nobuild": "borp --timeout 180000 --expose-gc --concurrency 1 -p \"test/fetch/*.js\" && npm run test:webidl && npm run test:busboy",
81+
"test:h2": "npm run test:h2:core && npm run test:h2:fetch",
82+
"test:h2:core": "borp -p \"test/http2*.js\"",
83+
"test:h2:fetch": "npm run build:node && borp -p \"test/fetch/http2*.js\"",
8184
"test:interceptors": "borp -p \"test/interceptors/*.js\"",
8285
"test:jest": "cross-env NODE_V8_COVERAGE= jest",
8386
"test:unit": "borp --expose-gc -p \"test/*.js\"",

0 commit comments

Comments
 (0)