Repository navigation
feat(new-webui): Add client library for real-time MongoDB updates. #892
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,63 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ClientToServerEvents, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ServerToClientEvents, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from "@common/index.js"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| io, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Socket, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from "socket.io-client"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import {MongoCursorSocket} from "./MongoCursorSocket.js"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Socket connection to a MongoDB collection residing on a server. Class provides methods to | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * query the collection. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| class MongoCollectionSocket { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private socket: Socket<ServerToClientEvents, ClientToServerEvents>; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
davemarco marked this conversation as resolved.
Outdated
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Initalizes socket connection to a MongoDB collection on the server. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param collectionName | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+20
to
+24
Contributor
There was a problem hiding this comment. 🧹 Nitpick (assertive) Fix typo and clarify JSDoc in constructor - * Initalizes socket connection to a MongoDB collection on the server.
+ * Initializes the shared socket connection for a MongoDB collection on the server.
+ *
+ * @param collectionName - The name of the MongoDB collection to subscribe to.📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| constructor (collectionName: string) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // eslint-disable-next-line no-warning-comments | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // TODO: Add support for user provided domain name (i.e. io("https://server-domain.com")). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Implementation could involve parsing server .env file and moving server .env to a | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // common folder. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // eslint-disable-next-line no-warning-comments | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // TODO: Current implementation creates a new socket connection for each collection. This | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // could be problematic if the number of collections is large since browsers limit the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // number of web sockets per domain. An simple change is to use Socket.IO namespace | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // feature, and having Socket.IO multiplex the socket per collection namespace. Another | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // more involved option is to implement a shared socket. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Namespace reference: https://socket.io/docs/v4/namespaces/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.socket = io(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.socket.emit("collection::init", { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| collectionName: collectionName, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.log(`MongoDB collection:${collectionName} initialized.`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+25
to
+29
Contributor
There was a problem hiding this comment. 💡 Verification agent 🧩 Analysis chainSocket lifecycle is never closed – risk of resource leaks
Diff sketch: + /**
+ * Cleanly tears down the socket connection.
+ * Call this when the owning component unmounts.
+ */
+ public disconnect(): void {
+ if (this.socket.connected) {
+ this.socket.disconnect();
+ }
+ }Expose this method (or share a singleton socket—see next comment) and invoke it from the consumer (e.g., inside 🏁 Script executed: #!/bin/bash
# Inspect MongoCollectionSocket.ts and search for disconnect/close usage
FILE="components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts"
echo "=== Contents of $FILE ==="
sed -n '1,200p' "$FILE"
echo
echo "=== Searching for 'disconnect' across the repo ==="
rg "disconnect" -n .
echo
echo "=== Searching for 'close' across the repo ==="
rg "close" -n .Length of output: 66430 Add socket teardown to prevent connection leaks The Please implement and expose a cleanup method and invoke it from the owning component’s unmount logic: • File: /**
* Cleanly disconnects the socket.
* Call this when the owning component unmounts.
*/
public disconnect(): void {
if (this.socket.connected) {
this.socket.disconnect();
}
}• Consumer (e.g. in a React component): const collectionSocket = new MongoCollectionSocket("myCollection");
useEffect(() => {
return () => {
collectionSocket.disconnect();
};
}, []);This ensures sockets are closed when no longer needed.
Contributor
Author
There was a problem hiding this comment. When the class is garbage collected, will it automatically disconnect?
Contributor
There was a problem hiding this comment. 🧩 Analysis chainI need to research this question about Socket.IO garbage collection behavior to give you an accurate answer. 🌐 Web query: 💡 Result: Does Socket.IO Automatically Disconnect When the Socket Object Is Garbage Collected in JavaScript?Summary: Detailed Explanation1. Socket.IO Connection Lifecycle
2. Garbage Collection in JavaScript
3. Implicit vs. Explicit Disconnection
4. Memory and Resource Management
5. Analogies and Related Technologies
ConclusionSocket.IO does not automatically disconnect sockets when their JavaScript object is garbage collected. Citations:
@davemarco No, Socket.IO connections are not automatically disconnected when the object is garbage collected. JavaScript's garbage collector only frees memory; it doesn't handle cleanup of external resources like network connections. The socket connection will remain open even after the class instance is garbage collected, which can lead to:
You should implement the suggested /**
* Cleanly disconnects the socket.
* Call this when the owning component unmounts.
*/
public disconnect(): void {
if (this.socket.connected) {
this.socket.disconnect();
}
}This is standard practice for managing connection lifecycle in networking libraries. ✏️ Learnings added
Contributor
Author
There was a problem hiding this comment. hmm i imagine this will be difficult to implement in the current useCusor hook. Will the connection automatically disconnect after a certain amount of no activity?
Contributor
There was a problem hiding this comment. 🧩 Analysis chain@davemarco Socket.IO does have a built-in ping/pong mechanism, but it doesn't automatically disconnect inactive connections by default. Here's how it works:
Regarding implementation in You could modify the // In useCursor.tsx
useEffect(() => {
// ... existing subscription logic ...
return () => {
// Cleanup function when hook unmounts
if (cursor) {
cursor.unsubscribe();
// If you had access to the MongoCollectionSocket instance:
// collectionSocket.disconnect();
}
};
}, [dependencies]);The challenge is getting access to the underlying
(●__●) 🌐 Web query: 💡 Result: Socket.IO and Automatic Disconnection on InactivitySummary: How the Timeout Works
Example Configuration (Node.js): const io = new Server({
pingInterval: 25000,
pingTimeout: 20000,
});You can increase Common Scenarios and Limitations
Key Takeaway
References to the Mechanism in Official Documentation:
Conclusion: Citations:
@davemarco Yes, Socket.IO has a built-in heartbeat mechanism, but it's designed to detect broken connections, not application inactivity:
This mechanism only detects network failures. If your connection is healthy but idle, it will remain open indefinitely. For implementing disconnection in
The simplest approach would be to pass the collection instance to the cursor and expose a getter: // When creating the cursor
const cursor = collectionSocket.find(query, options);
useEffect(() => {
// ...subscription logic
return () => {
cursor.unsubscribe();
cursor.getCollection().disconnect();
}
}, []);(x__x) ✏️ Learnings added
Contributor
Author
There was a problem hiding this comment. No this looks terrible. will namespace help?
Contributor
There was a problem hiding this comment. 🧩 Analysis chain@davemarco Yes, namespaces would help significantly! They're actually the perfect solution for this problem. Socket.IO namespaces allow multiple logical connections to share a single physical WebSocket connection. This directly addresses your TODO comment about browser connection limits. Here's how you could implement it: // SocketManager.ts - Singleton to manage the socket connection
export class SocketManager {
private static instance: SocketManager;
private socket: Socket;
private collectionSockets: Map<string, Socket> = new Map();
private constructor() {
this.socket = io();
}
public static getInstance(): SocketManager {
if (!SocketManager.instance) {
SocketManager.instance = new SocketManager();
}
return SocketManager.instance;
}
public getCollectionSocket(collectionName: string): Socket {
if (!this.collectionSockets.has(collectionName)) {
// Create a namespace for this collection
const collectionSocket = this.socket.io(`/${collectionName}`);
this.collectionSockets.set(collectionName, collectionSocket);
}
return this.collectionSockets.get(collectionName)!;
}
public disconnect(): void {
if (this.socket.connected) {
this.socket.disconnect();
this.collectionSockets.clear();
}
}
}
// Modified MongoCollectionSocket.ts
class MongoCollectionSocket {
private socket: Socket<ServerToClientEvents, ClientToServerEvents>;
constructor(collectionName: string) {
// Get the namespaced socket from the singleton manager
this.socket = SocketManager.getInstance().getCollectionSocket(collectionName);
this.socket.emit("collection::init", { collectionName });
console.log(`MongoDB collection:${collectionName} initialized.`);
}
// Rest of your class...
}Benefits:
This avoids the "terrible" approach of having to manage disconnection in every component that uses a collection. 🌐 Web query: 💡 Result: Socket.IO namespaces help organize and separate different communication channels within a single Socket.IO connection. They provide a way to split the connection into multiple logical endpoints, each with its own event handlers and logic. However, namespaces alone do not fully solve connection and disconnection management problems. Here's a more detailed explanation:
Summary:
If you're looking to manage connection and disconnection issues comprehensively, consider combining namespaces with other Socket.IO features like rooms, middleware, and custom reconnection logic. @davemarco Yes, Socket.IO namespaces would solve the primary issue mentioned in your TODO comment - they allow multiple logical connections to share a single physical WebSocket connection. From the TODO in your code:
Here's how namespaces help:
Basic implementation approach: // Create a singleton socket manager
export class SocketManager {
private static instance: SocketManager;
private socket: Socket;
private constructor() {
this.socket = io(); // Single connection
}
public static getInstance(): SocketManager {
if (!SocketManager.instance) {
SocketManager.instance = new SocketManager();
}
return SocketManager.instance;
}
public getCollectionNamespace(collectionName: string): Socket {
return this.socket.io(`/${collectionName}`);
}
// Single disconnect point for application shutdown
public disconnect(): void {
this.socket.disconnect();
}
}This addresses both the connection limit issue and simplifies disconnection management by centralizing it. Note that server-side changes would also be needed to properly handle these namespaces. 🧠 Learnings used
Contributor
Author
There was a problem hiding this comment. I moved to shared socket which should fix the issue
Contributor
There was a problem hiding this comment. @davemarco Great! Using a shared socket singleton is the perfect solution for this issue. This implementation:
The (＾◡＾) 🧠 Learnings used
Comment on lines
+27
to
+29
Contributor
There was a problem hiding this comment. 🛠️ Refactor suggestion Expose a deterministic teardown path for the collection-level socket
Example: class MongoCollectionSocket {
…
+ /**
+ * Releases any listeners added by this instance.
+ * No-op when the shared socket is already closed.
+ */
+ public disconnect(): void {
+ if (this.socket.connected) {
+ // Only detach collection-specific listeners here
+ // (do not close the underlying shared socket).
+ this.socket.removeAllListeners("collection::find::update");
+ }
+ }
}📝 Committable suggestion
Suggested change
Contributor
Author
There was a problem hiding this comment. This is only one socket. So the only way to get another socket should disconnect the previous socket. I think it should be okay
Contributor
There was a problem hiding this comment. @davemarco I understand your point about using a socket singleton pattern. Since the socket is shared and managed centrally, and since the unsubscription is handled in the Given that the class isn't directly attaching listeners (those appear to be in the ✏️ Learnings added
Comment on lines
+25
to
+29
Contributor
There was a problem hiding this comment. 🧹 Nitpick (assertive) Replace console.log with proper logger |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Selects documents in collection and returns a cursor-like object. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param query | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param options | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @return a `MongoCursorSocket`. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| find (query: object, options: object) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new MongoCursorSocket( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.socket, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| query, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| options, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+41
to
+46
Contributor
There was a problem hiding this comment. 🧹 Nitpick (assertive) Strengthen typing & return signature of
In addition, the method relies on type inference for its return value. Being explicit helps consumers and documentation tools: - find (query: object, options: object) {
- return new MongoCursorSocket(
+ find<Q extends Record<string, unknown>, O extends Record<string, unknown>>(
+ query: Q,
+ options: O
+ ): MongoCursorSocket {
+ return new MongoCursorSocket(
this.socket,
this.collectionName,
query,
options,
);
}📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export default MongoCollectionSocket; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Relocate
vite-tsconfig-pathsto devDependencies.The
vite-tsconfig-pathsplugin is only needed at build time and for local development. Moving it todevDependencieswill reduce production install size and clarify its usage scope.