http request 'end' event not fired

[gonenduk]

Hi,

I am using the http request and breakdown the response times using events emitted at different stages of the request/response process.
With node < 10 it used to work well. With node >= 10 it has stopped working. The on end event of the response is not called. If I remove the readable event - then it is called.

Here is a sample code of rising stack on this subject (measuring http timings). Their sample also works on node < 10 but has the same problem with node >= 10:
Rising Stack sample code

Was anything changed in the way http calls are made in version 10?

[apapirovski]

@gonenduk there was a change around the semantics, yes. See the following section of the Streams documentation: https://nodejs.org/dist/latest-v10.x/docs/api/stream.html#stream_event_readable

[apapirovski]

ping @mcollina — is this currently working fully as intended?

[gonenduk]

Do we have a document with the changes in version 10 about this?
In the sample code I linked, the end event if not fired on version 10 but it is fired on previous versions.
Need to know what was the breaking change to adopt the code to the changes.

[mcollina]

I have very limited connectivity this week. I can have a look next week. Cc @mafintosh - can you have a look Il giorno mer 20 giu 2018 alle 05:05 Anatoli Papirovski < notifications@github.com> ha scritto:

…

ping @mcollina <https://github.com/mcollina> — is this currently working fully as intended? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#21398 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADL45Y6Mjp1H39PE-ZMWFeN-itPuhC-ks5t-dgegaJpZM4Us7q9> .

[gonenduk]

any updates?

[yosuke-furukawa]

similar issue is opened from my lib.
https://github.com/yosuke-furukawa/httpstat

http is called end event, but https is not called end when readable is not finished.

    const req = protocol.request(url, (res) => {
      res.once('readable', () => {
        onTransfer = Date.now();
      });
      res.on('data', (chunk) => {
        body += chunk;
      });
      res.on('end', () => {
        onTotal = Date.now();
        res.body = body;
        resolve({
          url: url,
          response: res,
          time: {
            begin: begin,
            onLookup: onLookup,
            onConnect: onConnect,
            onSecureConnect: onSecureConnect,
            onTransfer: onTransfer,
            onTotal: onTotal,
          }
        });
      });
    });

[mcollina]

I think there is a bug when using both 'readable' and 'data'.

[gonenduk]

But why is the end event not fired with node 10 and was always fired with 8 and below?

[mcollina]

@gonenduk I think it's due to #18994 possibly in combination of the _readableState manipulation within http. Moreover we are not really testing this use anywhere.

[lundibundi]

@gonenduk until the bug is fixed, if you need it to work you can put listener on data event inside 'readable' event:

    res.once('readable', () => {
      eventTimes.firstByteAt = process.hrtime()
      res.on('data', (chunk) => { responseBody += chunk })
    })

This seems to work fine.