Skip to content
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

Code cleanup, logging #485

Merged
merged 11 commits into from
Dec 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/handlers/pathHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' };
}

Expand All @@ -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}`,
};
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/install/installationValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.`
Expand Down
6 changes: 3 additions & 3 deletions src/main-process/appWindow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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');

Expand Down
44 changes: 26 additions & 18 deletions src/main-process/comfyDesktopApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);

Expand Down
4 changes: 3 additions & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
19 changes: 15 additions & 4 deletions src/models/DownloadManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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)
);
}
}
7 changes: 2 additions & 5 deletions src/preload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
/**
Expand Down
4 changes: 2 additions & 2 deletions src/services/sentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions src/virtualEnvironment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());
});
Expand Down
Loading