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

multiple sync http2 requests lead to multiple connections instead of reusing #39

Closed
cmawhorter opened this issue Mar 27, 2019 · 4 comments
Labels

Comments

@cmawhorter
Copy link

i don't know enough about the fetch spec to say if this is a bug or not but it's definitely not what i expected.

this code below causes 10 separate connections. i'd expect what to happen in this situation is the first request blocks the others until the connection is established and they all share it.

additionally, this causes something to be off with a stuck reference or something. "complete" is printed but the process never exits.

const { fetch, disconnectAll } = require('fetch-h2');

const urls = [
'https://example.com/api/1',
'https://example.com/api/2',
'https://example.com/api/3',
'https://example.com/api/4',
'https://example.com/api/5',
'https://example.com/api/6',
'https://example.com/api/7',
'https://example.com/api/9',
'https://example.com/api/10',
];

const main = async () => {
	const results = await Promise.all(urls.map(async url => {
		const res = await fetch(url);
		const body = await res.text();
		return body;
	}));
	await disconnectAll();
	return 'complete';
};

main().then(console.log);

blocking with an initial call fixes issue:

const main = async () => {
  await fetch('https://example.com/');
  const results = await Promise.all(...

That results in just one connection being used and the process exiting as expected. It seems related to getOrCreateHttp2 but not sure. 😪 🛏

@grantila
Copy link
Owner

That's likely a bug, if the ALPN negotiation results in http2.

The flow is supposed to be that when an origin (in your case example.com is unknown to fetch-h2, ALPN negotiation ends up in either http1 or http2 and consecutive immediate requests to the same origin is blocked until the negotiation is complete.
If negotiation results in http2, a single connection should be re-used. If http1, connection pooling will happen.

I'll look into this.

@cmawhorter
Copy link
Author

Ah, makes sense. Everything works great otherwise. Loving it.

Btw - I think the stuck references not being cleared by disconnectAll is a different issue. I've been running into it separate from what I described above.

And totally unrelated, but what's stopping this from node 8 support? Have you looked into porting the node10 http2.js back or something? AWS lambda doesn't have v10 yet. I can get fetch-h2 mostly running under node 8 via rollup except for http2.

@grantila
Copy link
Owner

Yeah http1 probably works in Node 8, but since the library started with (only) http2, I don't want to claim it in the readme.
http2 in Node 8 is really broken + the API has changed since then.

Anyway, it's pretty sad we don't have Node 10-support in AWS lambdas, shame on them ;) On the other hand, you can upload binaries in lambdas, e.g. Node 10 😆

@github-actions
Copy link

🎉 This issue has been resolved in version 2.4.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

No branches or pull requests

2 participants