From ad782455f2e0a22b8d8c4fb4ad69a687384e19a6 Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Fri, 15 Nov 2024 05:37:03 -0800 Subject: [PATCH 1/2] fix(logger): support module-scope getLogger() Problem: Assigning a `const logger = getLogger()` in module scope, may default to `ConsoleLogger`, because modules may be loaded before logging is initialized. Solution: Move the decision into `TopicLogger`, so that the time of the `getLogger()` assignment is irrelevant (just-in-time). --- packages/core/src/shared/fs/watchedFiles.ts | 4 +-- packages/core/src/shared/logger/logger.ts | 34 ++++++++++++--------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/packages/core/src/shared/fs/watchedFiles.ts b/packages/core/src/shared/fs/watchedFiles.ts index 67d66e1bfca..5e10ea8a358 100644 --- a/packages/core/src/shared/fs/watchedFiles.ts +++ b/packages/core/src/shared/fs/watchedFiles.ts @@ -220,7 +220,7 @@ export abstract class WatchedFiles implements vscode.Disposable { item: item, } } else { - getLogger().info(`${this.name}: failed to process: ${uri}`) + getLogger().debug(`${this.name}: failed to process: ${uri}`) // if value isn't valid for type, remove from registry this.registryData.delete(pathAsString) } @@ -228,7 +228,7 @@ export abstract class WatchedFiles implements vscode.Disposable { if (!quiet) { throw e } - getLogger().info(`${this.name}: failed to process: ${uri}: ${(e as Error).message}`) + getLogger().error(`${this.name}: failed to process: ${uri}: ${(e as Error).message}`) } return undefined } diff --git a/packages/core/src/shared/logger/logger.ts b/packages/core/src/shared/logger/logger.ts index bfead668bc9..b5a99e543b9 100644 --- a/packages/core/src/shared/logger/logger.ts +++ b/packages/core/src/shared/logger/logger.ts @@ -5,7 +5,7 @@ import * as vscode from 'vscode' -export type LogTopic = 'crashReport' | 'notifications' | 'test' | 'unknown' +export type LogTopic = 'crashReport' | 'dev/beta' | 'notifications' | 'test' | 'unknown' class ErrorLog { constructor( @@ -143,24 +143,18 @@ function prependTopic(topic: string, message: string | Error): string | ErrorLog } /** - * Gets the logger if it has been initialized - * the logger is of `'main'` or `undefined`: Main logger; default impl: logs to log file and log output channel + * Gets the global default logger. + * * @param topic: topic to be appended in front of the message. */ export function getLogger(topic?: LogTopic): Logger { - const logger = toolkitLoggers['main'] - if (!logger) { - return new ConsoleLogger() - } - return new TopicLogger(topic ?? 'unknown', logger) + // `TopicLogger` will lazy-load the "main" logger when it becomes available. + return new TopicLogger(topic ?? 'unknown', 'main') } export function getDebugConsoleLogger(topic?: LogTopic): Logger { - const logger = toolkitLoggers['debugConsole'] - if (!logger) { - return new ConsoleLogger() - } - return new TopicLogger(topic ?? 'unknown', logger) + // `TopicLogger` will lazy-load the "debugConsole" logger when it becomes available. + return new TopicLogger(topic ?? 'unknown', 'debugConsole') } // jscpd:ignore-start @@ -215,15 +209,25 @@ export class ConsoleLogger extends BaseLogger { } /** - * Wraps a `ToolkitLogger` and defers to it for everything except `topic`. + * Wraps the specified `ToolkitLogger` and defers to it for everything except `topic`. + * + * Falls back to `ConsoleLogger` when the logger isn't available yet (during startup). */ export class TopicLogger extends BaseLogger implements vscode.Disposable { + // HACK: crude form of "lazy initialization", to support module-scope assignment of + // `getLogger()` without being sensitive to module-load ordering. So even if logging isn't ready + // at the time of the `getLogger` call, it will recover later. (This is a bit hacky, because it + // arguably doesn't belong in `TopicLogger`.) + public get logger() { + return toolkitLoggers[this.loggerKey] ?? new ConsoleLogger() + } + /** * Wraps a `ToolkitLogger` and defers to it for everything except `topic`. */ public constructor( public override topic: LogTopic, - public readonly logger: Logger + public readonly loggerKey: keyof typeof toolkitLoggers ) { super() } From a527f965b2723de58e659be9dd43185eb4bdcb09 Mon Sep 17 00:00:00 2001 From: "Justin M. Keyes" Date: Fri, 15 Nov 2024 05:53:44 -0800 Subject: [PATCH 2/2] feat(beta): improve beta URL logging --- packages/core/src/dev/beta.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/core/src/dev/beta.ts b/packages/core/src/dev/beta.ts index 7718d1f49fc..fbcf3bf4748 100644 --- a/packages/core/src/dev/beta.ts +++ b/packages/core/src/dev/beta.ts @@ -21,8 +21,10 @@ import { CancellationError } from '../shared/utilities/timeoutUtils' import { isAmazonQ, isCloud9, productName } from '../shared/extensionUtilities' import * as config from './config' import { isReleaseVersion } from '../shared/vscode/env' +import { getRelativeDate } from '../shared/datetime' const localize = nls.loadMessageBundle() +const logger = getLogger('dev/beta') const downloadIntervalMs = 1000 * 60 * 60 * 24 // A day in milliseconds @@ -57,12 +59,13 @@ export async function activate(ctx: vscode.ExtensionContext) { * If this is the first time we are watching the beta version or if its been 24 hours since it was last checked then try to prompt for update */ export function watchBetaVSIX(vsixUrl: string): vscode.Disposable { - getLogger().info(`dev: watching ${vsixUrl} for beta artifacts`) - const toolkit = getBetaToolkitData(vsixUrl) + const lastCheckRel = toolkit ? getRelativeDate(new Date(toolkit.lastCheck)) : '' + logger.info('watching beta artifacts url (lastCheck: %s): %s', lastCheckRel, vsixUrl) + if (!toolkit || toolkit.needUpdate || Date.now() - toolkit.lastCheck > downloadIntervalMs) { runAutoUpdate(vsixUrl).catch((e) => { - getLogger().error('runAutoUpdate failed: %s', (e as Error).message) + logger.error('runAutoUpdate failed: %s', (e as Error).message) }) } @@ -71,13 +74,13 @@ export function watchBetaVSIX(vsixUrl: string): vscode.Disposable { } async function runAutoUpdate(vsixUrl: string) { - getLogger().debug(`dev: checking ${vsixUrl} for a new version`) + logger.debug(`checking url for a new version: %s`, vsixUrl) try { await telemetry.aws_autoUpdateBeta.run(() => checkBetaUrl(vsixUrl)) } catch (e) { if (!isUserCancelledError(e)) { - getLogger().warn(`dev: beta extension auto-update failed: %s`, e) + logger.warn('beta extension auto-update failed: %s', e) } } } @@ -165,7 +168,7 @@ async function promptInstallToolkit(pluginPath: vscode.Uri, newVersion: string, switch (response) { case installBtn: try { - getLogger().info(`dev: installing artifact ${vsixName}`) + logger.info(`installing artifact: ${vsixName}`) await vscode.commands.executeCommand('workbench.extensions.installExtension', pluginPath) await updateBetaToolkitData(vsixUrl, { lastCheck: Date.now(),