fix(server): export UpgradeableConnection

[lucacasonato]

This struct is part of the public API (returned from
Connection::with_upgrades), but was previously not exported.

Why this is a problem doesn't require explanation I think 😉

[seanmonstar]

I'd actually be really interested in the motivation. In my mind this was always just an implementation detail that I hoped no one would have to care about. It was mostly just meant to be an opaque type, kinda like an impl Future.

[lucacasonato]

@seanmonstar We store the Connection in a wrapper struct that implements some traits we need resources in Deno to have. We now want to use a UpgradeableConnection instead of a regular Connection to support server side websockets. In the current model we can not have a wrapper struct anymore, because we can not specify UpgradeableConnection as being the inner type of that struct, because we can not reference the UpgradeableConnection type, because it is not exposed.

[lucacasonato]

Also, do you know what the commit message bot is complaining about? I can't see the actual error to correct the commit message.

[seanmonstar]

The commit format is in the CONTRIBUTING file, it's looking for commits like feat(module): short description. But don't worry about it if it's hard, I don't have it marked as a "required" check, it's just there to remind me to fix when merging.

[lucacasonato]

Some more background on the motivation. Here is the actual usecase: https://github.com/denoland/deno/blob/46d65919737d644cc57b569b9636673cd1d94d2b/extensions/net/ops_http.rs#L104-L107. We store UpgradeableConnection and then later manually poll it with poll_fn. We do not "await" the connection (in which case I understand why it would make sense to treat this like an impl Future). I believe @bartlomieju has previously talked to you about why we have to poll the connection in this way in Deno.

[seanmonstar]

Ah I see, so you need to store the future inside a struct. And being able to store type Conn = impl Future; in a struct isn't stable yet (there's progress, though!).

Do you otherwise need to call any inherent methods on the UpgradeableConnection? Or do you only call Future::poll on it? If only poll, you could store it as a Pin<Box<dyn Future<Output = hyper::Result<()>> + Send>>. Then you wouldn't need to "name" the type. Does that seem acceptable?

[lucacasonato]

Do you otherwise need to call any inherent methods on the UpgradeableConnection? Or do you only call Future::poll on it? If only poll, you could store it as a Pin<Box<dyn Future<Output = hyper::Result<()>> + Send>>. Then you wouldn't need to "name" the type. Does that seem acceptable?

I think only Future::poll, but not sure now. I'll try the Pin<Box<dyn Future>> and get back to you.

[lucacasonato]

That works as expected. I still feel like the type should be exported if it is used as a return type.

Thanks