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

http2 demo not working as expected #2257

Closed
mcollina opened this issue Sep 8, 2023 · 7 comments · Fixed by #2258 or #2275
Closed

http2 demo not working as expected #2257

mcollina opened this issue Sep 8, 2023 · 7 comments · Fixed by #2258 or #2275
Labels
bug Something isn't working

Comments

@mcollina
Copy link
Member

mcollina commented Sep 8, 2023

Consider the following:

import { request, fetch, Agent, Client, setGlobalDispatcher } from './index.js'

setGlobalDispatcher(new Agent({
  allowH2: true
}))

// Not working as expected
// the request is done using HTTP/1.1
const res1 = await fetch(`https://http2.pro/api/v1`)
console.log(await res1.json())

// Not working as expected, the response is never received
const client = new Client('https://http2.pro', {
  allowH2: true
})

const res2 = await request(`https://http2.pro/api/v1`, { dispatcher: client })
console.log(await res2.body.json())
@mcollina mcollina added the bug Something isn't working label Sep 8, 2023
@mcollina
Copy link
Member Author

mcollina commented Sep 8, 2023

@metcoder95

@KhafraDev
Copy link
Member

maybe a "forceH2" option is needed?

@mcollina
Copy link
Member Author

mcollina commented Sep 8, 2023

technically it should use http2 if alpn says it's available

@metcoder95
Copy link
Member

I need to dig deeper into it, but the bug is quite peculiar.

The ALPN negotiation was successful, and the socket has been connected through h2. A request has been successfully triggered to the server, but the server basically has not replied at all, meaning that no body is being received, not even the response headers static the stream has been made.

@metcoder95
Copy link
Member

I also find smaller improvements that I'll push soon. I'll try to dig deeper soon into this to see if I can find the root cause

@metcoder95
Copy link
Member

metcoder95 commented Sep 8, 2023

nvm, find the issue 😓
Opening PR now

@metcoder95 metcoder95 mentioned this issue Sep 8, 2023
7 tasks
@ShenHongFei
Copy link

setGlobalDispatcher(new Agent({
  allowH2: true
}))

// Not working as expected
// the request is done using HTTP/1.1
const res1 = await fetch(`https://http2.pro/api/v1`)
console.log(await res1.json())

This part does not work properly because Agent uses lib/pool.js, and allowH2 is not passed here. The relevant logic is in

undici/lib/pool.js

Lines 53 to 62 in 9fa8224

if (typeof connect !== 'function') {
connect = buildConnector({
...tls,
maxCachedSessions,
socketPath,
timeout: connectTimeout == null ? 10e3 : connectTimeout,
...(util.nodeHasAutoSelectFamily && autoSelectFamily ? { autoSelectFamily, autoSelectFamilyAttemptTimeout } : undefined),
...connect
})
}

And

undici/lib/agent.js

Lines 21 to 25 in 9fa8224

function defaultFactory (origin, opts) {
return opts && opts.connections === 1
? new Client(origin, opts)
: new Pool(origin, opts)
}

Client will be used only if connections is set to 1, otherwise Pool will be used, so you need to set connections: 1 (I don’t know if it is reasonable)

setGlobalDispatcher(new Agent({
  allowH2: true,
  connections: 1
}))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants