refactor: split loggers #111
Conversation
src/logger.ts
Outdated
| super(); | ||
| } | ||
|
|
||
| async initialize(): Promise<void> { |
There was a problem hiding this comment.
Rather than having an async instance method for initializing the logger, wouldn't it be cleaner to turn it into an async factori that calls a private constructor? That way we won't have ambiguous optionality for the logWriter.
Something like:
class DiskLogger extends LoggerBase {
private constructor(private logWriter: MongoLogWriter) {
super();
}
public static async create(logPath: string) {
...
const logWriter = await manager.createLogWriter();
return new DiskLogger(logWriter);
}
}
Pull Request Test Coverage Report for Build 14660881741Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
src/logger.ts
Outdated
| export default logger; | ||
|
|
||
| export async function initializeLogger(server: McpServer, logPath: string): Promise<void> { | ||
| logger.setLoggers(new McpLogger(server), new DiskLogger(logPath)); |
There was a problem hiding this comment.
combined with Nikola's comment, I'd move the logger from global variable into this scope and return it; that way the logger variable will only exist in a valid, initalized state similar to https://github.com/mongodb-js/mongosh/blob/0ad691f4878dc52a8782ecaf66fc46928b2613f4/packages/logging/src/logging-and-telemetry.ts#L59-L67
There was a problem hiding this comment.
I wanted to do this originally, but it became a bigger change, leaving for another PR
4 participants
No description provided.