-
Notifications
You must be signed in to change notification settings - Fork 92
refactor(new-webui): Improve logging for easier debugging; Improve names of Mongo SocketIO library classes. #997
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
Changes from all commits
2d00db2
c279f45
3f0fd19
fd190f5
84726c5
0964a8e
74d2cfe
b37f38a
0d52757
cb36bdf
02cd638
795c6bc
0ead028
2d9da72
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,22 +5,22 @@ import { | |
| } from "react"; | ||
|
|
||
| import {Nullable} from "../../typings/common"; | ||
| import {MongoCursorSocket} from "./MongoCursorSocket.js"; | ||
| import {MongoSocketCursor} from "./MongoSocketCursor.js"; | ||
|
|
||
|
|
||
| /** | ||
| * Custom hook which returns a real-time reactive array of documents from a `MongoCursorSocket`. | ||
| * 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 | ||
|
Comment on lines
+12
to
+18
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 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 unsubscribe
🤖 Prompt for AI Agents |
||
| * the subscription is pending, and a reactive array of documents when the subscription is ready. | ||
| * - If `query` returns null, then the hook also returns null. | ||
| */ | ||
| const useCursor = <T = object>( | ||
| query: () => Nullable<MongoCursorSocket>, | ||
| query: () => Nullable<MongoSocketCursor>, | ||
| dependencies: DependencyList = [] | ||
| ): Nullable<T[]> => { | ||
| const [data, setData] = useState<Nullable<T[]>>(null); | ||
|
|
@@ -37,7 +37,6 @@ const useCursor = <T = object>( | |
|
|
||
| // Flag to ignore updates after unmounting. | ||
| let ignore = false; | ||
| console.log("Subscribing to cursor"); | ||
|
|
||
| // Handler to set data updates from the server. | ||
| const onDataUpdate = (dataUpdate: object[]) => { | ||
|
|
@@ -63,7 +62,6 @@ const useCursor = <T = object>( | |
| .then(() => { | ||
| // Unsubscribe will not run if the subscription failed since the promise was | ||
| // rejected. | ||
| console.log("Unsubscribing from cursor"); | ||
| cursor.unsubscribe(); | ||
| }) | ||
| .catch((error: unknown) => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -89,10 +89,17 @@ const handleQuerySubmit = (payload: QueryJobCreationSchema) => { | |||||||||
|
|
||||||||||
| submitQuery(payload) | ||||||||||
| .then((result) => { | ||||||||||
| store.updateSearchJobId(result.data.searchJobId); | ||||||||||
| store.updateAggregationJobId(result.data.aggregationJobId); | ||||||||||
| const {searchJobId, aggregationJobId} = result.data; | ||||||||||
| store.updateSearchJobId(searchJobId); | ||||||||||
| store.updateAggregationJobId(aggregationJobId); | ||||||||||
| store.updateSearchUiState(SEARCH_UI_STATE.QUERYING); | ||||||||||
| console.log("Query ID Returned", result); | ||||||||||
| console.debug( | ||||||||||
| "Search job created - ", | ||||||||||
| "Search job ID:", | ||||||||||
| searchJobId, | ||||||||||
| "Aggregation job ID:", | ||||||||||
| aggregationJobId | ||||||||||
| ); | ||||||||||
| }) | ||||||||||
| .catch((err: unknown) => { | ||||||||||
| console.error("Failed to submit query:", err); | ||||||||||
|
|
@@ -117,7 +124,7 @@ const handleQueryCancel = (payload: QueryJobSchema) => { | |||||||||
| cancelQuery( | ||||||||||
| payload | ||||||||||
| ).then(() => { | ||||||||||
| console.log("Query cancelled successfully"); | ||||||||||
| console.debug("Query cancelled successfully"); | ||||||||||
| }) | ||||||||||
|
Comment on lines
+127
to
128
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||
| .catch((err: unknown) => { | ||||||||||
| console.error("Failed to cancel query:", err); | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,4 +1,4 @@ | ||||||||||||||||||||||
| import MongoCollectionSocket from "../../../../api/socket/MongoCollectionSocket"; | ||||||||||||||||||||||
| import MongoSocketCollection from "../../../../api/socket/MongoSocketCollection"; | ||||||||||||||||||||||
| import {useCursor} from "../../../../api/socket/useCursor"; | ||||||||||||||||||||||
| import {TimelineBucket} from "../../../../components/ResultsTimeline/typings"; | ||||||||||||||||||||||
| import useSearchStore, {SEARCH_STATE_DEFAULT} from "../../SearchState/index"; | ||||||||||||||||||||||
|
|
@@ -20,7 +20,11 @@ const useAggregationResults = () => { | |||||||||||||||||||||
| return null; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const collection = new MongoCollectionSocket(aggregationJobId.toString()); | ||||||||||||||||||||||
| console.log( | ||||||||||||||||||||||
| `Subscribing to updates to aggregation results with job ID: ${aggregationJobId}` | ||||||||||||||||||||||
| ); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const collection = new MongoSocketCollection(aggregationJobId.toString()); | ||||||||||||||||||||||
|
Comment on lines
+23
to
+27
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Use All other updated modules migrated from - console.log(
+ console.debug(📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
| return collection.find({}, {}); | ||||||||||||||||||||||
| }, | ||||||||||||||||||||||
| [aggregationJobId] | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,6 @@ | ||||||||||||||||||||||||||||||||||||||
| import {SearchResultsMetadataDocument} from "@common/index.js"; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| import MongoCollectionSocket from "../../../api/socket/MongoCollectionSocket"; | ||||||||||||||||||||||||||||||||||||||
| import MongoSocketCollection from "../../../api/socket/MongoSocketCollection"; | ||||||||||||||||||||||||||||||||||||||
| import {useCursor} from "../../../api/socket/useCursor"; | ||||||||||||||||||||||||||||||||||||||
| import {settings} from "../../../settings"; | ||||||||||||||||||||||||||||||||||||||
| import useSearchStore, {SEARCH_STATE_DEFAULT} from "./index"; | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -22,10 +22,14 @@ const useResultsMetadata = () => { | |||||||||||||||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const collection = new MongoCollectionSocket( | ||||||||||||||||||||||||||||||||||||||
| 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}); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+25
to
33
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Switch to Same rationale as in - console.log(
+ console.debug(
`Subscribing to updates for results metadata for search job ID: ${searchJobId}`
);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @davemarco Understood—keeping the |
||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||
| [searchJobId] | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -201,7 +201,7 @@ class MongoSocketIoServer { | |||||||||||
| let watcherCollection = this.#collections.get(collectionName); | ||||||||||||
| if ("undefined" === typeof watcherCollection) { | ||||||||||||
| watcherCollection = new MongoWatcherCollection(collectionName, this.#mongoDb); | ||||||||||||
| this.#fastify.log.info(`Initialize Mongo watcher collection:${collectionName}.`); | ||||||||||||
| this.#fastify.log.debug(`Initialize Mongo watcher collection:${collectionName}.`); | ||||||||||||
| this.#collections.set(collectionName, watcherCollection); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
|
|
@@ -226,7 +226,7 @@ class MongoSocketIoServer { | |||||||||||
| ): Promise<void> { | ||||||||||||
| const {collectionName, query, options} = requestArgs; | ||||||||||||
|
|
||||||||||||
| this.#fastify.log.info( | ||||||||||||
| this.#fastify.log.debug( | ||||||||||||
| `Socket:${socket.id} requested query:${JSON.stringify(query)} ` + | ||||||||||||
| `with options:${JSON.stringify(options)} to collection:${collectionName}` | ||||||||||||
| ); | ||||||||||||
|
|
@@ -252,7 +252,11 @@ class MongoSocketIoServer { | |||||||||||
| callback({data: {queryId, initialDocuments}}); | ||||||||||||
|
|
||||||||||||
| this.#addQueryIdToSubscribedList(queryId, socket.id); | ||||||||||||
| this.#fastify.log.info(`Socket:${socket.id} subscribed to queryID:${queryId}.`); | ||||||||||||
| this.#fastify.log.info( | ||||||||||||
| `Socket:${socket.id} subscribed to query:${JSON.stringify(query)} ` + | ||||||||||||
| `with options:${JSON.stringify(options)} ` + | ||||||||||||
| `on collection:${collectionName} with ID:${queryId}` | ||||||||||||
| ); | ||||||||||||
|
Comment on lines
+255
to
+259
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Consider downgrading this final “subscribed” message to 🤖 Prompt for AI Agents |
||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
|
|
@@ -288,7 +292,7 @@ class MongoSocketIoServer { | |||||||||||
| #unsubscribe (socket: MongoCustomSocket, queryId: number) { | ||||||||||||
| const queryHash: string | undefined = this.#queryIdToQueryHashMap.get(queryId); | ||||||||||||
| if ("undefined" === typeof queryHash) { | ||||||||||||
| this.#fastify.log.error(`QueryId ${queryId} not found in query map`); | ||||||||||||
| this.#fastify.log.error(`Query:${queryId} not found in query map`); | ||||||||||||
|
|
||||||||||||
| return; | ||||||||||||
| } | ||||||||||||
|
|
@@ -303,10 +307,10 @@ class MongoSocketIoServer { | |||||||||||
| } | ||||||||||||
|
|
||||||||||||
| const isLastSubscriber = collection.unsubscribe(queryId, socket.id); | ||||||||||||
| this.#fastify.log.info(`Socket ${socket.id} unsubscribed from query ${queryId}`); | ||||||||||||
| this.#fastify.log.info(`Socket:${socket.id} unsubscribed from query:${queryId}`); | ||||||||||||
|
|
||||||||||||
| if (isLastSubscriber) { | ||||||||||||
| this.#fastify.log.info(`QueryID:${queryId} deleted from query map.`); | ||||||||||||
| this.#fastify.log.debug(`Query:${queryId} deleted from query map.`); | ||||||||||||
| this.#queryIdToQueryHashMap.delete(queryId); | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
|
|
@@ -316,7 +320,7 @@ class MongoSocketIoServer { | |||||||||||
| ); | ||||||||||||
|
|
||||||||||||
| if (false === collection.isReferenced()) { | ||||||||||||
| this.#fastify.log.info(`Collection:${queryParams.collectionName}` + | ||||||||||||
| this.#fastify.log.debug(`Collection:${queryParams.collectionName}` + | ||||||||||||
| " deallocated from server."); | ||||||||||||
|
Comment on lines
+323
to
324
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 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
Suggested change
🧰 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 |
||||||||||||
| this.#collections.delete(queryParams.collectionName); | ||||||||||||
| } | ||||||||||||
|
|
@@ -334,8 +338,8 @@ class MongoSocketIoServer { | |||||||||||
| requestArgs: {queryId: number} | ||||||||||||
| ): Promise<void> { | ||||||||||||
| const {queryId} = requestArgs; | ||||||||||||
| this.#fastify.log.info( | ||||||||||||
| `Socket:${socket.id} requested unsubscription to QueryId:${queryId}` | ||||||||||||
| this.#fastify.log.debug( | ||||||||||||
| `Socket:${socket.id} requested unsubscription to query:${queryId}` | ||||||||||||
| ); | ||||||||||||
|
|
||||||||||||
| const subscribedQueryIds = this.#subscribedQueryIdsMap.get(socket.id); | ||||||||||||
|
|
||||||||||||
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)
Minor readability tweak
Add a space after the colon to align with the other log messages.
📝 Committable suggestion
🤖 Prompt for AI Agents