From 8d6ebc1e8d1144a05cdcedaa880261940699c130 Mon Sep 17 00:00:00 2001 From: filtered <176114999+webfiltered@users.noreply.github.com> Date: Sun, 15 Dec 2024 06:45:34 +1100 Subject: [PATCH] Code cleanup, logging (#485) * Remove unused vars * Prefer unknown over explicit any * Add TS types * nit * nit * Prefer const over let * Add TS types * Remove unnecessary type assertion * nit * Add TS type * Ensure app startup errors are logged in the main log --- src/handlers/pathHandlers.ts | 4 +-- src/install/installationValidator.ts | 4 +-- src/main-process/appWindow.ts | 6 ++-- src/main-process/comfyDesktopApp.ts | 44 ++++++++++++++++------------ src/main.ts | 4 ++- src/models/DownloadManager.ts | 19 +++++++++--- src/preload.ts | 7 ++--- src/services/sentry.ts | 4 +-- src/virtualEnvironment.ts | 4 +-- 9 files changed, 57 insertions(+), 39 deletions(-) diff --git a/src/handlers/pathHandlers.ts b/src/handlers/pathHandlers.ts index b86bbb76..fe16f52f 100644 --- a/src/handlers/pathHandlers.ts +++ b/src/handlers/pathHandlers.ts @@ -57,7 +57,7 @@ export class PathHandlers { // Check if path is writable try { fs.accessSync(inputPath, fs.constants.W_OK); - } catch (err) { + } catch { return { isValid: false, error: 'Path is not writable' }; } @@ -76,7 +76,7 @@ export class PathHandlers { log.error('Error validating install path:', error); return { isValid: false, - error: 'Failed to validate install path: ' + error, + error: `Failed to validate install path: ${error}`, }; } } diff --git a/src/install/installationValidator.ts b/src/install/installationValidator.ts index 8ffc032f..2bf7ed5d 100644 --- a/src/install/installationValidator.ts +++ b/src/install/installationValidator.ts @@ -33,14 +33,14 @@ export class InstallationValidator { log.debug(`Attempting to open containing directory: ${parsed.dir}`); await fs.access(file); shell.showItemInFolder(file); - } catch (error) { + } catch { log.warn(`Could not access file whilst attempting to exit gracefully after a critical error.`, file); try { // Failed - try the parent dir const parsed = path.parse(file); await fs.access(parsed.dir); await shell.openPath(parsed.dir); - } catch (error) { + } catch { // Nothing works. Log, notify, quit. log.error( `Could not read directory containing file, whilst attempting to exit gracefully after a critical error.` diff --git a/src/main-process/appWindow.ts b/src/main-process/appWindow.ts index 2094a436..3304f0fa 100644 --- a/src/main-process/appWindow.ts +++ b/src/main-process/appWindow.ts @@ -16,7 +16,7 @@ export class AppWindow { private window: BrowserWindow; /** Volatile store containing window config - saves window state between launches. */ private store: Store; - private messageQueue: Array<{ channel: string; data: any }> = []; + private messageQueue: Array<{ channel: string; data: unknown }> = []; private rendererReady: boolean = false; /** The application menu. */ private menu: Electron.Menu | null; @@ -69,7 +69,7 @@ export class AppWindow { return this.rendererReady; } - public send(channel: string, data: any): void { + public send(channel: string, data: unknown): void { if (!this.isReady()) { this.messageQueue.push({ channel, data }); return; @@ -248,7 +248,7 @@ export class AppWindow { 'UI', process.platform === 'darwin' ? 'Comfy_Logo_x16_BW.png' : 'Comfy_Logo_x32.png' ); - let tray = new Tray(trayImage); + const tray = new Tray(trayImage); tray.setToolTip('ComfyUI'); diff --git a/src/main-process/comfyDesktopApp.ts b/src/main-process/comfyDesktopApp.ts index ce4b056d..d20c0d90 100644 --- a/src/main-process/comfyDesktopApp.ts +++ b/src/main-process/comfyDesktopApp.ts @@ -64,7 +64,7 @@ export class ComfyDesktopApp { this.terminal?.resize(cols, rows); }); - ipcMain.handle(IPC_CHANNELS.TERMINAL_RESTORE, (_event) => { + ipcMain.handle(IPC_CHANNELS.TERMINAL_RESTORE, () => { return this.terminal?.restore(); }); } @@ -83,7 +83,7 @@ export class ComfyDesktopApp { })); // Combine all GPU info into a single object - const allGpuInfo = Object.assign({}, ...gpuInfo); + const allGpuInfo = { ...gpuInfo }; // Set Sentry context with all GPU information Sentry.setContext('gpus', allGpuInfo); } catch (e) { @@ -120,26 +120,34 @@ export class ComfyDesktopApp { log.info('Reinstalling...'); this.reinstall(); }); - ipcMain.handle(IPC_CHANNELS.SEND_ERROR_TO_SENTRY, async (_event, { error, extras }): Promise => { - try { - return Sentry.captureMessage(error, { - level: 'error', - extra: { ...extras, comfyUIExecutionError: true }, - tags: { - comfyorigin: 'core', - }, - }); - } catch (err) { - log.error('Failed to send error to Sentry:', err); - return null; + type SentryErrorDetail = { + error: string; + extras?: Record; + }; + + ipcMain.handle( + IPC_CHANNELS.SEND_ERROR_TO_SENTRY, + async (_event, { error, extras }: SentryErrorDetail): Promise => { + try { + return Sentry.captureMessage(error, { + level: 'error', + extra: { ...extras, comfyUIExecutionError: true }, + tags: { + comfyorigin: 'core', + }, + }); + } catch (err) { + log.error('Failed to send error to Sentry:', err); + return null; + } } - }); + ); // Config - ipcMain.handle(IPC_CHANNELS.GET_GPU, async (_event): Promise => { + ipcMain.handle(IPC_CHANNELS.GET_GPU, async (): Promise => { return await useDesktopConfig().getAsync('detectedGpu'); }); // Restart core - ipcMain.handle(IPC_CHANNELS.RESTART_CORE, async (_event): Promise => { + ipcMain.handle(IPC_CHANNELS.RESTART_CORE, async (): Promise => { if (!this.comfyServer) return false; await this.comfyServer?.kill(); await this.comfyServer.start(); @@ -200,7 +208,7 @@ export class ComfyDesktopApp { log.info('Server start'); await this.appWindow.loadRenderer('server-start'); - DownloadManager.getInstance(this.appWindow!, getModelsDirectory(this.basePath)); + DownloadManager.getInstance(this.appWindow, getModelsDirectory(this.basePath)); this.appWindow.sendServerStartProgress(ProgressStatus.PYTHON_SETUP); diff --git a/src/main.ts b/src/main.ts index 31b1a71b..f671f814 100644 --- a/src/main.ts +++ b/src/main.ts @@ -53,6 +53,7 @@ async function startApp() { const store = await DesktopConfig.load(shell); if (!store) throw new Error('Unknown error loading app config on startup.'); } catch (error) { + log.error('Unhandled exception during config load', error); dialog.showErrorBox('User Data', `Unknown error whilst writing to user data folder:\n\n${error}`); app.exit(20); return; @@ -99,11 +100,12 @@ async function startApp() { appWindow.sendServerStartProgress(ProgressStatus.READY); await appWindow.loadComfyUI({ host, port, extraServerArgs }); } catch (error) { + log.error('Unhandled exception during app startup', error); appWindow.sendServerStartProgress(ProgressStatus.ERROR); appWindow.send(IPC_CHANNELS.LOG_MESSAGE, error); } } catch (error) { - log.error('Fatal error occurred during app startup.', error); + log.error('Fatal error occurred during app pre-startup.', error); app.exit(2024); } } diff --git a/src/models/DownloadManager.ts b/src/models/DownloadManager.ts index 88badcf5..bb236dc7 100644 --- a/src/models/DownloadManager.ts +++ b/src/models/DownloadManager.ts @@ -36,7 +36,7 @@ export class DownloadManager { this.mainWindow = mainWindow; this.modelsDirectory = modelsDirectory; - session.defaultSession.on('will-download', (event, item, webContents) => { + session.defaultSession.on('will-download', (event, item) => { const url = item.getURLChain()[0]; // Get the original URL in case of redirects. log.info('Will-download event ', url); const download = this.downloads.get(url); @@ -320,13 +320,24 @@ export class DownloadManager { } private registerIpcHandlers() { - ipcMain.handle(IPC_CHANNELS.START_DOWNLOAD, (event, { url, path, filename }) => + interface FileAndPath { + filename: string; + path: string; + } + interface DownloadDetails extends FileAndPath { + url: string; + } + + ipcMain.handle(IPC_CHANNELS.START_DOWNLOAD, (event, { url, path, filename }: DownloadDetails) => this.startDownload(url, path, filename) ); ipcMain.handle(IPC_CHANNELS.PAUSE_DOWNLOAD, (event, url: string) => this.pauseDownload(url)); ipcMain.handle(IPC_CHANNELS.RESUME_DOWNLOAD, (event, url: string) => this.resumeDownload(url)); ipcMain.handle(IPC_CHANNELS.CANCEL_DOWNLOAD, (event, url: string) => this.cancelDownload(url)); - ipcMain.handle(IPC_CHANNELS.GET_ALL_DOWNLOADS, (event) => this.getAllDownloads()); - ipcMain.handle(IPC_CHANNELS.DELETE_MODEL, (event, { filename, path }) => this.deleteModel(filename, path)); + ipcMain.handle(IPC_CHANNELS.GET_ALL_DOWNLOADS, () => this.getAllDownloads()); + + ipcMain.handle(IPC_CHANNELS.DELETE_MODEL, (event, { filename, path }: FileAndPath) => + this.deleteModel(filename, path) + ); } } diff --git a/src/preload.ts b/src/preload.ts index 417c1b19..5d1b5591 100644 --- a/src/preload.ts +++ b/src/preload.ts @@ -149,11 +149,8 @@ const electronAPI = { * @param error The error object or message to send * @param extras Optional additional context/data to attach */ - sendErrorToSentry: (error: string, extras?: Record) => { - return ipcRenderer.invoke(IPC_CHANNELS.SEND_ERROR_TO_SENTRY, { - error: error, - extras, - }); + sendErrorToSentry: (error: string, extras?: Record) => { + return ipcRenderer.invoke(IPC_CHANNELS.SEND_ERROR_TO_SENTRY, { error, extras }); }, Terminal: { /** diff --git a/src/services/sentry.ts b/src/services/sentry.ts index 47140a5a..7f47f89a 100644 --- a/src/services/sentry.ts +++ b/src/services/sentry.ts @@ -56,12 +56,12 @@ class SentryLogging { try { const record = obj as Record; record[k] = this.filterEvent(record[k]); - } catch (error) { + } catch { // Failed to read/write key } } } - } catch (error) { + } catch { // Failed to enumerate keys } diff --git a/src/virtualEnvironment.ts b/src/virtualEnvironment.ts index b59933eb..a8b2a2af 100644 --- a/src/virtualEnvironment.ts +++ b/src/virtualEnvironment.ts @@ -288,12 +288,12 @@ export class VirtualEnvironment { }); if (callbacks) { - childProcess.stdout?.on('data', (data) => { + childProcess.stdout?.on('data', (data: Buffer) => { console.log(data.toString()); callbacks.onStdout?.(data.toString()); }); - childProcess.stderr?.on('data', (data) => { + childProcess.stderr?.on('data', (data: Buffer) => { console.log(data.toString()); callbacks.onStderr?.(data.toString()); });