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

Async iterator does not work on Node.js 14 #1172

Closed
2 tasks done
darksabrefr opened this issue Apr 22, 2020 · 6 comments
Closed
2 tasks done

Async iterator does not work on Node.js 14 #1172

darksabrefr opened this issue Apr 22, 2020 · 6 comments
Labels
bug Something does not work as it should external The issue related to an external project

Comments

@darksabrefr
Copy link

Describe the bug

  • Node.js version: 14.0.0
  • OS & version: Ubuntu 19
  • Got version: 11.0.2

With the brand new node 14 and when awaiting something before consuming a got stream with the async iterator, I don't get any chunks. I don't know if this bug is node related but in my tries, I was able to successfully request as this way with the native http module or in all cases under node 13, as this bug only occurs with node 14. It may be related to #1159.

Code to reproduce

Server part
'use strict';
const http = require('http');
const server = http.createServer((req, res) => {
	res.writeHead(200, { 'Content-Type': 'text/plain' });
	setTimeout(() => res.write('chunk 1'), 500);
	setTimeout(() => res.write('chunk 2'), 1000);
	setTimeout(() => res.write('chunk 3'), 1500);
	setTimeout(() => res.end(), 2000);
});
server.listen(8000);
Got client part
'use strict';
const got = require('got');
(async () => {
	try {
		const req = await got.stream('http://localhost:8000');
		req.on('end', () => {
			console.log('end event');
		});

		await new Promise((resolve) => {
			setTimeout(resolve, 100);
		})

		for await (const data of req) {
			console.log('data', data.toString())
		}
		console.log(`after for await`);
	} catch (e) {
		console.log(`error: error=${e}`);
	}
})();
Native node client part
'use strict';
const http = require('http');
const {once} = require('events');
(async () => {
	try {
		const req = http.request('http://localhost:8000');
		req.end();

		const [res] = await once(req, 'response');
		res.on('end', () => {
			console.log('end event');
		});

		await new Promise((resolve) => {
			setTimeout(resolve, 100);
		})

		for await (const data of res) {
			console.log('data', data.toString())
		}
		console.log(`after for await`);
	} catch (e) {
		console.log(`error: error=${e}`);
	}
})();

Expected behavior

data chunk 1
data chunk 2
data chunk 3
end event
after for await

Actual behavior

Only for got client + node 14:

after for await

For other combinations, (native client + node 13/14 or got client + node 13), the actual behavior is the expected behavior

Checklist

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

diervo commented Apr 27, 2020

I have traced down a maybe similar issue on this library when using node v14.

In this case is making webdriverio fail since a request from got never succeeds nor it raises an error.

The code flows through as-promise.js and ends up returning with _a.aborted = true, which leave the application in a nondeterministic state.

   if ((_a = response.req) === null || _a === void 0 ? void 0 : _a.aborted) {
                // Canceled while downloading - will throw a `CancelError` or `TimeoutError` error
                return;
            }

In case someone want to take a look, but this is the a contrived repro case:
https://github.com/diervo/wdio-repro-example

For the record: webdriverio/webdriverio#5319

@szmarczak
Copy link
Collaborator

Haven't looked at this yet, will do later today.

@szmarczak szmarczak added bug Something does not work as it should external The issue related to an external project wontfix The issue cannot be fixed on Got side and removed unconfirmed bug The issue is possibly a bug - needs more info labels Apr 28, 2020
@szmarczak szmarczak changed the title node 14 + streams + async iterator bug Async iterator does not work on Node.js 14 Apr 28, 2020
@darksabrefr
Copy link
Author

Very nice catch @szmarczak, thanks for digging deep in node internals !

@szmarczak szmarczak removed the wontfix The issue cannot be fixed on Got side label May 2, 2020
@szmarczak
Copy link
Collaborator

We need to wait for a 14.2.0 release.

@szmarczak
Copy link
Collaborator

Node.js 14.2.0 got released 20h ago, I can confirm that it's fixed now.

@darksabrefr
Copy link
Author

Node.js 14.2.0 got released 20h ago, I can confirm that it's fixed now.

I can confirm too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something does not work as it should external The issue related to an external project
Projects
None yet
Development

No branches or pull requests

3 participants