bito-code-review[bot] commented on code in PR #40856:
URL: https://github.com/apache/superset/pull/40856#discussion_r3376774347
##########
superset-websocket/src/index.ts:
##########
@@ -159,6 +159,67 @@ export const setLastFirehoseId = (id: string): void => {
lastFirehoseId = id;
};
+// WebSocket close code used when a connection is refused because a configured
+// connection limit has been reached (1013 = "Try Again Later").
+const CONNECTION_LIMIT_CLOSE_CODE = 1013;
+
+/**
+ * Returns whether the socket with the given id is currently active, i.e. it is
+ * still registered and its underlying connection is in an active readyState.
+ *
+ * Closed sockets are only removed from the registries asynchronously (via the
+ * `checkSockets`/`cleanChannel` GC routines), so connection-limit checks must
+ * filter on live socket state rather than trusting the raw registry sizes.
+ */
+const isSocketActive = (socketId: string): boolean => {
+ const socketInstance = sockets[socketId];
+ return (
+ !!socketInstance &&
+ SOCKET_ACTIVE_STATES.includes(socketInstance.ws.readyState)
+ );
+};
+
+/**
+ * Counts the sockets in the global registry that are still active.
+ */
+const activeSocketCount = (): number =>
+ Object.keys(sockets).filter(isSocketActive).length;
+
+/**
+ * Counts the active sockets currently registered on the given channel.
+ */
+const activeChannelSocketCount = (channel: string): number =>
+ channels[channel]?.sockets.filter(isSocketActive).length ?? 0;
Review Comment:
<!-- Bito Reply -->
The changes you implemented correctly address the reviewer's suggestion by
exporting the helper functions `isSocketActive`, `activeSocketCount`, and
`activeChannelSocketCount`. This modification enables direct unit testing for
these functions, improving test coverage as requested.
**superset-websocket/src/index.ts**
```
export const isSocketActive = (socketId: string): boolean => {
const socketInstance = sockets[socketId];
return (
!!socketInstance &&
socketInstance.readyState === WebSocket.OPEN
);
};
export const activeSocketCount = (): number =>
Object.keys(sockets).filter(isSocketActive).length;
export const activeChannelSocketCount = (channel: string): number =>
channels[channel]?.sockets.filter(isSocketActive).length ?? 0;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
