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

websockets seems broken in 1.32.4 after merging #18587 #18719

Closed
github-enetrino-com opened this issue Apr 16, 2023 · 2 comments
Closed

websockets seems broken in 1.32.4 after merging #18587 #18719

github-enetrino-com opened this issue Apr 16, 2023 · 2 comments

Comments

@github-enetrino-com
Copy link

perf(ext/websocket): replace tokio_tungstenite server with fastwebsockets #18587

The above commit seems to break websockets.

Symptom: websocket sends do not go through until new messages arrive to the connection.

It's easy to reproduce with example close to the one from https://medium.com/deno-the-complete-reference/native-web-sockets-client-server-in-deno-928678a65cf2 replacing prompts to input text with setTimeout with sending ping-pongs.

//chat_server.ts

import { serve } from "https://deno.land/std/http/mod.ts";
function logError(msg: string) {
  console.log(msg);
  Deno.exit(1);
}
function handleConnected() {
  console.log("Connected to client ...");
}
function handleMessage(ws: WebSocket, data: string) {
  console.log("CLIENT >> " + data);
  setTimeout(()=>ws.send('pong'),2000);
  // const reply = prompt("Server >> ") || "No reply";
  // if (reply === "exit") {
  //   return ws.close();
  // }
  // ws.send(reply as string);
}
function handleError(e: Event | ErrorEvent) {
  console.log(e instanceof ErrorEvent ? e.message : e.type);
}
async function reqHandler(req: Request) {
  if (req.headers.get("upgrade") != "websocket") {
    return new Response(null, { status: 501 });
  }
  const { socket: ws, response } = Deno.upgradeWebSocket(req);
  ws.onopen = () => handleConnected();
  ws.onmessage = (m) => handleMessage(ws, m.data);
  ws.onclose = () => logError("Disconnected from client ...");
  ws.onerror = (e) => handleError(e);
  return response;
}
console.log("Waiting for client ...");
serve(reqHandler, { port: 8001 });
//chat_client.ts

function logError(msg: string) {
  console.log(msg);
  Deno.exit(1);
}
function handleConnected(ws: WebSocket) {
  console.log("Connected to server ...");
  handleMessage(ws, "Welcome!");
}
function handleMessage(ws: WebSocket, data: string) {
  console.log("SERVER >> " + data);
  setTimeout(()=>ws.send('ping'),2000);
  // const reply = prompt("Client >> ") || "No reply";
  // if (reply === "exit") {
  //   return ws.close();
  // }
  // ws.send(reply as string);
}
function handleError(e: Event | ErrorEvent) {
  console.log(e instanceof ErrorEvent ? e.message : e.type);
}
console.log("Connecting to server ...");
try {
  const ws = new WebSocket("ws://localhost:8001");
  ws.onopen = () => handleConnected(ws);
  ws.onmessage = (m) => handleMessage(ws, m.data);
  ws.onclose = () => logError("Disconnected from server ...");
  ws.onerror = (e) => handleError(e);
} catch (err) {
  logError("Failed to connect to server ... exiting");
}

This example sends ping-pongs every 2 seconds on 1.32.3, but stalls on 1.32.4

@github-enetrino-com github-enetrino-com changed the title websockets seems broken in 1.32.4 by merging #18587 websockets seems broken in 1.32.4 after merging #18587 Apr 16, 2023
@etodanik
Copy link
Contributor

Yes, I can confirm! 1.32.4 wreaked complete havoc on our test suite (for a websocket based server project)

@littledivy
Copy link
Member

This is fixed on main. You can either upgrade to canary or downgrade to 1.32.3. Duplicate of #18715 #18700

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

3 participants