accessing quinn ClientBuilder options from the web_transport::Client

[jin-eld]

Hi,

I am developing an application which supports both, browser WASM32 and native and I ran into the following issue.

I'd like to provide an option to disable certificate verification, I understand, that this would not work on WASM, but it would be nice to be able to still use the web_transport::Client which sits on top of WASM and native, instead of having to split the code by platform and go the web-transport-quinn::Client route.

For instance, web-transrpot::Client already provides:

pub fn with_unreliable(self, val: bool) -> Self

For compatibility with WASM. Panics if val is false, but does nothing else.

..and having something similar for pub fn with_no_certificate_verification(self) would be very helpful.

Would you consider looking at this or is there perhaps an existing way in the API which I am not seeing?

[kixelated]

You can use web_transport_quinn or web_transport_wasm to establish the connection then .into() web_transport

[kixelated]

The idea is that we only expose generic options in the web_transport, and it's not possible to disable TLS verification in browsers.

[jin-eld]

Oh, you mean instead of trying to .into() the Client rather only split the connection code and convert an established Session? Indeed, I did not think of that, thanks for the pointer!

The idea is that we only expose generic options in the web_transport, and it's not possible to disable TLS verification in browsers.

Right, that I understand, but pub fn with_unreliable(self, val) -> Self kind of suggested there may be a shortcut for this option too 😅

[kixelated]

I don't think From is implemented for Client but that makes sense to me. with_unreliable exists because browsers do support that option; it toggles if a TCP fallback is allowed for WebTransport.

[jin-eld]

with_unreliable exists because browsers do support that option; it toggles if a TCP fallback is allowed for WebTransport.

I thought this exists for compatibility, to avoid forcing the user to split their code by platform? After all the docs say:

For compatibility with WASM. Panics if val is false, but does nothing else.

So it would panic if anyone actually tried to enable it on the wrong platform, my thought about the certificate verification option was going among the same lines. It would still force the users to add at least a minimum of platform dependencies for their options, but at least everything else could avoid a split.

That being said I ran in yet another scenario like this, where I had to check web_transport::Error::Session-> web_transport::quinn::SessionError::ConnectionError -> quinn_proto::ConnectionError::LocallyClosed and this of course does not work on WASM, because the web_transport::Error::Session looks completely different there due to the underlying web-transport-wasm crate. So not sure where the red line should be drawn in terms of a generic API which would provide some reasonable minimum over both platforms...

[kixelated]

heh well it definitely shouldn't panic, that's really dumb on my part.

[kixelated]

And yeah, Error is absolutely wrong for the web_transport crate. There's no generic interface for errors, or at least I don't want to try to build one. It should box the error as a string or something instead of using an associated trait.

[jin-eld]

heh well it definitely shouldn't panic, that's really dumb on my part.

I thought panic was a bit too harsh, I'd rather have it do nothing, but it's good that it is there for compatibility.

And yeah, Error is absolutely wrong for the web_transport crate. There's no generic interface for errors, or at least I don't want to try to build one. It should box the error as a string or something instead of using an associated trait.

I'd say String would be enough in 90% of the cases, but then there is at least one situation where it would be valuable to recognize what happened: when a connection gets closed (which isn't really an error as such).

Currently:

pub async fn recv_datagram(&mut self) -> Result<Bytes, Error>

Receive a datagram over the network.

When a connection gets closed either as LocallyClosed or ApplicationClosed we'll get this as an Error, indeed right now checking for it is somewhat cumbersome and requires platform code, however if this was a String, recognizing this case would not be possible at all.

An alternative would be to borrow the API idea from Stream and return Result<Option<Bytes>>, Error> (or in the future String), where Some(Bytes) means, that a packet was received and None means - end of stream.

As for #136 - I would keep it for compatibility, just don't panic. The "selling point" of this crate to me was the ability to develop a cross platform client which uses the same code for WASM and native. Of course there are minor deviations like the above, which can not work for both, but this is manageable, if the function does nothing on one of the platforms - I think it's fine, because it's still better to have one common object across both platforms even if some of the functionality will be missing, depending on the platform.