"Remote end closed socket abruptly" redirects on Node 8.12.0

[darscan]

Something is fishy with Needle and redirects on Node 8.12.0!

Remote end closed socket abruptly

The issue does not seem to appear with Node 4.x, 6.x, 9.x or 10.x.

I've put together the smallest example I could that demonstrates the problem (please excuse the gross style.. I wanted to check Node 4 behaviour):

var needle = require('needle');
var express = require('express');
var app = express();
var port = 3000;

app.get('/', (req, res) => res.send('Hello World!'));
app.get('/redirector', (req, res) => res.redirect('/'));

var server = app.listen(port, () => {
  console.log(`NodeJS version ${process.version}`);
  console.log(`Example app listening on port ${port}!`);
  needle('get', 'http://localhost:3000/redirector', {}, {
    'follow_max': 5,
  })
  .then(res => {
    console.log('Worked with redirect:', res.body === 'Hello World!');
  })
  .catch(e => {
    console.log(`Redirect failed on NodeJS version ${process.version}`);
    console.error(e);
  })
  .then(() => server.close());
});

I mostly tested with Express 4.16.3 and Needle 2.2.3, but tried a few combinations with the same results as below:

NodeJS version v4.9.1
Example app listening on port 3000!
Worked with redirect: true

NodeJS version v6.14.3
Example app listening on port 3000!
Worked with redirect: true

NodeJS version v8.11.4
Example app listening on port 3000!
Worked with redirect: true

NodeJS version v8.12.0
Example app listening on port 3000!
Redirect failed on NodeJS version v8.12.0
Error: Remote end closed socket abruptly. <----- BOOOOO!

NodeJS version v9.11.2
Example app listening on port 3000!
Worked with redirect: true

NodeJS version v10.10.0
Example app listening on port 3000!
Worked with redirect: true

So, what is going on with Node 8.12.0? 8.11.4 works just fine.

Perhaps something to do with no longer calling maybeDestroy(this)?: https://github.com/nodejs/node/pull/18708/files

But I'd expect that to cause us problems with Node 10.x too.

Please let me know if I can provide any further details, and thanks for the great library 😺

P.S. The example code has both Express and Needle running on the same version of Node for simplicity. I have tested with running the Express server on Node 8.11.4 and hitting it from Needle running on 8.12.0 to rule out an issue with the server side of things.

[darscan]

Actually.. here's a trimmed down version (no express involved):

var needle = require('needle');

needle('get', 'https://docs.openstack.org/developer/horizon/', {}, {
  'follow_max': 5,
})
.then(() => {
  console.log(`Redirect worked on NodeJS version ${process.version}`);
})
.catch(e => {
  console.log(`Redirect failed on NodeJS version ${process.version}`);
  console.error(e);
});

And the results:

Redirect worked on NodeJS version v4.9.1

Redirect worked on NodeJS version v6.14.3

Redirect worked on NodeJS version v8.11.4

Redirect failed on NodeJS version v8.12.0
Error: Remote end closed socket abruptly.

Redirect worked on NodeJS version v9.11.2

Redirect worked on NodeJS version v10.10.0

[cmain]

Hitting the same exact issue. This was causing me major headaches because needle is used in node-pre-gyp and in my case the pre-built binaries were hosted on Github which involves a redirect. This error can cause node-pre-gyp to fail for the redirect and if the download after the redirect also fails two source builds will be kicked off which will stomp on eachother. This essentially prevents the newest versions of libxmljs from being installed with node v8.12.0.

[tomas]

Yikes, will check asap!

[tomas]

Ok I think I found a fix (fix-socket-destroy-on-redirect branch). It looks like the socket.destroy logic was modified/fixed in Node v8.12.0. This affects a check that Needle does to see whether the underlying socket was correctly destroyed.

Now, if I run the test script I get the correct output:

Redirect worked on NodeJS version v8.12.0

Without breaking previous versions:

Redirect worked on NodeJS version v8.11.3
Redirect worked on NodeJS version v7.5.0

If everyone's OK with the patch I can push a new version to npm.

[darscan]

Thanks @tomas ! I took a quick look at the branch.. such a small fix 😸 I don't have enough context to validate the impact of the patch, but it looks good (and safe) to me.

[tomas]

Ok, just pushed 2.2.4 to npm. Thanks everyone for reporting!

[darscan]

👋 Aaaand.. it's back:

Redirect worked on NodeJS version v12.16.2
Redirect failed on NodeJS version v12.16.3
Error: Remote end closed socket abruptly.
    at TLSSocket.on_socket_end (/redacted/node_modules/needle/lib/needle.js:480:17)

[mxcoder]

Just got bit by this too, my current solution is to lock my builds to the matching official image:
https://hub.docker.com/_/node/?tab=tags&page=1&name=12.16.2