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

"Remote end closed socket abruptly" Node.js 12.16.3 #312

Closed
ivanstanev opened this issue Apr 28, 2020 · 17 comments
Closed

"Remote end closed socket abruptly" Node.js 12.16.3 #312

ivanstanev opened this issue Apr 28, 2020 · 17 comments

Comments

@ivanstanev
Copy link

Hey! The new patch release of Node.js (12.16.3) broke needle (we're using 2.4.0) and using needle with redirects now causes the following error:

Error: Remote end closed socket abruptly.
    at TLSSocket.on_socket_end (/Users/ivan/local/node_modules/needle/lib/needle.js:473:17)
    at processTicksAndRejections (internal/process/task_queues.js:79:11)

You can reproduce it with the following piece of code:

await needle('get',
  `https://github.com/kubernetes-sigs/kind/releases/download/v0.7.0/kind-linux-amd64`,
  null,
  { follow_max: 2 },
);

A similar issue was raised a while ago here: #261

@tomas
Copy link
Owner

tomas commented Apr 28, 2020

Darn. Do you know what did the Node.js release change regarding TLS sockets?

@gitphill
Copy link

gitphill commented May 1, 2020

@tomas
Copy link
Owner

tomas commented May 1, 2020

Will take a look at this later today.

@ronag
Copy link

ronag commented May 5, 2020

nodejs/node#33254

@ronag
Copy link

ronag commented May 5, 2020

This condition looks wrong.

It worked before due to bug in Node that was fixed in nodejs/node@ed21d32#diff-e6ef024c3775d787c38487a6309e491dL634, which then broke needle.

Basically, just because the socket has 'end':d and is no longer writable does not mean it has to be destroyed. It get's destroyed after it has been shutdown.

If you want to detect a premature close please take look at hows eos or the native stream.finished module does it.

@tomas
Copy link
Owner

tomas commented May 5, 2020

Good call @ronag. So we should remove that check altogether?

@ronag

This comment has been minimized.

@ronag
Copy link

ronag commented May 5, 2020

Good call @ronag. So we should remove that check altogether?

Yes. Use response.on('aborted', handler) instead.

floryst added a commit to Kitware/vtk-js that referenced this issue May 11, 2020
node-pre-gyp works better with request rather than needle on node
12.13.6. Until tomas/needle#312 gets fixed, we
will use request.
floryst added a commit to Kitware/vtk-js that referenced this issue May 11, 2020
node-pre-gyp works better with request rather than needle on node
12.13.6. Until tomas/needle#312 gets fixed, we
will use request.
@mhio
Copy link

mhio commented May 12, 2020

Wow.. this has been driving me crazy for a week.

For some reason the issue seems to be exacerbated by builds that were running in a CI container. I couldn't reproduce the issue in a local docker build FROM node:12.16.3 container, but running the yarn install in a kaniko or buildah container node-pre-gyp downloads using needle would fail with the Remote end closed socket abruptly error every time.

@zbjornson
Copy link

Sorry to pester, but any chance this can be resolved soon? We're getting a lot of issues opened in node-canvas because our prebuilds are failing to download. Thank you!

@tomas
Copy link
Owner

tomas commented May 19, 2020

@zbjornson Looking at it right now. Sorry for the delay.

@tomas
Copy link
Owner

tomas commented May 19, 2020

Ok looks like I found a fix. Once I get the last three specs to pass on Node v12.16.3 I'll push a new version.

@marcosflick
Copy link

Any updates on this?

@tomas
Copy link
Owner

tomas commented May 20, 2020

Yes! I'll push a new version in a while.

@tomas
Copy link
Owner

tomas commented May 21, 2020

Ok we're good to go.

image

@tomas
Copy link
Owner

tomas commented May 21, 2020

Just pushed 2.5.0 which fixes this issue.

Sorry everyone for the long delay -- I run an ecommerce platform and these past weeks have been a bit crazy.

PS: @ronag I tried the aborted callback or the stream.finished route but they didn't work, so I ended up adding an simple check that does the trick, while still handling the scenario tested in test/long_string_spec.js. Feel free to submit a PR if you find a better strategy though. :)

@tomas tomas closed this as completed May 21, 2020
@darscan
Copy link

darscan commented May 21, 2020

@tomas Thanks for the fix. Working on open source can be a pretty thankless business sometimes, but your work here is much appreciated 😃

FauxFaux added a commit to snyk/unless-overloaded that referenced this issue Jun 1, 2020
needle has a bug with sockets on newer node 12,
we're currently avoiding this newer node 12 to
evade the problem. This bug is fixed in needle
2.5.0, so let's upgrade to that.

tomas/needle#312
FauxFaux added a commit to snyk/kubernetes-monitor that referenced this issue Jun 1, 2020
needle has a bug with sockets on newer node 12,
we're currently avoiding this newer node 12 to
evade the problem. This bug is fixed in needle
2.5.0, so let's upgrade to that.

tomas/needle#312
FauxFaux added a commit to snyk/nodejs-runtime-agent that referenced this issue Jun 1, 2020
needle has a bug with sockets on newer node 12,
we're currently avoiding this newer node 12 to
evade the problem. This bug is fixed in needle
2.5.0, so let's upgrade to that.

tomas/needle#312
FauxFaux added a commit to snyk/snyk-mvn-plugin that referenced this issue Jun 1, 2020
needle has a bug with sockets on newer node 12,
we're currently avoiding this newer node 12 to
evade the problem. This bug is fixed in needle
2.5.0, so let's upgrade to that.

tomas/needle#312
DylanLacey pushed a commit to DylanLacey/pdf.js that referenced this issue Jun 25, 2020
Versions of `needle` prior to `2.5.0` cannot cope with redirects (as documented: tomas/needle#312).

This prevents prebuilt `canvas` binaries from being downloaded on MacOS,
requiring the global install of its dependencies.

Updating Needle restores it to functionality, addressing this.  It also avoids
the need to add `request` to `package.json`; it also obsoletes mozilla#12018.
chen-keinan pushed a commit to snyk/kubernetes-monitor that referenced this issue Apr 26, 2021
needle has a bug with sockets on newer node 12,
we're currently avoiding this newer node 12 to
evade the problem. This bug is fixed in needle
2.5.0, so let's upgrade to that.

tomas/needle#312
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants