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

Experimental Websocket readyState returned as a string #79

Closed
dougw-bc opened this issue Sep 26, 2024 · 4 comments · Fixed by #80
Closed

Experimental Websocket readyState returned as a string #79

dougw-bc opened this issue Sep 26, 2024 · 4 comments · Fixed by #80
Assignees
Labels
bug Something isn't working

Comments

@dougw-bc
Copy link

dougw-bc commented Sep 26, 2024

Brief summary

The experimental websocket encodes its internal readyState as a string.

https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/readyState

In a browser this value is represented as an integer.

k6 version

v0.53.0

OS

Ubuntu, Windows 11

Docker version and image (if applicable)

N/A

Steps to reproduce the problem

Try to compare readyState to an integer, and it will not work as expected

Expected behaviour

WebSocket.readyState should expose integers rather than string values.

Actual behaviour

WebSocket.readyState returns a String instead of Integer

@dougw-bc dougw-bc added the bug Something isn't working label Sep 26, 2024
@mstoykov mstoykov assigned mstoykov and unassigned oleiade Sep 27, 2024
@mstoykov mstoykov transferred this issue from grafana/k6 Sep 27, 2024
@mstoykov
Copy link
Collaborator

@dougw-bc can you please provide an exampe of this as it seems like the implementaiton is with integers and that has been the case for the last 2 years at least.

Are you sure you are not runnign a particularly old version of k6.

@oleiade oleiade removed the triage label Sep 27, 2024
@dougw-bc
Copy link
Author

dougw-bc commented Sep 27, 2024

I noticed this when trying to test an Elixir Phoenix Channel websocket server.

Here is the code that was not functioning as expected on k6:
https:/phoenixframework/phoenix/blob/main/assets/js/phoenix/socket.js#L525-L532

Updating this constant: https:/phoenixframework/phoenix/blob/main/assets/js/phoenix/constants.js#L5 now behaves 'correctly'

UPDATE: - this still behaves oddly, but not because of the websocket.readyState return value, it is something to do with the switch statement

Here is a reproduction of the issue with a small script:

import { WebSocket } from "k6/experimental/websockets";

export const options = {
  vus: 1,
  iterations: 1,
};

// this just hids the default it seems - maybe with any integer? 
function connectionState(state) {
  switch (state) {
    case 0:
      return "connecting";
    case 1:
      return "open";
    case 2:
      return "closing";
    case 3:
      return "closed";
    default:
      return "uh oh";
  }
}

export default async () => {
  const url = "wss://test-api.k6.io/ws/crocochat/publicRoom/";
  const socket = new WebSocket(url);

  socket.onopen = () => {
    console.log("connected! ", connectionState(socket.readyState));
  };

  socket.onclose = () => {
    console.log("closed! ", connectionState(socket.readyState));
  };

  console.log(connectionState(socket.readyState));

  const checkInterval = setInterval(() => {
    console.log(connectionState(socket.readyState));
  }, 250);

  setTimeout(function () {
    console.log(`Closing the socket`);
    socket.close();
    clearInterval(checkInterval);
  }, 3000);
};

This is on the following version: k6 v0.53.0 (commit/f82a27da8f, go1.22.6, linux/amd64)

My logs look like this:

INFO[0000] uh oh                                         source=console
INFO[0000] uh oh                                         source=console
INFO[0000] connected!  uh oh                     source=console
INFO[0000] uh oh                                         source=console
INFO[0000] uh oh                                         source=console
INFO[0001] uh oh                                         source=console
INFO[0001] uh oh                                         source=console
INFO[0001] uh oh                                         source=console
INFO[0001] uh oh                                         source=console
INFO[0002] uh oh                                         source=console
INFO[0002] uh oh                                         source=console
INFO[0002] uh oh                                         source=console
INFO[0002] uh oh                                         source=console
INFO[0003] Closing the socket                     source=console
INFO[0003] closed!  uh oh                            source=console

@dougw-bc
Copy link
Author

After further testing - there might be something else going on with the switch statement itself - as this seems to work as expected:
Maybe this is a bug with switch?

const vals = { 0: "connecting", 1: "open", 2: "closing", 3: "closed" };

function connectionState(state) {
  return vals[state] || "uh oh";
}

mstoykov added a commit that referenced this issue Sep 27, 2024
for some reason - likely a bug in goja/Sobek returning a type that is
uint8 under the hood (or int) doesn't actually get to be a number in js.

The current fix casts the ReadyState type to uint8 so that Sobek handles
it correctly.

Fixes #79
@mstoykov
Copy link
Collaborator

Whank you for reportuing this 🙇

I managed to reproduce it and it seems to be a Sobek/goja bug where if you have type that is uint8 under the hood and return it goja/Sobek makes it into a string instead. But if you just return uint8 it works ... 🤦

I will try to go further to fix it in the goja/Sobek code, but until then I have made #80

mstoykov added a commit that referenced this issue Sep 27, 2024
for some reason - likely a bug in goja/Sobek returning a type that is
uint8 under the hood (or int) doesn't actually get to be a number in js.

The current fix casts the ReadyState type to uint8 so that Sobek handles
it correctly.

Fixes #79
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants