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

xwt_web_sys fails to create datagram reader on Firefox #156

Open
aecsocket opened this issue Jul 28, 2024 · 10 comments
Open

xwt_web_sys fails to create datagram reader on Firefox #156

aecsocket opened this issue Jul 28, 2024 · 10 comments

Comments

@aecsocket
Copy link
Contributor

When creating an xwt_web_sys::Connection on Firefox, it fails with a TypeError: ReadableStream.getReader: Trying to read with incompatible controller, which likely comes from web_sys_stream_utils creating a BYOB stream reader for reading the datagram stream.

https://developer.mozilla.org/en-US/docs/Web/API/ReadableStream/getReader#exceptions

This is also thrown if a BYOB reader is requested and the stream controller is not a ReadableByteStreamController (the stream was not constructed as an underlying source with type="bytes").

@MOZGIII
Copy link
Owner

MOZGIII commented Jul 28, 2024

From the https://developer.mozilla.org/en-US/docs/Web/API/WebTransport it looks like FF has support for BYOB streams... I wonder what's going on there, and if I need to change anything in the code.

What Firefox version fo you use? Can you create a simple pure-JS reproduction for this issue?

@aecsocket
Copy link
Contributor Author

I'm using LibreWolf version 128.0 and have tracking protection/anti-fingerprinting disabled.

After some trial and error I managed to make a index.html which produces this error: https://gist.github.com/aecsocket/f4f64387860b48395728432c62c59385

Instructions:

  • start a webtransport server on [::1]:25565, it can be serving anything, it just has to be a WT server
  • open that index.html, change the let hash hardcoded value to the server's certificate hash
  • serve the index.html, open that page in a web browser, click connect, and check the console. You should see a reader error

Screenshot from 2024-07-29 11-20-20

Whereas in Chromium it works fine

image

In testing, I think that Firefox support might just be hopeless at the moment. The browser crashed multiple times when trying to open a connection, the connection often stayed alive even after closing the browser tab, and I also couldn't connect a lot of the time due to "connection rejected" even though the certificate hash was 100% correct. I think that it might be too much effort to support it right now given how buggy it seems to be, at least from my experience.

@MOZGIII
Copy link
Owner

MOZGIII commented Jul 29, 2024

Thanks for researching this!

I agree with your summary, Firefox had issues in the past when it comes to WebTransport implementation, and they claimed they've resolved them but it seems the implementation is somehow still buggy. There is a chance, of course, the issue is with LibreWolf, but I doubt it.

We might want to summon some FF devs here (if they're willing) to get to the bottom of this. Not much I can do with this alone.

@MOZGIII
Copy link
Owner

MOZGIII commented Jul 29, 2024

I was able to reproduce it in tests with NO_HEADLESS=1 cargo test --target wasm32-unknown-unknown -p xwt-web-sys and manually navigating Firefox to the printed URL

image

@MOZGIII
Copy link
Owner

MOZGIII commented Jul 29, 2024

There is a relevant PR at wtransport btw: BiagioFesta/wtransport#192. It might just be a server-side issue.
I'll update xwt to the latest wtransport.

@aecsocket
Copy link
Contributor Author

I wasn't aware that there was a wtransport issue for this, but I think that might be unrelated since it's server-side. Of course I could be wrong, this is web browsers we're talking about, so there may be some weird draft behaviour. Please do update xwt and I can test if this issue is reproducible on latest wtransport.

One possible solution would be to not use BYOB readers on Firefox, or not use BYOB readers at all. But from what I understand about BYOB, this will lead to reduced performance since we would have to copy bytes, so this probably isn't a good solution.

For now I don't have the capacity to debug weird Firefox issues, so I won't be supporting it and will just say to use Chromium instead. This looks like a Firefox issue anyway, and not directly xwt-related.

@MOZGIII
Copy link
Owner

MOZGIII commented Jul 30, 2024

What backend do you use? Just to gather more info

@MOZGIII
Copy link
Owner

MOZGIII commented Jul 30, 2024

I checked with https://github.com/BiagioFesta/wtransport/commits/2a5b37ee78cd967734ee0473eed3809888c1fdfb - same crash at Firefox.

Firefox has BYOB marked as supported, but it clearly is not.

We'll leave this open for a while, until either FF fixes it or we close it as "not planned", or we deem this is worth implementing a workaround.

@aecsocket
Copy link
Contributor Author

Can you elaborate on what you mean by backend? If you mean xwt, xwt_web_sys for the client and xwt_wtransport for the server, although I don't fully understand what you mean.

It looks like BYOB wasn't part of the spec originally, and this had to be resolved later: w3c/webtransport#480
Maybe Firefox implemented datagram reading before this part of the spec was written, and now has to catch up to Chromium? Wouldn't be the first time that Chromium has pushed ahead with stuff out of the spec.

As a solution, how bad do you think it would be if xwt_web_sys used a regular stream reader instead of a BYOB one? I'm not sure what exactly the pros/cons to using BYOB is.

@MOZGIII
Copy link
Owner

MOZGIII commented Sep 17, 2024

As of 131.0a1 (2024-08-19) the tests are still failing.


Without BYOB, the document memory may growing uncontrollably since you are potentially allocating a new buffer for every read. It is unfit for games with their long-running fast-paced streams of data, although it might work for fetch-like request/response type scenarios.


I think you've summarized the situation correctly.

We have used non-BYOB streams at first and the memory usage issues were breaking things bad; switch to BYOB solved the issue, so implementing support for non-BYOB mode into xwt might not be reasonable.

I suggest you experiment with a hacked-in non-BYOB fork implementation of the xwt to see if that would even work in your case, as it might be a pointless effort to add non-BYOB mode properly in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants