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
Comfy-Org · Dec 14, 2024 · 8d6ebc1 · 8d6ebc1
1 parent 34f8457
commit 8d6ebc1
Show file tree

Hide file tree

Showing 9 changed files with 57 additions and 39 deletions.
diff --git 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
@@ -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
@@ -16,7 +16,7 @@ export class AppWindow {
   private window: BrowserWindow;
   /** Volatile store containing window config - saves window state between launches. */
   private store: Store<AppWindowSettings>;
-  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
@@ -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<string | null> => {
-      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<string, unknown>;
+    };
+
+    ipcMain.handle(
+      IPC_CHANNELS.SEND_ERROR_TO_SENTRY,
+      async (_event, { error, extras }: SentryErrorDetail): Promise<string | null> => {
+        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<TorchDeviceType | undefined> => {
+    ipcMain.handle(IPC_CHANNELS.GET_GPU, async (): Promise<TorchDeviceType | undefined> => {
       return await useDesktopConfig().getAsync('detectedGpu');
     });
     // Restart core
-    ipcMain.handle(IPC_CHANNELS.RESTART_CORE, async (_event): Promise<boolean> => {
+    ipcMain.handle(IPC_CHANNELS.RESTART_CORE, async (): Promise<boolean> => {
       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
@@ -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
@@ -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
@@ -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<string, any>) => {
-    return ipcRenderer.invoke(IPC_CHANNELS.SEND_ERROR_TO_SENTRY, {
-      error: error,
-      extras,
-    });
+  sendErrorToSentry: (error: string, extras?: Record<string, unknown>) => {
+    return ipcRenderer.invoke(IPC_CHANNELS.SEND_ERROR_TO_SENTRY, { error, extras });
   },
   Terminal: {
     /**

diff --git a/src/services/sentry.ts b/src/services/sentry.ts
@@ -56,12 +56,12 @@ class SentryLogging {
           try {
             const record = obj as Record<string, unknown>;
             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
@@ -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());
       });