The http2 server throws TypeError `server.shouldUpgradeCallback` is not a function

[ShenHongFei]

Version

origin/main branch

Platform

windows "$([Environment]::OSVersion.VersionString) $(('x86', 'x64')[[Environment]::Is64BitOperatingSystem])"

Subsystem

http

What steps will reproduce the bug?

import { createSecureServer } from 'http2'

let server = createSecureServer(...)

server.on('upgrade', () => { ... })

server.listen(443)

// Create a websocket connection to port 443

How often does it reproduce? Is there a required condition?

Must appear

What is the expected behavior? Why is that the expected behavior?

Websocket connection is normal

What do you see instead?

Additional information

relates to: #59824

Can you (@pimterry) add relevant test cases for this PR? http server, http2 server and test websocket connection

[pimterry]

Thanks @ShenHongFei I'll look into this now. Can you share a complete example including the full HTTP/2 server options and the client connection code you're using? I'm a bit surprised that it's possible to hit this code path - in theory (https://developer.mozilla.org/en-US/docs/Web/HTTP/Guides/Protocol_upgrade_mechanism) upgrade shouldn't be possible in HTTP/2 at all.

[ShenHongFei]

{
    ca: ...
    allowHTTP1: true
}

[pimterry]

Should be fixed in #59924 @ShenHongFei

[ShenHongFei]

@pimterry I just merged this PR locally and tried it. It works fine without any errors. Thank you very much.

Do I need to close this issue? Or should I wait until your PR is merged?

[pimterry]

Great, thanks for testing that @ShenHongFei! Let's leave this open until a fix is fully completed, that PR will close this issue automatically once it's merged.

[cbn-falias]

Hello,
this also happened to me when upgrading from 22.17.0 to 22.21.0.
Didn't check it in detail if it is the exact same error case but I noticed that the fix is tagged as dont-land-on-v22.x.
Shouldn't this be also fixed in the LTS release?
Thanks in advance!

[pimterry]

Oooh, that's unfortunate, this may cause some breakage. Thanks for reporting this @cbn-falias.

@aduh95 I think you did the 22.21.0 release, and it looks like you also tagged #59924 as not for v22. Unfortunately that change needed to go out with #59824, or it breaks at least one fairly common use case (websockets + HTTP/2).

I'm not sure how the release & backporting workflows work, or why that was tagged not to be released. We will need to somehow do a patch release to include #59824 in v22 now though (or revert #59924, but I expect that's problematic). CC @nodejs/backporters.

[aduh95]

I'm not sure how the release & backporting workflows work

It is documented in https://github.com/nodejs/node/blob/main/doc/contributing/backporting-to-release-lines.md, please open a manual backport PR so the change can be included in the next one

[pimterry]

Ok, thanks, I'll backport it now.

[cirix]

Hi all,
I have just upgraded in my osx using brew and I am facing the same issue. Will a patch be issued?

best regards

[pimterry]

@cirix there's a fix open for this here: #60341. That needs to be reviewed & merged first, and then released in what will probably be v22.21.1.

In the meantime, you can work around this by downgrading to v22.20 or older, or by upgrading to v24 - it's only v22.21.0 specifically that's broken.

[pimterry]

There's a separate open issue for the v22 backport-specific issue at #60336, so lets track this there instead of reopening this original issue from the initial main branch changes.