Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot read property 'emit' of null in _http_client.js #20690

Closed
xloem opened this issue May 12, 2018 · 15 comments
Closed

Cannot read property 'emit' of null in _http_client.js #20690

xloem opened this issue May 12, 2018 · 15 comments
Labels
confirmed-bug Issues with confirmed bugs. http Issues or PRs related to the http subsystem.

Comments

@xloem
Copy link

xloem commented May 12, 2018

  • Version: v6.14.2
  • Platform: Linux clearnet 4.9.56-21.pvops.qubes.x86_64 deps: update openssl to 1.0.1j #1 SMP Tue Oct 17 23:58:50 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
  • Subsystem: http

I'm getting the following error on every run of registry-static:

_http_client.js:277
  req.emit('close');
     ^

TypeError: Cannot read property 'emit' of null
    at TLSSocket.socketCloseListener (_http_client.js:277:6)
    at emitOne (events.js:101:20)
    at TLSSocket.emit (events.js:188:7)
    at _handle.close (net.js:509:12)
    at TCP.done [as _onclose] (_tls_wrap.js:332:7)

It's happening reliably for me at 317673 while processing ng2-comps. I haven't tried with an empty output folder yet because it takes me a few days to get that far into the database.

@xloem
Copy link
Author

xloem commented May 13, 2018

note: problem disappeared when I upgraded to node v8.11.1

@tniessen
Copy link
Member

cc @nodejs/http

@tniessen tniessen added the http Issues or PRs related to the http subsystem. label May 13, 2018
@MylesBorins
Copy link
Member

@xloem have you tried it on earlier versions of 6.x? Is this a new regression?

@xloem
Copy link
Author

xloem commented May 15, 2018

I'm afraid that now I have found a way to make it work I'm not available to test this a lot as it takes so long to trigger it. I'm sorry. An obvious approach would be to git bisect between the two versions, but this would take many weeks to complete.

@jordanrogers
Copy link

I am also seeing this error after upgrading from v8.11.1 to v8.11.2. Scanning the changelog for v8.11.2, the error seems to be plausibly related to #18865, which seems to have landed in both 6.14.2 and 8.11.2

@tniessen
Copy link
Member

Pinging @lpinca.

@lpinca
Copy link
Member

lpinca commented May 16, 2018

Weird, there are only two (1, 2) places where 'agentRemove' is emitted and in both cases (1, 2) the socketCloseListener is removed, so I'm not sure how this can happen.

Is there a way to reproduce the issue without relying on userland modules?

@jordanrogers
Copy link

@lpinca the API docs include an example of how to emit the 'agentRemove' event from userland code to remove an socket from an agent.
https://nodejs.org/dist/latest-v8.x/docs/api/http.html#http_class_http_agent

this code reproduces the TypeError for me on v8.11.2:
require('http').get('http://0.0.0.0', (res)=>{}).once('socket', (socket)=>{ socket.emit('agentRemove'); });

@lpinca
Copy link
Member

lpinca commented May 16, 2018

@jordanrogers thanks! Didn't think about that use case. I will open a PR to null the property without relying on the 'agentRemove' event and add a regression test.

@lpinca lpinca added the confirmed-bug Issues with confirmed bugs. label May 16, 2018
@lpinca
Copy link
Member

lpinca commented May 16, 2018

It's funny that this did go unnoticed for 3 months but I guess that is not a common use case.

lpinca added a commit to lpinca/node that referenced this issue May 16, 2018
Do not use the `'agentRemove'` event to null `socket._httpMessage` as
that event is public and can be used to not keep a request in the agent.

Fixes: nodejs#20690
MylesBorins pushed a commit that referenced this issue May 22, 2018
Do not use the `'agentRemove'` event to null `socket._httpMessage` as
that event is public and can be used to not keep a request in the agent.

PR-URL: #20786
Fixes: #20690
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
MylesBorins pushed a commit that referenced this issue May 23, 2018
Do not use the `'agentRemove'` event to null `socket._httpMessage` as
that event is public and can be used to not keep a request in the agent.

PR-URL: #20786
Fixes: #20690
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
@bedney
Copy link

bedney commented Jul 6, 2018

I apologize in advance if there is a procedure that I'm not following here, but I couldn't find any for formally requesting backporting of fixes.

Is there a plan to backport this fix into the 6.X and 8.X branches? This bug is biting us on servers running both of those versions.

Thanks in advance.

@lpinca
Copy link
Member

lpinca commented Jul 6, 2018

@bedney it should be backported, not sure why it didn't happen yet. I'll add the required labels to the PR to backport.

MylesBorins pushed a commit that referenced this issue Jul 13, 2018
Do not use the `'agentRemove'` event to null `socket._httpMessage` as
that event is public and can be used to not keep a request in the agent.

Backport-PR-URL: #20786
PR-URL: #20786
Fixes: #20690
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
rvagg pushed a commit that referenced this issue Aug 16, 2018
Do not use the `'agentRemove'` event to null `socket._httpMessage` as
that event is public and can be used to not keep a request in the agent.

Backport-PR-URL: #20786
PR-URL: #20786
Fixes: #20690
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
@bugg2844
Copy link

bugg2844 commented Oct 3, 2018

Hi, it appears that this issue still exists in 8.11.4, so we are still stuck at 8.11.1. Is there a plan to fix in 8.11, or is 8.12 the nearest fix version?

@MylesBorins
Copy link
Member

There will not be another 8.11 release.

Is this fixed in 8.12?

@bedney
Copy link

bedney commented Oct 3, 2018

I can confirm that this is fixed in 8.12

abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this issue May 5, 2024
Do not use the `'agentRemove'` event to null `socket._httpMessage` as
that event is public and can be used to not keep a request in the agent.

PR-URL: nodejs/node#20786
Fixes: nodejs/node#20690
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants