Skip to content

Commit

Permalink
fix: check if connection is closed at ping, and allow multiple same u…
Browse files Browse the repository at this point in the history
…ser tabs for organizers
  • Loading branch information
edalholt committed Nov 12, 2023
1 parent ad933e7 commit 51879fd
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 64 deletions.
74 changes: 21 additions & 53 deletions backend/utils/socketNotifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,19 @@ import { NTNUINoFromRequest } from "./wsCookieRetriever";
/*
* This file contains functions for storing connections and notifying users connected to the WebSocket servers.
* Because the connections are stored in arrays/local data structures, the service has to run on a single instance, as the connections cannot be shared between instances.
* For making it possible to run multiple instances, this part can be rewritten using for example a redis or mongoDB database.
* For making it possible to run multiple instances, this part can be rewritten using for example a Redis or the MongoDB database.
* However, this is not necessary for the current use case, as the one instance should be able to handle 350 concurrent connections on Azure basic plan. And thousands on the standard plan.
* https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/azure-subscription-service-limits
*/

// Store all active organizer connections, the connections are stored by their respective groupSlug.
// This makes it possible to send messages to all logged in organizers of a group.
export const organizerConnections = new Map<string, Map<number, WebSocket>>();
// Store all active participant connections, for access when sending messages about assembly.
// An organizer can be logged in on multiple devices, and multiple organizers can receive events from the same group.
export const organizerConnections = new Map<string, WebSocket[]>();
// Store all active lobby connections, for access when sending messages to assembly participants.
// Maximum one lobby connection per user.
export const lobbyConnections = new Map<number, WebSocket>();

export const waitingForPongLobby = new Map<number, boolean>();

const sendPing = (ws: WebSocket) => {
if (ws.readyState === WebSocket.OPEN) {
ws.ping();
Expand All @@ -27,18 +27,21 @@ const sendPing = (ws: WebSocket) => {
// Send ping to all participants to check if they are still connected and prevent the connection from closing.
export const startHeartbeatInterval = setInterval(() => {
lobbyConnections.forEach((ws: WebSocket, userID: number) => {
// If the participant has not responded to the last ping, close and remove the connection.
if (waitingForPongLobby.get(userID) === true) {
// Remove connection if it is closed by the client.
if (ws.readyState === WebSocket.CLOSED) {
lobbyConnections.delete(userID);
ws.close();
return;
}
waitingForPongLobby.set(userID, true);
sendPing(ws);
});

organizerConnections.forEach((socketList) => {
socketList.forEach((ws: WebSocket) => {
// Remove connection if it is closed by the client.
if (ws.readyState === WebSocket.CLOSED) {
socketList.splice(socketList.indexOf(ws), 1);
return;
}
sendPing(ws);
});
});
Expand Down Expand Up @@ -68,39 +71,11 @@ export const removeLobbyConnectionByCookie = (req: IncomingMessage) => {
}
};

export const removeOrganizerConnectionByCookie = (req: IncomingMessage) => {
const ntnuiNo = NTNUINoFromRequest(req);
if (ntnuiNo !== null) {
for (const groupSlug of organizerConnections.keys()) {
for (const ntnui_no of organizerConnections.get(groupSlug)?.keys() ||
[]) {
if (ntnui_no === ntnuiNo) {
try {
organizerConnections.get(groupSlug)?.delete(ntnui_no);
} catch (e) {
console.error(
"Tried to delete connection that did not exist. " + e
);
}
console.log(
"Organizer " + ntnui_no + " unsubscribed from group " + groupSlug
);
return;
}
}
}
}
};

export const storeOrganizerConnectionByNTNUINo = (
ntnui_no: number,
groupSlug: string,
ws: WebSocket
) => {
export const storeOrganizerConnection = (groupSlug: string, ws: WebSocket) => {
if (!organizerConnections.get(groupSlug)) {
organizerConnections.set(groupSlug, new Map<number, WebSocket>());
organizerConnections.set(groupSlug, [ws]);
}
organizerConnections.get(groupSlug)?.set(ntnui_no, ws);
organizerConnections.get(groupSlug)?.push(ws);
};

export const notifyOneParticipant = (ntnui_no: number, message: string) => {
Expand All @@ -110,23 +85,16 @@ export const notifyOneParticipant = (ntnui_no: number, message: string) => {
console.log(
"Could not notify user " +
ntnui_no +
" (disconnected). Is there a problem with the socket URL? (Ignore if testing / dev has restarted)"
" (disconnected). Is there a problem with the socket URL? (Ignore if headless testing / dev has restarted and wiped the connections)"
);
}
};

export const notifyOrganizers = (groupSlug: string, message: string) => {
for (const ntnui_no of organizerConnections.get(groupSlug)?.keys() || []) {
console.log("Sending message to organizer " + ntnui_no);
const socket = organizerConnections.get(groupSlug)?.get(ntnui_no);
if (socket) {
try {
socket.send(message);
} catch (e) {
console.error(
"Error when sending message to organizer " + ntnui_no + ": " + e
);
}
}
const connections = organizerConnections.get(groupSlug);
if (connections) {
connections.forEach((connection) => {
connection.send(message);
});
}
};
7 changes: 2 additions & 5 deletions backend/wsServers/lobby.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import { WebSocketServer } from "ws";
import {
storeLobbyConnectionByCookie,
waitingForPongLobby,
} from "../utils/socketNotifier";
import { storeLobbyConnectionByCookie } from "../utils/socketNotifier";
import { NTNUINoFromRequest } from "../utils/wsCookieRetriever";

Check warning on line 3 in backend/wsServers/lobby.ts

View workflow job for this annotation

GitHub Actions / prettier-and-lint (18.x)

'NTNUINoFromRequest' is defined but never used

export const lobbyWss = new WebSocketServer({ noServer: true });
Expand All @@ -13,6 +10,6 @@ lobbyWss.on("connection", function connection(ws, req) {

ws.on("pong", () => {
// The client responded to the ping, so the connection is still active.
waitingForPongLobby.set(NTNUINoFromRequest(req) || 0, false);
// Connections are deleted when the user logs out or closes the connection/tab.
});
});
8 changes: 3 additions & 5 deletions backend/wsServers/organizer.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
import { WebSocketServer } from "ws";
import { NTNUINoFromRequest } from "../utils/wsCookieRetriever";
import { User } from "../models/user";
import {
removeOrganizerConnectionByCookie,
storeOrganizerConnectionByNTNUINo,
} from "../utils/socketNotifier";
import { storeOrganizerConnection } from "../utils/socketNotifier";

export const organizerWss = new WebSocketServer({ noServer: true });

Expand All @@ -31,7 +28,7 @@ organizerWss.on("connection", function connection(ws, req) {
membership.organizer && membership.groupSlug == groupSlug
)
) {
storeOrganizerConnectionByNTNUINo(ntnuiNo, groupSlug, ws);
storeOrganizerConnection(groupSlug, ws);
console.log(
"Organizer " + ntnuiNo + " are subscribed to group " + groupSlug
);
Expand All @@ -51,5 +48,6 @@ organizerWss.on("connection", function connection(ws, req) {

ws.on("pong", () => {
// The client responded to the ping, so the connection is still active.
// Connections are deleted when the assembly is deleted or the organizer closes the connection/tab.
});
});
2 changes: 1 addition & 1 deletion frontend/src/pages/AssemblyPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ export function AssemblyLobby() {
{kickedOut ? (
<WaitingRoom
message={
"You have logged in on another device, or you are kicked from this assembly."
"You have logged in on another device/tab, this tab is therefore disconnected."
}
/>
) : checkedIn && groupSlug && voted ? (
Expand Down

0 comments on commit 51879fd

Please sign in to comment.