feat(unstable): remove Deno.upgradeHttp API

[bartlomieju]

Closes #21828.

This API is a huge footgun. And given that "Deno.serveHttp" is a deprecated
API that is discouraged to use (use "Deno.serve()" instead); it makes no
sense to keep this API around.

This is a step towards fully migrating to Hyper 1.

[iuioiua]

I'm not against this change, but it'll break std/http's server implementation, which is due for removal once the Standard Library reaches v1. A couple of solutions:
1. Bring the removal of std/http's server forward. I think this is fine since Deno.serve() is solid, and people can pin old versions of std.
2. Delay the merging of this PR.

https://github.com/denoland/deno_std/blob/8c55d22b2d16bde3a8b6260b76931c34bc92c107/http/server.ts#L426

Ignore. I misread Deno.serveHttp() instead of Deno.upgradeHttp() 😅

[imcotton]

So is there any workaround for #21384 (from #11744)?

[bartlomieju]

I'm not against this change, but it'll break std/http's server implementation, which is due for removal once the Standard Library reaches v1. A couple of solutions:

1. Bring the removal of std/http's server forward. I think this is fine since Deno.serve() is solid, and people can pin old versions of std.
2. Delay the merging of this PR.

https://github.com/denoland/deno_std/blob/8c55d22b2d16bde3a8b6260b76931c34bc92c107/http/server.ts#L426

@iuioiua how would that break std/http? I don't see Deno.upgradeHttp being used anywhere in std.

So is there any workaround for #21384 (from #11744)?

@imcotton yes, Deno.upgradeHttpRaw is the preferred API for this use case. You should be able to do CONNECT proxy using this API.

[imcotton]

Oh, I thought Deno.upgradeHttpRaw was dropped, because there no document for it since Deno v1.33.0, unlike Deno.upgradeHttp at least it can be found under unstable section.

So Deno.upgradeHttpRaw is stable now?

[bartlomieju]

Oh, I thought Deno.upgradeHttpRaw was dropped, because there no document for it since Deno v1.33.0, unlike Deno.upgradeHttp at least it can be found under unstable section.

So Deno.upgradeHttpRaw is stable now?

Wait, looks like I'm mistaken. @mmastrac do you remember why we made Deno.upgradeHttpRaw internal?

[mmastrac]

Wait, looks like I'm mistaken. @mmastrac do you remember why we made Deno.upgradeHttpRaw internal?

I don't remember exactly why, but I believe it's because the API was not compatible with HTTP/2 and required us to write an "emulation" layer for HTTP responses.

EDIT: I'm looking through the APIs again and I've forgotten why we ended up in this state. I'd have to refresh my memory a bit, but there were a few concerns -- one of them was that the upgradeHttp method could deadlock.

[mmastrac]

OK, after some further analysis:

• upgradeHttpRaw is the API we should be using in the future because upgradeHttp is susceptible to deadlocks.
• upgradeHttpRaw's docs were wrong in the past. This API actually returns { response: Response, conn: Conn }.
• Both upgradeHttp and upgradeHttpRaw do HTTP/1 emulation to parse a "fake" response
• I believe that upgradeHttp{raw,} were both developed for node.js emulation

Note that upgradeHttpRaw has not been tested or validated for use w/CONNECT, but this should be reasonable easy to confirm.

Deno.upgradeWebSocket is the API that should be used for websockets.

Note that I would not be against developing a new API to handle CONNECT specifically, as there are complications that make this not-so-straightforward w/HTTP/2.

[imcotton]

Thanks @mmastrac for the scope breakdown and initialing the RFC ticket #21870 regarding the userland CONNECT feature.

I think it's OK to remove Deno.upgradeHttp since it has been unstable from the beginning and focus on RFC discussions for further development.

[mmastrac]

LGTM