refactor(new-webui): Improve logging for easier debugging; Improve names of Mongo SocketIO library classes.#997
Conversation
WalkthroughThis update standardizes class and variable names related to MongoDB socket collections and cursors throughout the codebase, renaming them for consistency. Logging statements are revised to use Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant MongoSocketCollection
participant MongoSocketCursor
participant Server
UI->>MongoSocketCollection: new MongoSocketCollection()
UI->>MongoSocketCollection: find(query, options)
MongoSocketCollection->>MongoSocketCursor: new MongoSocketCursor(query, options)
MongoSocketCursor->>Server: subscribe(query, options)
Server-->>MongoSocketCursor: subscription confirmation / data
MongoSocketCursor-->>UI: emits data updates
UI->>MongoSocketCursor: unsubscribe()
MongoSocketCursor->>Server: unsubscribe(queryId)
Server-->>MongoSocketCursor: unsubscription confirmation
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 10
🔭 Outside diff range comments (3)
components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/index.ts (2)
229-233: 🧹 Nitpick (assertive)Replace string concatenation with a single template literal
Biome flagged this; switching removes the unnecessary
+operators and is easier to read.- this.#fastify.log.debug( - `Socket:${socket.id} requested query:${JSON.stringify(query)} ` + - `with options:${JSON.stringify(options)} to collection:${collectionName}` - ); + this.#fastify.log.debug( + `Socket:${socket.id} requested query:${JSON.stringify(query)} with options:${JSON.stringify(options)} to collection:${collectionName}` + );
318-324: 🧹 Nitpick (assertive)Template-literal cleanup & minor re-wrap
Same concatenation issue as above plus a trailing line break inside the template.
- this.#fastify.log.debug( - "Query ID to query hash map:" + - ` ${JSON.stringify(Array.from(this.#queryIdToQueryHashMap.entries()))}` - ); + this.#fastify.log.debug( + `Query ID to query hash map: ${JSON.stringify( + Array.from(this.#queryIdToQueryHashMap.entries()), + )}`, + );🧰 Tools
🪛 Biome (1.9.4)
[error] 318-319: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
[error] 323-324: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
components/log-viewer-webui/client/src/api/socket/MongoSocketCollection.ts (1)
15-28: 🧹 Nitpick (assertive)Typo in JSDoc.
Initalizes→Initializes.- * Initalizes socket connection to a MongoDB collection on the server. + * Initializes socket connection to a MongoDB collection on the server.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
components/log-viewer-webui/client/src/api/socket/MongoSocketCollection.ts(3 hunks)components/log-viewer-webui/client/src/api/socket/MongoSocketCursor.ts(4 hunks)components/log-viewer-webui/client/src/api/socket/useCursor.tsx(1 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/search-requests.ts(2 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/useSearchResults.ts(2 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/useAggregationResults.ts(2 hunks)components/log-viewer-webui/client/src/pages/SearchPage/SearchState/useResultsMetadata.ts(2 hunks)components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/index.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/useSearchResults.tscomponents/log-viewer-webui/client/src/pages/SearchPage/SearchControls/search-requests.tscomponents/log-viewer-webui/client/src/api/socket/useCursor.tsxcomponents/log-viewer-webui/server/src/plugins/MongoSocketIoServer/index.tscomponents/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/useAggregationResults.tscomponents/log-viewer-webui/client/src/api/socket/MongoSocketCursor.tscomponents/log-viewer-webui/client/src/pages/SearchPage/SearchState/useResultsMetadata.tscomponents/log-viewer-webui/client/src/api/socket/MongoSocketCollection.ts
🧠 Learnings (3)
components/log-viewer-webui/client/src/api/socket/useCursor.tsx (1)
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
components/log-viewer-webui/client/src/api/socket/MongoSocketCursor.ts (1)
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
components/log-viewer-webui/client/src/api/socket/MongoSocketCollection.ts (1)
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
🪛 Biome (1.9.4)
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/search-requests.ts
[error] 92-92: This let declares some variables that are only assigned once.
'searchJobId' is never reassigned.
'aggregationJobId' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/index.ts
[error] 323-324: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build (macos-latest)
- GitHub Check: lint-check (macos-latest)
- GitHub Check: lint-check (ubuntu-latest)
🔇 Additional comments (9)
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/useSearchResults.ts (1)
1-1: Renamed import – looks goodThe switch to
MongoSocketCollectionkeeps naming consistent with the rest of the refactor. No issues spotted here.components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/index.ts (1)
323-324: Single template literal & full stop for consistency- this.#fastify.log.debug(`Collection:${queryParams.collectionName}` + - " deallocated from server."); + this.#fastify.log.debug( + `Collection:${queryParams.collectionName} deallocated from server.`, + );🧰 Tools
🪛 Biome (1.9.4)
[error] 323-324: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
components/log-viewer-webui/client/src/pages/SearchPage/SearchState/useResultsMetadata.ts (1)
3-3: Import rename aligns with API change – goodcomponents/log-viewer-webui/client/src/api/socket/useCursor.tsx (2)
8-8: Import rename looks correctNo functional impact – matches other files.
23-24: Generic type preserved – all goodcomponents/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/useAggregationResults.ts (2)
1-1: Import rename looks correct.The switch to
MongoSocketCollectionkeeps the naming consistent with the broader refactor. 👍
27-28: Instantiation aligns with the new class name.No functional issues spotted.
components/log-viewer-webui/client/src/api/socket/MongoSocketCursor.ts (1)
114-114: Nice unsubscribe trace.Helps correlate server-side unsubscription events.
components/log-viewer-webui/client/src/api/socket/MongoSocketCollection.ts (1)
39-44: Return type correctly updated to new cursor class.No issues—matches the refactor and preserves the singleton socket usage documented earlier.
| console.log( | ||
| `Subscribing to updates to search results with job ID: ${searchJobId}` | ||
| ); | ||
|
|
||
| const collection = new MongoSocketCollection(searchJobId.toString()); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use console.debug instead of console.log for parity with the new logging policy
All other new client-side logs were moved to console.debug; this lone console.log will stick out and re-introduce noise when log levels are filtered.
- console.log(
+ console.debug(
`Subscribing to updates to search results with job ID: ${searchJobId}`
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log( | |
| `Subscribing to updates to search results with job ID: ${searchJobId}` | |
| ); | |
| const collection = new MongoSocketCollection(searchJobId.toString()); | |
| console.debug( | |
| `Subscribing to updates to search results with job ID: ${searchJobId}` | |
| ); | |
| const collection = new MongoSocketCollection(searchJobId.toString()); |
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/useSearchResults.ts
between lines 23 and 27, replace the console.log call with console.debug to
align with the new logging policy and reduce noise when filtering log levels.
This change ensures consistent use of debug-level logging for client-side logs.
| const collection = new MongoSocketCollection( | ||
| settings.MongoDbSearchResultsMetadataCollectionName | ||
| ); | ||
|
|
||
| console.log( | ||
| `Subscribing to updates for results metadata for search job ID: ${searchJobId}` | ||
| ); | ||
|
|
||
| return collection.find({_id: searchJobId.toString()}, {limit: 1}); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Switch to console.debug for uniform log level
Same rationale as in useSearchResults.ts.
- console.log(
+ console.debug(
`Subscribing to updates for results metadata for search job ID: ${searchJobId}`
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const collection = new MongoSocketCollection( | |
| settings.MongoDbSearchResultsMetadataCollectionName | |
| ); | |
| console.log( | |
| `Subscribing to updates for results metadata for search job ID: ${searchJobId}` | |
| ); | |
| return collection.find({_id: searchJobId.toString()}, {limit: 1}); | |
| const collection = new MongoSocketCollection( | |
| settings.MongoDbSearchResultsMetadataCollectionName | |
| ); | |
| console.debug( | |
| `Subscribing to updates for results metadata for search job ID: ${searchJobId}` | |
| ); | |
| return collection.find({_id: searchJobId.toString()}, {limit: 1}); |
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchState/useResultsMetadata.ts
between lines 25 and 33, replace the console.log statement with console.debug to
maintain consistent log levels across the codebase, matching the approach used
in useSearchResults.ts. This change ensures debug-level messages are uniformly
handled.
There was a problem hiding this comment.
@davemarco Understood—keeping the console.log as-is.
| console.debug("Query cancelled successfully"); | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Nit: add context to cancellation log
Including IDs helps when multiple tabs/users are active.
- console.debug("Query cancelled successfully");
+ console.debug(`Query ${payload.searchJobId} cancelled successfully`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.debug("Query cancelled successfully"); | |
| }) | |
| console.debug(`Query ${payload.searchJobId} cancelled successfully`); | |
| }) |
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/search-requests.ts
at lines 123-124, the console.debug log for query cancellation lacks context.
Modify the log message to include relevant identifiers such as query IDs or
user/session IDs to help distinguish cancellations when multiple tabs or users
are active.
| console.debug("Search job created - ", | ||
| "Search job ID:", searchJobId, | ||
| "Aggregation job ID:", aggregationJobId); | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Optional: collapse multi-argument console.debug into a template for readability
Not critical, just easier to copy-paste from logs.
- console.debug("Search job created - ",
- "Search job ID:", searchJobId,
- "Aggregation job ID:", aggregationJobId);
+ console.debug(
+ `Search job created - Search job ID: ${searchJobId}, Aggregation job ID: ${aggregationJobId}`,
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.debug("Search job created - ", | |
| "Search job ID:", searchJobId, | |
| "Aggregation job ID:", aggregationJobId); | |
| }) | |
| console.debug( | |
| `Search job created - Search job ID: ${searchJobId}, Aggregation job ID: ${aggregationJobId}`, | |
| ); | |
| }) |
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchControls/search-requests.ts
around lines 96 to 99, the console.debug call uses multiple arguments which can
reduce readability and make copying logs harder. Refactor this to use a single
template literal string that includes the searchJobId and aggregationJobId
variables inline for clearer, more readable output.
| * Custom hook which returns a real-time reactive array of documents from a `MongoSocketCursor`. | ||
| * | ||
| * @template T The document type returned by the cursor. | ||
| * @param query Function which returns a `MongoCursorSocket` instance or null. | ||
| * @param query Function which returns a `MongoSocketCursor` instance or null. | ||
| * @param dependencies Array of dependencies for the query. | ||
| * @return | ||
| * - If `query` returns a `MongoCursorSocket` instance, then hook returns null while | ||
| * - If `query` returns a `MongoSocketCursor` instance, then hook returns null while |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Updated JSDoc – spellcheck minor typo
The word “recieved” in the comment below is still misspelled.
- // recieved the queryID from the server, making it impossible to unsubscribe
+ // received the queryID from the server, making it impossible to unsubscribeCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In components/log-viewer-webui/client/src/api/socket/useCursor.tsx around lines
12 to 18, correct the spelling of the word "recieved" to "received" in the JSDoc
comment to fix the minor typo.
| console.log( | ||
| `Subscribing to updates to aggregation results with job ID: ${aggregationJobId}` | ||
| ); | ||
|
|
||
| const collection = new MongoSocketCollection(aggregationJobId.toString()); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use console.debug for consistency with new logging policy.
All other updated modules migrated from console.log/console.info to console.debug to reduce noise in production builds. These two lines revert to console.log, breaking that consistency.
- console.log(
+ console.debug(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log( | |
| `Subscribing to updates to aggregation results with job ID: ${aggregationJobId}` | |
| ); | |
| const collection = new MongoSocketCollection(aggregationJobId.toString()); | |
| console.debug( | |
| `Subscribing to updates to aggregation results with job ID: ${aggregationJobId}` | |
| ); | |
| const collection = new MongoSocketCollection(aggregationJobId.toString()); |
🤖 Prompt for AI Agents
In
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTimeline/useAggregationResults.ts
between lines 23 and 27, replace the console.log statement with console.debug to
maintain consistency with the new logging policy and reduce noise in production
builds.
| console.debug( | ||
| `Subscribing to query: ${JSON.stringify(this.#query)} ` + | ||
| `on collection:${this.#collectionName}` | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Good adoption of console.debug, but drop string concatenation.
Since you’re already using template literals, the + concatenation is unnecessary and slightly harder to scan.
- console.debug(
- `Subscribing to query: ${JSON.stringify(this.#query)} ` +
- `on collection:${this.#collectionName}`
- );
+ console.debug(
+ `Subscribing to query: ${JSON.stringify(this.#query)} on collection: ${this.#collectionName}`,
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.debug( | |
| `Subscribing to query: ${JSON.stringify(this.#query)} ` + | |
| `on collection:${this.#collectionName}` | |
| ); | |
| console.debug( | |
| `Subscribing to query: ${JSON.stringify(this.#query)} on collection: ${this.#collectionName}`, | |
| ); |
🤖 Prompt for AI Agents
In components/log-viewer-webui/client/src/api/socket/MongoSocketCursor.ts around
lines 55 to 58, remove the string concatenation operator (+) within the
console.debug call. Instead, combine the entire message into a single template
literal for better readability and cleaner code.
| console.debug( | ||
| `Successfully subscribed to query: ${JSON.stringify(this.#query)} ` + | ||
| `on collection:${this.#collectionName} with MongoSocketIoQueryID:${this.#queryId}` | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Same minor formatting nit as above.
- console.debug(
- `Successfully subscribed to query: ${JSON.stringify(this.#query)} ` +
- `on collection:${this.#collectionName} with MongoSocketIoQueryID:${this.#queryId}`
- );
+ console.debug(
+ `Successfully subscribed to query: ${JSON.stringify(this.#query)} on collection: ${this.#collectionName} with MongoSocketIoQueryID: ${this.#queryId}`,
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.debug( | |
| `Successfully subscribed to query: ${JSON.stringify(this.#query)} ` + | |
| `on collection:${this.#collectionName} with MongoSocketIoQueryID:${this.#queryId}` | |
| ); | |
| console.debug( | |
| `Successfully subscribed to query: ${JSON.stringify(this.#query)} on collection: ${this.#collectionName} with MongoSocketIoQueryID: ${this.#queryId}`, | |
| ); |
🤖 Prompt for AI Agents
In components/log-viewer-webui/client/src/api/socket/MongoSocketCursor.ts around
lines 88 to 91, the console.debug statement has a minor formatting issue with
string concatenation. Refactor the code to use a single template literal instead
of concatenating two strings with the + operator for cleaner and more consistent
formatting.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/index.ts (1)
229-233: 🧹 Nitpick (assertive)Collapse string concatenation in debug statement
Template literals alone suffice.
- this.#fastify.log.debug( - `Socket:${socket.id} requested query:${JSON.stringify(query)} ` + - `with options:${JSON.stringify(options)} to collection:${collectionName}` - ); + this.#fastify.log.debug( + `Socket:${socket.id} requested query: ${JSON.stringify(query)} with options: ${JSON.stringify(options)} to collection: ${collectionName}`, + );
♻️ Duplicate comments (3)
components/log-viewer-webui/client/src/api/socket/MongoSocketCursor.ts (2)
55-59: 🧹 Nitpick (assertive)Use a single template literal instead of
+concatenationThe same nitpick was raised before; concatenation is still present. One template literal is clearer and eliminates the extra whitespace bookkeeping.
- console.debug( - `Subscribing to query: ${JSON.stringify(this.#query)} ` + - `with options:${JSON.stringify(this.#options)} ` + - `on collection:${this.#collectionName}` - ); + console.debug( + `Subscribing to query: ${JSON.stringify(this.#query)} with options: ${JSON.stringify(this.#options)} on collection: ${this.#collectionName}`, + );
90-94: 🧹 Nitpick (assertive)Same concatenation nitpick applies here
Please collapse into one template literal for consistency with other debug statements.
- console.debug( - `Successfully subscribed to query: ${JSON.stringify(this.#query)} ` + - `with options:${JSON.stringify(this.#options)} ` + - `on collection:${this.#collectionName} ` + - `MongoSocketIoQueryID:${this.#queryId}` - ); + console.debug( + `Successfully subscribed to query: ${JSON.stringify(this.#query)} with options: ${JSON.stringify(this.#options)} on collection: ${this.#collectionName} MongoSocketIoQueryID: ${this.#queryId}`, + );components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/index.ts (1)
255-259: 🧹 Nitpick (assertive)Downgrade to
debugfor log-level consistency (repeat comment)All other subscription-flow logs were moved to
debug; keeping this atinfowill re-introduce noise.- this.#fastify.log.info( + this.#fastify.log.debug( `Socket:${socket.id} subscribed to query:${JSON.stringify(query)} ` + `with options:${JSON.stringify(options)} ` + `on collection:${collectionName} with ID:${queryId}` );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/log-viewer-webui/client/src/api/socket/MongoSocketCursor.ts(4 hunks)components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/index.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
components/log-viewer-webui/client/src/api/socket/MongoSocketCursor.tscomponents/log-viewer-webui/server/src/plugins/MongoSocketIoServer/index.ts
🧠 Learnings (1)
components/log-viewer-webui/client/src/api/socket/MongoSocketCursor.ts (1)
Learnt from: davemarco
PR: y-scope/clp#892
File: components/log-viewer-webui/client/src/api/socket/MongoCollectionSocket.ts:27-31
Timestamp: 2025-05-09T19:15:26.180Z
Learning: For the MongoDB real-time updates implementation in components/log-viewer-webui/client/src/api/socket, a socket singleton pattern is used where a single shared socket connection is maintained rather than creating multiple connections. The socket lifecycle is managed centrally, with unsubscription handling in the useCursor React hook's cleanup function.
🪛 Biome (1.9.4)
components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/index.ts
[error] 324-325: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint-check (ubuntu-latest)
- GitHub Check: lint-check (macos-latest)
| } | ||
|
|
||
| console.log(`Unsubscribed to queryID:${this.#queryId}.`); | ||
| console.debug(`Unsubscribed from MongoSocketIoQueryID:${this.#queryId}.`); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Minor readability tweak
Add a space after the colon to align with the other log messages.
- console.debug(`Unsubscribed from MongoSocketIoQueryID:${this.#queryId}.`);
+ console.debug(`Unsubscribed from MongoSocketIoQueryID: ${this.#queryId}.`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.debug(`Unsubscribed from MongoSocketIoQueryID:${this.#queryId}.`); | |
| console.debug(`Unsubscribed from MongoSocketIoQueryID: ${this.#queryId}.`); |
🤖 Prompt for AI Agents
In components/log-viewer-webui/client/src/api/socket/MongoSocketCursor.ts at
line 117, the debug log message lacks a space after the colon in
"MongoSocketIoQueryID:". Add a space after the colon so the message reads
"MongoSocketIoQueryID: " to improve readability and maintain consistency with
other log messages.
junhaoliao
left a comment
There was a problem hiding this comment.
some changes in this PR are not directly related to logging (e.g., the MongoSocketCollection renaming) While I agree the proposed names are more accurate, can we split those changes into a separate PR?
…rver/index.ts Co-authored-by: Junhao Liao <junhao@junhao.ca>
…rver/index.ts Co-authored-by: Junhao Liao <junhao@junhao.ca>
…rver/index.ts Co-authored-by: Junhao Liao <junhao@junhao.ca>
|
How do you feel about changing title to I feel the change is small, and this PR has convential commit refactor? anyways let me know |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/useSearchResults.ts (1)
26-29: Replaceconsole.logwithconsole.debugfor logging-level consistency
All new client-side logs were migrated toconsole.debug; keeping a loneconsole.logre-introduces noise when filtering log levels.- console.log( + console.debug( `Subscribing to updates to search results with job ID: ${searchJobId}` );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/useSearchResults.ts(3 hunks)components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/index.ts(7 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}`: - Prefer `false == ` rather than `!`.
**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}: - Preferfalse == <expression>rather than!<expression>.
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/useSearchResults.tscomponents/log-viewer-webui/server/src/plugins/MongoSocketIoServer/index.ts
🪛 Biome (1.9.4)
components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/index.ts
[error] 323-324: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: lint-check (ubuntu-latest)
- GitHub Check: lint-check (macos-latest)
- GitHub Check: build (macos-latest)
🔇 Additional comments (5)
components/log-viewer-webui/client/src/pages/SearchPage/SearchResults/SearchResultsTable/useSearchResults.ts (1)
45-46: LGTM – correct class rename applied
Instantiation reflects the newMongoSocketCollectionclass. No further action required.components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/index.ts (4)
204-206: 👍 Switched todebug– matches reduced verbosity goal
Moving the collection-initialisation log todebugaligns with the overall logging-level refactor.
229-233: Detailed subscription request logged atdebug– looks good
The richer context (query/options) will aid troubleshooting without polluting higher log levels.
342-343: Unsubscription request logged atdebug– consistent with subscribe flow
No issues found.
295-296: 🧹 Nitpick (assertive)Use template literal to satisfy linter & improve readability
- this.#fastify.log.error(`Query:${queryId} not found in query map`); + this.#fastify.log.error(`Query:${queryId} not found in query map`);(Only exemplar; no functional change, but keeps style uniform and clears Biome’s
useTemplatewarning.)Likely an incorrect or invalid review comment.
| this.#fastify.log.info( | ||
| `Socket:${socket.id} subscribed to query:${JSON.stringify(query)} ` + | ||
| `with options:${JSON.stringify(options)} ` + | ||
| `on collection:${collectionName} with ID:${queryId}` | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider downgrading this final “subscribed” message to debug
Earlier discussion left it at info, but now both the request (229-233) and most watcher lifecycle logs are debug. Re-evaluating could further tame noise.
🤖 Prompt for AI Agents
In components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/index.ts
around lines 255 to 259, the log message indicating a socket subscription is
currently at info level, which is inconsistent with earlier request and watcher
lifecycle logs set at debug level. Change the log level from info to debug for
this message to reduce log noise and maintain consistency.
| this.#fastify.log.debug(`Collection:${queryParams.collectionName}` + | ||
| " deallocated from server."); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Swap string concatenation for template literal
- this.#fastify.log.debug(`Collection:${queryParams.collectionName}` +
- " deallocated from server.");
+ this.#fastify.log.debug(
+ `Collection:${queryParams.collectionName} deallocated from server.`
+ );This removes the lint error (useTemplate) and is easier to read.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.#fastify.log.debug(`Collection:${queryParams.collectionName}` + | |
| " deallocated from server."); | |
| this.#fastify.log.debug( | |
| `Collection:${queryParams.collectionName} deallocated from server.` | |
| ); |
🧰 Tools
🪛 Biome (1.9.4)
[error] 323-324: Template literals are preferred over string concatenation.
Unsafe fix: Use a template literal.
(lint/style/useTemplate)
🤖 Prompt for AI Agents
In components/log-viewer-webui/server/src/plugins/MongoSocketIoServer/index.ts
at lines 323-324, replace the string concatenation used in the debug log
statement with a single template literal. This means combining the entire
message inside backticks and embedding the variable directly using ${}, which
will remove the lint error and improve readability.
…mes of Mongo SocketIO library classes. (y-scope#997)
Description
PR adds more info to certains logs, and reduces verbosity of others to reduce clutter when debugging. I decided to make these changes while i was testing.
Adds a few new logs as well.
I also renamed a couple variables.
Checklist
breaking change.
Validation performed
Logs look better when running queries
Summary by CodeRabbit
Summary by CodeRabbit