Broken with Node.js ≥ 13.10.0

[TimothyGu]

Describe the bug

• Node.js version: 13.10.0 or 13.10.1
• OS & version: Linux 5.5.6-1-MANJARO x86_64 GNU/Linux

got.stream does not work with Node.js ≥ 13.10.0. Node.js 13.9.0 works fine.

Actual behavior

When piped into a stream, the receiving stream receives no data.

Expected behavior

When piped into a stream, the receiving stream receives the entire HTTP response.

Code to reproduce

const got = require("got");
got.stream("https://www.github.com/").pipe(process.stdout);
// one can also use stream.pipeline here. Same result.

With Node.js < 13.9.0, output fills the screen. With ≥ 13.10.0, there is no console output.

Checklist

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

[darksabrefr]

Same behavior here but all got is affected not only streams :

const got = require("got");

// ensure no exit before the timer was cleared
const interval = setInterval(() => {}, 60000);

(async () => {
	console.log('step 1');
	const result = await got("https://www.github.com/");
	console.log(result);
	console.log('step 2');
	clearInterval(interval);
})();

This code never throws, exits or output results. It only display step 1 and do nothing more. Before node 13.10.0 it displays results.

[mrGibi]

2 hours of debugging and searching for my mistake... 😄
Same here after upgrading Node to v13.10.1.

[arcanis]

@szmarczak Are there plans to backport the fix into a minor / patch release? This bug is fairly serious. Yarn 2 relies on Got, which causes builds to abort mid-sequence.

[szmarczak]

It's gonna be a patch release. I'm working on this right now.

[szmarczak]

LOL @nodejs messed up!

const {PassThrough, pipeline} = require('stream');
const https = require('https');

const body = new PassThrough();
const request = https.request('https://example.com');

request.once('abort', () => {
	console.log('The request has been aborted');
});

pipeline(
	body,
	request,
	error => {
		console.log(`Pipeline errored: ${!!error}`);
	}
);

body.end();

gives

Pipeline errored: false
The request has been aborted

This is the culprit: https://github.com/nodejs/node/pull/31940/files#diff-eefad5e8051f1634964a3847b691ff84R36

It calls .abort() on every request you make using pipeline.

https://github.com/nxtedition/node/blob/5a55b1df97a1f93c5fb81bf758050d81524ae834/lib/internal/streams/destroy.js#L166

[szmarczak]

nodejs/node#32105

[TimothyGu]

@szmarczak Are you sure pipeline is the issue here? My original example used the plain old .pipe().

[szmarczak]

I am sure. I see you haven't looked at Got source code. Few people do.

[szmarczak]

Waiting for:

• [v13.x backport] stream: don't destroy final readable stream in pipeline nodejs/node#32111
• http: detach socket from IncomingMessage on keep-alive nodejs/node#32153 (will be featured in Node.js 14.x)
  [v13.x backport] pipeline destroy compat nodejs/node#32212
• http: align OutgoingMessage and ClientRequest destroy nodejs/node#32148 (optional, will be featured in Node.js 14.x)
• stream: make finished try to wait for 'close' nodejs/node#32158 (optional, will be featured in Node.js 14.x)

[darksabrefr]

Node's team have released a new version (13.11.0) today that may fix this issue. Depending of your download source for node and your os, the release may not be available right now. For example, the ubuntu version is not available at this instant from nodesource.

[szmarczak]

Tests are still failing on Node.js 13.11.0. I'll look at this later today.

[Pytal]

Updating to Node.js v13.11.0 has fixed the issue for me!

[szmarczak]

Glad to hear this! But the tests are still failing, so there's some fixing needed to be done. I haven't looked at this yet, need to get some sleep...

[darksabrefr]

Same for me, node v13.11.0 fixes this issue for my usages.

[SpineEyE]

It's the first time I use this library, I haven't used nodejs for a long time so it took me a long time to even get here, not to mention that I did not read got's source code... I'm using node v13.11.0 and the bug is still there for me. How can this happen on an LTS release? This is my code:

async function() {
 try {
  const response = await got.post(API_URI + '/Account/token');
  console.log("hello"); //never reached
 }
 catch (e) {
    // never reached
 }
}

Is there any workaround?

[TimothyGu]

Nose.js v13.x is not an LTS branch.

[darksabrefr]

It's the first time I use this library, I haven't used nodejs for a long time so it took me a long time to even get here, not to mention that I did not read got's source code... I'm using node v13.11.0 and the bug is still there for me. How can this happen on an LTS release? This is my code:

async function() {
 try {
  const response = await got.post(API_URI + '/Account/token');
  console.log("hello"); //never reached
 }
 catch (e) {
    // never reached
 }
}

Is there any workaround?

Can you provide a reproducible example ? Your use case is too imprecise. I use got and node (v13.11.0) intensively and I have no problem other than #1096 which is not node version related and is similar to yours.

[tagplus5]

All works with node v13.12.0

• Node.js version: 13.12.0
• OS & version: Ubuntu 19.10 5.3.0-46-generic

async function test() {
    try {
        const response = await got.get('https://google.com');
        console.log(response); // ok
    }
    catch (err) {
        console.error(err);
    }
}

async function testError() {
    try {
        const response = await got.get('https://google1.com');
        console.log(response);
    }
    catch (err) {
        console.error(err); // ok
    }
}

[szmarczak]

As of Node.js v13.11.0 and 48b817e all tests pass :D