HTTP2 support rarely breaks

[fengkx]

Describe the bug

• Node.js version: 12.16.1 & 13.13.0
• OS & version: Linux 5.5.18-1-MANJARO

Actual behavior

/home/fengkx/demos/got-error/node_modules/http2-wrapper/source/agent.js:111
        for (const session of where[name]) {
                                   ^

TypeError: where[name] is not iterable
    at closeSessionIfCovered (/home/fengkx/demos/got-error/node_modules/http2-wrapper/source/agent.js:111:29)
    at ClientHttp2Stream.<anonymous> (/home/fengkx/demos/got-error/node_modules/http2-wrapper/source/agent.js:524:9)
    at Object.onceWrapper (events.js:417:28)
    at ClientHttp2Stream.emit (events.js:323:22)
    at emitCloseNT (internal/streams/destroy.js:69:8)
    at processTicksAndRejections (internal/process/task_queues.js:83:21)

Get TypeError

Expected behavior

Get a HTTPError or RequestError

Code to reproduce

const got = require('got');
const pMap = require('p-map');

(async () => {
        const url = 'https://httpbin.org/status/404';
        const urls = Array(200).fill(url);
        const statusCodes = await pMap(urls, async url => {
                try {
                        const resp = await got(url, {http2: true}); // always 404 HTTPError
                        return resp.statusCode;
                } catch(e) {
                        return e.name;
                }
        }, {cocurrency: 25});
        console.log(statusCodes)
})();

I can get Error back when http2 options is false.

---

I also get a error when I enable http2 in production. But I cannot reproduce it with a few lines of code now.

error: https://rsshub.app/zhihu/hotlist HTTPError: Response code 404 (Not Found)
    at PromisableRequest.<anonymous> (/root/NodeRSSBot/node_modules/got/dist/source/as-promise/index.js:122:28)
    at processTicksAndRejections (internal/process/task_queues.js:85:5) {"timestamp":"2020-04-20T18:32:59.103Z"}
events.js:186
      throw er; // Unhandled 'error' event
      ^

Error: 140328935499648:error:14094123:SSL routines:ssl3_read_bytes:application data after close notify:../deps/openssl/openssl/ssl/record/rec_layer_s3.c:1600:

Emitted 'error' event on TLSSocket instance at:
    at TLSSocket._emitTLSError (_tls_wrap.js:748:10)
    at TLSWrap.onerror (_tls_wrap.js:330:11) {
  library: 'SSL routines',
  function: 'ssl3_read_bytes',
  reason: 'application data after close notify',
  code: 'ERR_SSL_APPLICATION_DATA_AFTER_CLOSE_NOTIFY'
}

It is when I request multi-sites at the same times and many of them will rise HTTPError, then it will rise an uncaughtException
Temporary set http2 option to false fix it.

Checklist

• I have read the documentation.
• I have tried my code with the latest version of Node.js and Got.

[szmarczak]

Thanks for reporting, I'll look at this ASAP.

[szmarczak]

TypeError: where[name] is not iterable seems to be caused by a race condition.
The SSL error is likely to be a bug in the Node.js core.

It is when I request multi-sites at the same times and many of them will rise HTTPError, then it will rise an uncaughtException

I'll test that.

[szmarczak]

TypeError: where[name] is not iterable happens when there are no free HTTP2 sessions and the current session is busy (you cannot use it to make requests anymore). I overlooked this scenario.

It is easily fixable by putting the closeSessionIfCovered(...) in

if (this.freeSessions[normalizedOptions]) {
	// closeSessionIfCovered(...)
}

I'll fix this tomorrow.

--
A note to myself (http2-wrapper): should _destroy utilize the callback fn it has as a second argument?

[szmarczak]

You made a typo in your code:

cocurrency -> concurrency

[szmarczak]

Your bug led me into another bugs in the http2-wrapper module. Gonna fix this tomorrow, need to get some sleep now.

[szmarczak]

Good news: I can reproduce the TypeError: where[name] is not iterable error.

[szmarczak]

Fixed in the http2-wrapper package, will do a release soon.

[szmarczak]

It is when I request multi-sites at the same times and many of them will rise HTTPError, then it will rise an uncaughtException

What uncaughtException are you talking about? The SSL error?

[fengkx]

What uncaughtException are you talking about? The SSL error?

Yes, The SSL error I think. I notice the comment in https://github.com/nodejs/node/blob/v13.8.0/lib/events.js#L182