-
Notifications
You must be signed in to change notification settings - Fork 331
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
π Bug Report β Runtime APIs: names of WebSocket ready state constants differ from the Living Standard #1370
Comments
Well, the reason was that we were (read: I was) working from https://datatracker.ietf.org/doc/html/rfc6455 which has the states but not a proscription on how they're exposed. Personally, I'm inclined to follow the web idl definition now that I know it exists. What do you think, @jasnell? Maybe we either just add the web idl def form or compat flag in the change to the web idl def form? |
Likely not worth a compat flag as long as there are no conflicts in the names. Let's just add the additional ones from the spec alongside the current set. If we want to deprecate the old names later we can do so with a compat flag then. |
@bcaimano ... as a follow up here, but only partially related to this issue... at some point in the not too far off future we really need to go through an audit just how far our implementation of |
This looks to be the cause of this issue with the Cognitive Speech Services SDK when running via Cloudflare Pages Functions. The Speech Services SDK checks the WebSocket readyState against WebSocket.OPEN, which works fine in a browser but fails in a Cloudflare function. My current workaround is to define these manually:
Just reporting for posterity in case someone comes across the same issue. From the discussion above, I guess workerd isn't yet compliant with the standard for the WebSocket interface, but as soon as the open PR lands, that should be fixed. |
Okay, thanks for the poke that reminded me to circle back on this! I've merged the relevant PR into workerd. Given the holiday season, it may or may not show up on cloudflare workers until the new year. |
Thank you @bcaimano ! Given that the fix has been landed (even if not released) let's go ahead and close this issue. |
Names of ready states constants on the
WebSocket
class, defined in here differ from that of the Web IDL definition in the WebSocket Living Standard. In workerd, every name have extra prefix:READY_STATE_
.It may cause problems when we want to use libraries that handle WebSocket connections and depend on the "standard" names for checking ready state of sockets in Cloudflare Workers. In workerd environment, ready state checks based on the "standard" constant names (e.g.
WebSocket.OPEN
) don't work correctly since they have no value (undefined
).I'm curious about why these constants are given different names than the "standard". If we have no reason for that, shouldn't we fix it?
The text was updated successfully, but these errors were encountered: