update based on review changes

Signed-off-by: Charlie Drage <charlie@charliedrage.com>

packages/main/src/plugin/index.ts

@@ -1293,6 +1293,13 @@ export class PluginSystem {
       },
     );
 
+    this.ipcHandle(
+      'troubleshooting:generateLogFileName',
+      async (_listener, filename: string, prefix?: string): Promise<string> => {
+        return troubleshooting.generateLogFileName(filename, prefix);
+      },
+    );
+
     this.ipcHandle(
       'cli-tool-registry:updateCliTool',
       async (_listener, id: string, loggerId: string): Promise<void> => {

packages/main/src/plugin/troubleshooting.spec.ts

@@ -16,13 +16,19 @@
  ***********************************************************************/
 
 import { beforeEach, expect, test, vi } from 'vitest';
-import { Troubleshooting, generateLogFileName } from './troubleshooting.js';
-import type { FileMap, LogType } from './troubleshooting.js';
+import { Troubleshooting } from './troubleshooting.js';
+import type { TroubleshootingFileMap, LogType } from './troubleshooting.js';
 import * as fs from 'node:fs';
+import type { ApiSenderType } from './api.js';
 
 const writeZipMock = vi.fn();
 const addFileMock = vi.fn();
 
+const apiSender: ApiSenderType = {
+  send: vi.fn(),
+  receive: vi.fn(),
+};
+
 vi.mock('electron', () => {
   return {
     ipcMain: {
@@ -47,7 +53,7 @@ beforeEach(() => {
 
 // Test the saveLogsToZip function
 test('Should save a zip file with the correct content', async () => {
-  const zipFile = new Troubleshooting(undefined);
+  const zipFile = new Troubleshooting(apiSender);
   const fileMaps = [
     {
       fileName: 'file1',
@@ -69,8 +75,8 @@ test('Should save a zip file with the correct content', async () => {
 
 // Do not expect writeZipMock to have been called if fileMaps is empty
 test('Should not save a zip file if fileMaps is empty', async () => {
-  const zipFile = new Troubleshooting(undefined);
-  const fileMaps: FileMap[] = [];
+  const zipFile = new Troubleshooting(apiSender);
+  const fileMaps: TroubleshootingFileMap[] = [];
 
   const zipSpy = vi.spyOn(zipFile, 'saveLogsToZip');
 
@@ -82,15 +88,15 @@ test('Should not save a zip file if fileMaps is empty', async () => {
 
 // Expect the file name to have a .txt extension
 test('Should have a .txt extension in the file name', async () => {
-  const zipFile = new Troubleshooting(undefined);
+  const zipFile = new Troubleshooting(apiSender);
   const fileMaps = [
     {
       fileName: 'file1',
-      content: 'content1',
+      content: '',
     },
     {
       fileName: 'file2',
-      content: 'content2',
+      content: '',
     },
   ];
 
@@ -99,13 +105,13 @@ test('Should have a .txt extension in the file name', async () => {
   await zipFile.saveLogsToZip(fileMaps, 'test.zip');
   expect(zipSpy).toHaveBeenCalledWith(fileMaps, 'test.zip');
 
-  expect(addFileMock).toHaveBeenCalledWith('file1.txt', Buffer.from('content1', 'utf8'));
-  expect(addFileMock).toHaveBeenCalledWith('file2.txt', Buffer.from('content2', 'utf8'));
+  expect(addFileMock).toHaveBeenCalledWith('file1', expect.any(Object));
+  expect(addFileMock).toHaveBeenCalledWith('file2', expect.any(Object));
 });
 
 // Expect getConsoleLogs to correctly format the console logs passed in
 test('Should correctly format console logs', async () => {
-  const zipFile = new Troubleshooting(undefined);
+  const zipFile = new Troubleshooting(apiSender);
   const consoleLogs = [
     {
       logType: 'log' as LogType,
@@ -144,7 +150,7 @@ test('Should return getMacSystemLogs if the platform is darwin', async () => {
     return true;
   });
 
-  const zipFile = new Troubleshooting(undefined);
+  const zipFile = new Troubleshooting(apiSender);
   const getSystemLogsSpy = vi.spyOn(zipFile, 'getSystemLogs');
 
   await zipFile.getSystemLogs();
@@ -155,12 +161,12 @@ test('Should return getMacSystemLogs if the platform is darwin', async () => {
 
   // Expect readFileMock to have been called with /Library/Logs/Podman Desktop/launchd-stdout.log but CONTAINED in the path
   expect(readFileMock).toHaveBeenCalledWith(
-    expect.stringContaining('/Library/Logs/Podman Desktop/launchd-stdout.log'),
-    'utf8',
+    expect.stringContaining('/Library/Logs/Podman Desktop/launchd-stdout'),
+    'utf-8',
   );
   expect(readFileMock).toHaveBeenCalledWith(
-    expect.stringContaining('/Library/Logs/Podman Desktop/launchd-stderr.log'),
-    'utf8',
+    expect.stringContaining('/Library/Logs/Podman Desktop/launchd-stderr'),
+    'utf-8',
   );
 });
 
@@ -179,7 +185,7 @@ test('Should return getWindowsSystemLogs if the platform is win32', async () =>
   const readFileMock = vi.spyOn(fs.promises, 'readFile');
   readFileMock.mockResolvedValue('content');
 
-  const zipFile = new Troubleshooting(undefined);
+  const zipFile = new Troubleshooting(apiSender);
   const getSystemLogsSpy = vi.spyOn(zipFile, 'getSystemLogs');
 
   await zipFile.getSystemLogs();
@@ -190,13 +196,14 @@ test('Should return getWindowsSystemLogs if the platform is win32', async () =>
 
   // Expect readFileMock to have been called with ~/AppData/Roaming/Podman Desktop/logs/podman-desktop.log but CONTAINED in the path
   expect(readFileMock).toHaveBeenCalledWith(
-    expect.stringContaining('/AppData/Roaming/Podman Desktop/logs/podman-desktop.log'),
-    'utf8',
+    expect.stringContaining('/AppData/Roaming/Podman Desktop/logs/podman-desktop'),
+    'utf-8',
   );
 });
 
 test('test generateLogFileName', async () => {
-  const fileName = generateLogFileName('test');
+  const ts = new Troubleshooting(apiSender);
+  const fileName = ts.generateLogFileName('test');
 
   // Simple regex to check that the file name is in the correct format (YYYMMDDHHmmss)
   expect(fileName).toMatch(/[0-9]{14}/);

packages/main/src/plugin/troubleshooting.ts

@@ -20,18 +20,19 @@ import AdmZip from 'adm-zip';
 import moment from 'moment';
 import * as os from 'node:os';
 import * as fs from 'node:fs';
+import type { ApiSenderType } from './api.js';
 
 const SYSTEM_FILENAME = 'system';
 
-export interface FileMap {
+export interface TroubleshootingFileMap {
   fileName: string;
   content: string;
 }
 
 export type LogType = 'log' | 'warn' | 'trace' | 'debug' | 'error';
 
 export class Troubleshooting {
-  constructor(private apiSender: any) {}
+  constructor(private apiSender: ApiSenderType) {}
 
   // The "main" function that is exposes that is used to gather
   // all the logs and save them to a zip file.
@@ -44,8 +45,10 @@ export class Troubleshooting {
     return fileMaps.map(fileMap => fileMap.fileName);
   }
 
-  async saveLogsToZip(fileMaps: FileMap[], destination: string): Promise<void> {
-    if (fileMaps.length === 0) return;
+  async saveLogsToZip(fileMaps: TroubleshootingFileMap[], destination: string): Promise<void> {
+    if (fileMaps.length === 0) {
+      return;
+    }
 
     const zip = new AdmZip();
     fileMaps.forEach(fileMap => {
@@ -54,12 +57,12 @@ export class Troubleshooting {
     zip.writeZip(destination);
   }
 
-  getConsoleLogs(consoleLogs: { logType: LogType; message: string }[]): FileMap[] {
+  getConsoleLogs(consoleLogs: { logType: LogType; message: string }[]): TroubleshootingFileMap[] {
     const content = consoleLogs.map(log => `${log.logType} : ${log.message}`).join('\n');
-    return [{ fileName: generateLogFileName('console'), content }];
+    return [{ fileName: this.generateLogFileName('console'), content }];
   }
 
-  async getSystemLogs(): Promise<FileMap[]> {
+  async getSystemLogs(): Promise<TroubleshootingFileMap[]> {
     switch (os.platform()) {
       case 'darwin':
         return this.getLogsFromFiles(
@@ -69,23 +72,18 @@ export class Troubleshooting {
       case 'win32':
         return this.getLogsFromFiles(['podman-desktop'], `${os.homedir()}/AppData/Roaming/Podman Desktop/logs`);
       default:
-        console.log('Unsupported platform for retrieving system logs');
+        // Unsupported platform, so do not return anything
         return [];
     }
   }
 
-  private async getFileSystemContent(filePath: string, logName: string): Promise<FileMap> {
-    try {
-      const content = await fs.promises.readFile(filePath, 'utf-8');
-      return { fileName: generateLogFileName(SYSTEM_FILENAME + '-' + logName), content };
-    } catch (error) {
-      console.error(`Error reading file from ${filePath}: `, error);
-      throw error;
-    }
+  private async getFileSystemContent(filePath: string, logName: string): Promise<TroubleshootingFileMap> {
+    const content = await fs.promises.readFile(filePath, 'utf-8');
+    return { fileName: this.generateLogFileName(SYSTEM_FILENAME + '-' + logName), content };
   }
 
-  private async getLogsFromFiles(logFiles: string[], logDir: string): Promise<FileMap[]> {
-    const logs: FileMap[] = [];
+  private async getLogsFromFiles(logFiles: string[], logDir: string): Promise<TroubleshootingFileMap[]> {
+    const logs: TroubleshootingFileMap[] = [];
     for (const file of logFiles) {
       try {
         const filePath = `${logDir}/${file}`;
@@ -103,13 +101,12 @@ export class Troubleshooting {
     }
     return logs;
   }
-}
-
-export function generateLogFileName(filename: string, extension?: string): string {
-  // If the filename has an extension like .log, move it to the end ofthe generated name
-  // otherwise just use .txt
-  const filenameParts = filename.split('.');
-  // Use the file extension if it's provided, otherwise use the one from the file name, or default to txt
-  const fileExtension = extension ? extension : filenameParts.length > 1 ? filenameParts[1] : 'txt';
-  return `${filenameParts[0]}-${moment().format('YYYYMMDDHHmmss')}.${fileExtension}`;
+  generateLogFileName(filename: string, extension?: string): string {
+    // If the filename has an extension like .log, move it to the end ofthe generated name
+    // otherwise just use .txt
+    const filenameParts = filename.split('.');
+    // Use the file extension if it's provided, otherwise use the one from the file name, or default to txt
+    const fileExtension = extension ? extension : filenameParts.length > 1 ? filenameParts[1] : 'txt';
+    return `${filenameParts[0]}-${moment().format('YYYYMMDDHHmmss')}.${fileExtension}`;
+  }
 }

packages/preload/src/index.ts

@@ -1084,6 +1084,13 @@ function initExposure(): void {
     return ipcInvoke('troubleshooting:saveLogs', memoryLogs, destinaton);
   });
 
+  contextBridge.exposeInMainWorld(
+    'troubleshootingGenerateLogFileName',
+    async (filename: string, extension?: string): Promise<string> => {
+      return ipcInvoke('troubleshooting:generateLogFileName', filename, extension);
+    },
+  );
+
   contextBridge.exposeInMainWorld('getContributedMenus', async (context: string): Promise<Menu[]> => {
     return ipcInvoke('menu-registry:getContributedMenus', context);
   });

packages/renderer/src/lib/troubleshooting/TroubleshootingGatherLogs.svelte

@@ -2,15 +2,15 @@
 import { faFileLines, faScroll } from '@fortawesome/free-solid-svg-icons';
 import Button from '/@/lib/ui/Button.svelte';
 import Fa from 'svelte-fa';
-import { generateLogFileName } from '../../../../main/src/plugin/troubleshooting';
 
 let logs: string[] = [];
 
 // Save files as a zip file (we first ask the user for the dialog, and then save the files to the filepath)
 async function saveLogsAsZip() {
-  const result = await window.saveFileDialog('Save Logs as .zip', generateLogFileName('podman-desktop', 'zip'));
+  const filename = await window.troubleshootingGenerateLogFileName('podman-desktop', 'zip');
+  const result = await window.saveFileDialog('Save Logs as .zip', filename);
   if (!result.canceled && result.filePath) {
-logs = await window.troubleshootingSaveLogs(result.filePath);
+    logs = await window.troubleshootingSaveLogs(result.filePath);
   }
 }
 </script>