fix: Add new extension errored state (#2424)

* fix: Add new extension failed state

Fixes #2209

Signed-off-by: Jeff MAURY <jmaury@redhat.com>
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
Co-authored-by: Florent Benoit <fbenoit@redhat.com>

packages/main/src/plugin/api/extension-info.ts

@@ -16,6 +16,11 @@
  * SPDX-License-Identifier: Apache-2.0
  ***********************************************************************/
 
+export interface ExtensionError {
+  message: string;
+  stack?: string;
+}
+
 export interface ExtensionInfo {
   id: string;
   name: string;
@@ -25,5 +30,6 @@ export interface ExtensionInfo {
   removable: boolean;
   version: string;
   state: string;
+  error?: ExtensionError;
   path: string;
 }

packages/main/src/plugin/extension-loader.spec.ts

@@ -40,6 +40,7 @@ import type { ApiSenderType } from './api';
 import type { AuthenticationImpl } from './authentication';
 import type { MessageBox } from './message-box';
 import type { Telemetry } from './telemetry/telemetry';
+import type * as containerDesktopAPI from '@podman-desktop/api';
 
 class TestExtensionLoader extends ExtensionLoader {
   public async setupScanningDirectory(): Promise<void> {
@@ -53,6 +54,10 @@ class TestExtensionLoader extends ExtensionLoader {
   setWatchTimeout(timeout: number): void {
     this.watchTimeout = timeout;
   }
+
+  getExtensionState() {
+    return this.extensionState;
+  }
 }
 
 let extensionLoader: TestExtensionLoader;
@@ -67,7 +72,7 @@ const configurationRegistry: ConfigurationRegistry = {} as unknown as Configurat
 
 const imageRegistry: ImageRegistry = {} as unknown as ImageRegistry;
 
-const apiSender: ApiSenderType = {} as unknown as ApiSenderType;
+const apiSender: ApiSenderType = { send: vi.fn() } as unknown as ApiSenderType;
 
 const trayMenuRegistry: TrayMenuRegistry = {} as unknown as TrayMenuRegistry;
 
@@ -91,7 +96,8 @@ const inputQuickPickRegistry: InputQuickPickRegistry = {} as unknown as InputQui
 
 const authenticationProviderRegistry: AuthenticationImpl = {} as unknown as AuthenticationImpl;
 
-const telemetry: Telemetry = {} as unknown as Telemetry;
+const telemetryTrackMock = vi.fn();
+const telemetry: Telemetry = { track: telemetryTrackMock } as unknown as Telemetry;
 
 /* eslint-disable @typescript-eslint/no-empty-function */
 beforeAll(() => {
@@ -118,6 +124,7 @@ beforeAll(() => {
 });
 
 beforeEach(() => {
+  telemetryTrackMock.mockImplementation(() => Promise.resolve());
   vi.clearAllMocks();
 });
 
@@ -230,3 +237,23 @@ test('Should load file from watching scanning folder', async () => {
   // expect to load only one file (other are invalid files/folder)
   expect(loadPackagedFileMock).toBeCalledWith(path.resolve(rootedFakeDirectory, 'watch.cdix'));
 });
+
+test('Verify extension error leads to failed state', async () => {
+  const id = 'extension.id';
+  await extensionLoader.activateExtension(
+    {
+      id: id,
+      path: 'dummy',
+      api: {} as typeof containerDesktopAPI,
+      mainPath: '',
+      removable: false,
+      manifest: {},
+    },
+    {
+      activate: () => {
+        throw Error('Failed');
+      },
+    },
+  );
+  expect(extensionLoader.getExtensionState().get(id)).toBe('failed');
+});

packages/main/src/plugin/extension-loader.ts

@@ -21,7 +21,7 @@ import * as path from 'path';
 import * as os from 'os';
 import * as fs from 'fs';
 import type { CommandRegistry } from './command-registry';
-import type { ExtensionInfo } from './api/extension-info';
+import type { ExtensionError, ExtensionInfo } from './api/extension-info';
 import * as zipper from 'zip-local';
 import type { TrayMenuRegistry } from './tray-menu-registry';
 import { Disposable } from './types/disposable';
@@ -89,7 +89,8 @@ export class ExtensionLoader {
   private analyzedExtensions = new Map<string, AnalyzedExtension>();
   private watcherExtensions = new Map<string, containerDesktopAPI.FileSystemWatcher>();
   private reloadInProgressExtensions = new Map<string, boolean>();
-  private extensionState = new Map<string, string>();
+  protected extensionState = new Map<string, string>();
+  protected extensionStateErrors = new Map<string, unknown>();
 
   protected watchTimeout = 1000;
 
@@ -121,6 +122,18 @@ export class ExtensionLoader {
     private telemetry: Telemetry,
   ) {}
 
+  mapError(err: unknown): ExtensionError | undefined {
+    if (err) {
+      if (err instanceof Error) {
+        return { message: err.message, stack: err.stack };
+      } else {
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        return { message: (err as any).toString() };
+      }
+    }
+    return undefined;
+  }
+
   async listExtensions(): Promise<ExtensionInfo[]> {
     return Array.from(this.analyzedExtensions.values()).map(extension => ({
       name: extension.manifest.name,
@@ -129,6 +142,7 @@ export class ExtensionLoader {
       version: extension.manifest.version,
       publisher: extension.manifest.publisher,
       state: this.extensionState.get(extension.id) || 'stopped',
+      error: this.mapError(this.extensionStateErrors.get(extension.id)),
       id: extension.id,
       path: extension.path,
       removable: extension.removable,
@@ -370,25 +384,40 @@ export class ExtensionLoader {
     }
 
     this.analyzedExtensions.set(extension.id, extension);
+    this.extensionState.delete(extension.id);
+    this.extensionStateErrors.delete(extension.id);
 
-    // in development mode, watch if the extension is updated and reload it
-    if (import.meta.env.DEV && !this.watcherExtensions.has(extension.id)) {
-      const extensionWatcher = this.fileSystemMonitoring.createFileSystemWatcher(extensionPath);
-      extensionWatcher.onDidChange(async () => {
-        // wait 1 second before trying to reload the extension
-        // this is to avoid reloading the extension while it is still being updated
-        setTimeout(() => {
-          this.reloadExtension(extension, removable).catch((error: unknown) =>
-            console.error('error while reloading extension', error),
-          );
-        }, 1000);
+    const telemetryOptions = { extensionId: extension.id };
+
+    try {
+      // in development mode, watch if the extension is updated and reload it
+      if (import.meta.env.DEV && !this.watcherExtensions.has(extension.id)) {
+        const extensionWatcher = this.fileSystemMonitoring.createFileSystemWatcher(extensionPath);
+        extensionWatcher.onDidChange(async () => {
+          // wait 1 second before trying to reload the extension
+          // this is to avoid reloading the extension while it is still being updated
+          setTimeout(() => {
+            this.reloadExtension(extension, removable).catch((error: unknown) =>
+              console.error('error while reloading extension', error),
+            );
+          }, 1000);
+        });
+        this.watcherExtensions.set(extension.id, extensionWatcher);
+      }
+
+      const runtime = this.loadRuntime(extension);
+
+      await this.activateExtension(extension, runtime);
+    } catch (err) {
+      this.extensionState.set(extension.id, 'failed');
+      this.extensionStateErrors.set(extension.id, err);
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      (telemetryOptions as any).error = err;
+    } finally {
+      this.telemetry.track('loadExtension', telemetryOptions).catch((error: unknown) => {
+        console.error('error while tracking loadExtension telemetry', error);
       });
-      this.watcherExtensions.set(extension.id, extensionWatcher);
     }
-
-    const runtime = this.loadRuntime(extension);
-
-    await this.activateExtension(extension, runtime);
   }
 
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -774,6 +803,7 @@ export class ExtensionLoader {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   async activateExtension(extension: AnalyzedExtension, extensionMain: any): Promise<void> {
     this.extensionState.set(extension.id, 'starting');
+    this.extensionStateErrors.delete(extension.id);
     this.apiSender.send('extension-starting', {});
 
     const subscriptions: containerDesktopAPI.Disposable[] = [];
@@ -786,21 +816,35 @@ export class ExtensionLoader {
     if (typeof extensionMain['deactivate'] === 'function') {
       deactivateFunction = extensionMain['deactivate'];
     }
-    if (typeof extensionMain['activate'] === 'function') {
-      // activate the extension
-      console.log(`Activating extension (${extension.id})`);
-      await extensionMain['activate'].apply(undefined, [extensionContext]);
-      console.log(`Activation extension (${extension.id}) ended`);
+
+    const telemetryOptions = { extensionId: extension.id };
+    try {
+      if (typeof extensionMain['activate'] === 'function') {
+        // it returns exports
+        console.log(`Activating extension (${extension.id})`);
+        await extensionMain['activate'].apply(undefined, [extensionContext]);
+        console.log(`Activation extension (${extension.id}) ended`);
+      }
+      const id = extension.id;
+      const activatedExtension: ActivatedExtension = {
+        id,
+        deactivateFunction,
+        extensionContext,
+      };
+      this.activatedExtensions.set(extension.id, activatedExtension);
+      this.extensionState.set(extension.id, 'started');
+      this.apiSender.send('extension-started');
+    } catch (err) {
+      console.log(`Activation extension ${extension.id} failed error:${err}`);
+      this.extensionState.set(extension.id, 'failed');
+      this.extensionStateErrors.set(extension.id, err);
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      (telemetryOptions as any).error = err;
+    } finally {
+      this.telemetry
+        .track('activateExtension', telemetryOptions)
+        .catch((error: unknown) => console.log(`Failed to track activateExtension telemetry event. Error: ${error}`));
     }
-    const id = extension.id;
-    const activatedExtension: ActivatedExtension = {
-      id,
-      deactivateFunction,
-      extensionContext,
-    };
-    this.activatedExtensions.set(extension.id, activatedExtension);
-    this.extensionState.set(extension.id, 'started');
-    this.apiSender.send('extension-started');
   }
 
   async deactivateExtension(extensionId: string): Promise<void> {
@@ -809,11 +853,19 @@ export class ExtensionLoader {
       return;
     }
 
+    const telemetryOptions = { extensionId: extension.id };
+
     this.extensionState.set(extension.id, 'stopping');
     this.apiSender.send('extension-stopping');
 
     if (extension.deactivateFunction) {
-      await extension.deactivateFunction();
+      try {
+        await extension.deactivateFunction();
+      } catch (err) {
+        console.log(`Deactivation extension ${extension.id} failed error:${err}`);
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        (telemetryOptions as any).error = err;
+      }
     }
 
     // dispose subscriptions
@@ -835,6 +887,9 @@ export class ExtensionLoader {
     this.activatedExtensions.delete(extensionId);
     this.extensionState.set(extension.id, 'stopped');
     this.apiSender.send('extension-stopped');
+    this.telemetry
+      .track('deactivateExtension', telemetryOptions)
+      .catch((error: unknown) => console.log(`Failed to track deactivateExtension telemetry event. Error: ${error}`));
   }
 
   async stopAllExtensions(): Promise<void> {

packages/renderer/src/lib/preferences/PreferencesExtensionRendering.svelte

@@ -36,10 +36,10 @@ async function removeExtension() {
         </div>
 
         <div class="py-2 flex flex-row items-center">
-          <!-- start is enabled only when stopped -->
+          <!-- start is enabled only when stopped or failed -->
           <div class="px-2 text-sm italic text-gray-700">
             <button
-              disabled="{extensionInfo.state !== 'stopped'}"
+              disabled="{extensionInfo.state !== 'stopped' && extensionInfo.state !== 'failed'}"
               on:click="{() => startExtension()}"
               class="pf-c-button pf-m-primary"
               type="button">
@@ -64,11 +64,11 @@ async function removeExtension() {
             </button>
           </div>
 
-          <!-- delete is enabled only when stopped -->
+          <!-- delete is enabled only when stopped or failed -->
           {#if extensionInfo.removable}
             <div class="px-2 text-sm italic text-gray-700">
               <button
-                disabled="{extensionInfo.state !== 'stopped'}"
+                disabled="{extensionInfo.state !== 'stopped' && extensionInfo.state !== 'failed'}"
                 on:click="{() => removeExtension()}"
                 class="pf-c-button pf-m-primary"
                 type="button">
@@ -82,6 +82,15 @@ async function removeExtension() {
             <div class="text-gray-900 items-center px-2 text-sm">Default extension, cannot be removed</div>
           {/if}
         </div>
+        {#if extensionInfo.error}
+          <div class="flex flex-col">
+            <div class="py-2">Extension error: {extensionInfo.error.message}</div>
+            {#if extensionInfo.error.stack}
+              <div class="py-2">Stack trace</div>
+              <div class="py-2">{extensionInfo.error.stack}</div>
+            {/if}
+          </div>
+        {/if}
       </Route>
     {/if}
   </div></SettingsPage>

packages/renderer/src/lib/ui/ConnectionStatus.svelte

@@ -42,6 +42,14 @@ const statusesStyle = new Map<string, connectionStatusStyle>([
       label: 'STOPPING',
     },
   ],
+  [
+    'failed',
+    {
+      bgColor: 'bg-red-500',
+      txtColor: 'text-red-500',
+      label: 'FAILED',
+    },
+  ],
 ]);
 $: statusStyle = statusesStyle.get(status) || {
   bgColor: 'bg-gray-900',