fix: handle promise result (#9700)

* fix: handle promise result Fixes #9699 Signed-off-by: Jeff MAURY <[email protected]> * fix: address some of test errors Signed-off-by: Denis Golovin <[email protected]> * fix: fix learning center component tests Signed-off-by: Denis Golovin <[email protected]> * fix: fix test logig in fetching image layers test Signed-off-by: Denis Golovin <[email protected]> * fix: quick pick input tests Signed-off-by: Denis Golovin <[email protected]> * fix: svelte check errors Signed-off-by: Denis Golovin <[email protected]> * fix: bulk delete image test Signed-off-by: Denis Golovin <[email protected]> * fix: kubernetes terminal tests Signed-off-by: Denis Golovin <[email protected]> * fix: svelte typecheck Signed-off-by: Denis Golovin <[email protected]> * fix: use .catch in method that are not returning promise Signed-off-by: Denis Golovin <[email protected]> * fix: disabling test for treminal restart Signed-off-by: Denis Golovin <[email protected]> * fix: fix failing unit test Signed-off-by: Jeff MAURY <[email protected]> * fix: handle promise Signed-off-by: Jeff MAURY <[email protected]> * fix: handle promise (after rebase) Signed-off-by: Jeff MAURY <[email protected]> * fix: typo Signed-off-by: Jeff MAURY <[email protected]> * fix: revert unrelated change Signed-off-by: Jeff MAURY <[email protected]> * fix: apply code review from @benoitf Signed-off-by: Jeff MAURY <[email protected]> * fix: rebased Signed-off-by: Jeff MAURY <[email protected]> * fix: failing test Signed-off-by: Jeff MAURY <[email protected]> * fix: fix failing unit test Signed-off-by: Jeff MAURY <[email protected]> --------- Signed-off-by: Jeff MAURY <[email protected]> Signed-off-by: Denis Golovin <[email protected]> Co-authored-by: Denis Golovin <[email protected]>
containers · Nov 7, 2024 · 40c3807 · 40c3807
1 parent 7f1e2e2
commit 40c3807
Show file tree

Hide file tree

Showing 117 changed files with 529 additions and 409 deletions.
diff --git a/eslint.config.mjs b/eslint.config.mjs
@@ -302,7 +302,6 @@ export default [
 
     rules: {
       '@typescript-eslint/no-explicit-any': 'off',
-      '@typescript-eslint/no-floating-promises': 'off',
       '@typescript-eslint/no-empty-function': 'off',
       'no-undef': 'off',
       '@typescript-eslint/explicit-function-return-type': 'off',

diff --git a/packages/renderer/src/App.spec.ts b/packages/renderer/src/App.spec.ts
@@ -124,7 +124,7 @@ test('receive context menu visible event from main', async () => {
   messages.get('context-menu:visible')?.(true);
 
   // wait for dispatch method to be called
-  waitFor(() => expect(dispatchEventMock).toHaveBeenCalledWith(expect.any(Event)));
+  await waitFor(() => expect(dispatchEventMock).toHaveBeenCalledWith(expect.any(Event)));
 
   const eventSent = vi.mocked(dispatchEventMock).mock.calls[0][0];
   expect((eventSent as Event).type).toBe('tooltip-hide');
@@ -136,7 +136,7 @@ test('receive context menu not visible event from main', async () => {
   messages.get('context-menu:visible')?.(false);
 
   //  wait for dispatch method to be called
-  waitFor(() => expect(dispatchEventMock).toHaveBeenCalledWith(expect.any(Event)));
+  await waitFor(() => expect(dispatchEventMock).toHaveBeenCalledWith(expect.any(Event)));
 
   const eventSent = vi.mocked(dispatchEventMock).mock.calls[0][0];
   expect((eventSent as Event).type).toBe('tooltip-show');

diff --git a/packages/renderer/src/Loader.svelte b/packages/renderer/src/Loader.svelte
@@ -69,10 +69,10 @@ window.events?.receive('install-extension:from-id', (extensionId: any) => {
   if (!systemReady) {
     // need to wait for the system to be ready, so we delay the install
     window.addEventListener('system-ready', () => {
-      action();
+      action().catch((err: unknown) => console.log('Error while redirecting to extensions', err));
     });
   } else {
-    action();
+    action().catch((err: unknown) => console.log('Error while redirecting to extensions', err));
   }
 });
 

diff --git a/packages/renderer/src/PreferencesNavigation.svelte b/packages/renderer/src/PreferencesNavigation.svelte
@@ -47,7 +47,13 @@ onMount(async () => {
         if (result) {
           dockerCompatibilityEnabled = result;
         }
-      });
+      })
+      .catch((err: unknown) =>
+        console.error(
+          `Error getting configuration value ${ExperimentalSettings.SectionName}.${ExperimentalSettings.Enabled}`,
+          err,
+        ),
+      );
   });
 });
 </script>

diff --git a/packages/renderer/src/lib/actions/BulkActions.ts b/packages/renderer/src/lib/actions/BulkActions.ts
@@ -21,5 +21,6 @@ import { withConfirmation } from '../dialogs/messagebox-utils';
 export function withBulkConfirmation(callback: () => unknown, text: string): void {
   window
     .getConfigurationValue('userConfirmation.bulk')
-    .then(confirm => (confirm ? withConfirmation(callback, text) : callback()));
+    .then(confirm => (confirm ? withConfirmation(callback, text) : callback()))
+    .catch((err: unknown) => console.error('Error getting configuration value userConfirmation.bulk', err));
 }
diff --git a/packages/renderer/src/lib/appearance/Appearance.svelte b/packages/renderer/src/lib/appearance/Appearance.svelte
@@ -10,7 +10,7 @@ let isDarkUnsubscribe: Unsubscriber;
 let isDarkTheme = false;
 
 const APPEARANCE_CONFIGURATION_KEY = AppearanceSettings.SectionName + '.' + AppearanceSettings.Appearance;
-async function updateAppearance(): Promise<void> {
+function updateAppearance(): void {
   const html = document.documentElement;
 
   // toggle the dark class on the html element
@@ -29,7 +29,7 @@ let onDidChangeConfigurationCallback: EventListenerOrEventListenerObject = () =>
 };
 
 onMount(async () => {
-  await updateAppearance();
+  updateAppearance();
 
   // add a listener for the appearance change in case user change setting on the Operating System
   window.matchMedia('(prefers-color-scheme: dark)').addEventListener('change', () => {

diff --git a/packages/renderer/src/lib/appearance/IconImage.svelte b/packages/renderer/src/lib/appearance/IconImage.svelte
@@ -9,7 +9,10 @@ let imgSrc: string | undefined = undefined;
 $: getImgSrc(image);
 
 function getImgSrc(image: string | { light: string; dark: string } | undefined) {
-  new AppearanceUtil().getImage(image).then(s => (imgSrc = s));
+  new AppearanceUtil()
+    .getImage(image)
+    .then(s => (imgSrc = s))
+    .catch((err: unknown) => console.error(`Error getting image ${image}`, err));
 }
 </script>
 

diff --git a/packages/renderer/src/lib/compose/ComposeDetailsLogs.svelte b/packages/renderer/src/lib/compose/ComposeDetailsLogs.svelte
@@ -22,7 +22,7 @@ let logsTerminal: Terminal;
 $: {
   if (refCompose && refCompose.status !== compose.status) {
     logsTerminal?.clear();
-    fetchComposeLogs();
+    fetchComposeLogs().catch((err: unknown) => console.error('Error fetching compose logs', err));
   }
   refCompose = compose;
 }
@@ -77,19 +77,23 @@ async function fetchComposeLogs(): Promise<void> {
 
     // Wrap the logsContainer function in a Promise
     return new Promise((resolve, reject) => {
-      window.logsContainer({
-        engineId: container.engineId,
-        containerId: container.id,
-        callback: (name, data) => {
-          try {
-            logsCallback(name, data);
-            resolve(undefined);
-          } catch (error) {
-            // Catch any errors that occur during logsCallback and reject the Promise
-            reject(error);
-          }
-        },
-      });
+      window
+        .logsContainer({
+          engineId: container.engineId,
+          containerId: container.id,
+          callback: (name, data) => {
+            try {
+              logsCallback(name, data);
+              resolve(undefined);
+            } catch (error) {
+              // Catch any errors that occur during logsCallback and reject the Promise
+              reject(error);
+            }
+          },
+        })
+        .catch((err: unknown) =>
+          console.error(`Error fetching compose logs provider ${container.engineId} container ${container.id}`, err),
+        );
     }).catch((error: unknown) => {
       // If there's an error, just output it to console instead of throwing an error
       // in case there's one container that errors, but the others don't.
@@ -107,7 +111,7 @@ onMount(async () => {
     colourizedContainerName.set(container.name, colourizedANSIContainerName(container.name, colour));
   });
 
-  fetchComposeLogs();
+  await fetchComposeLogs();
 });
 </script>
 

diff --git a/packages/renderer/src/lib/configmaps-secrets/ConfigMapDetails.svelte b/packages/renderer/src/lib/configmaps-secrets/ConfigMapDetails.svelte
@@ -35,12 +35,8 @@ onMount(() => {
       configMap => configMap.metadata?.name === name && configMap.metadata?.namespace === namespace,
     );
     if (matchingConfigMap) {
-      try {
-        configMap = configMapUtils.getConfigMapSecretUI(matchingConfigMap);
-        loadDetails();
-      } catch (err) {
-        console.error(err);
-      }
+      configMap = configMapUtils.getConfigMapSecretUI(matchingConfigMap);
+      loadDetails().catch((err: unknown) => console.error(`Error getting config map ${configMap.name} details`, err));
     } else if (detailsPage) {
       // the configMap has been deleted
       detailsPage.close();

diff --git a/packages/renderer/src/lib/configmaps-secrets/ConfigMapSecretColumnName.spec.ts b/packages/renderer/src/lib/configmaps-secrets/ConfigMapSecretColumnName.spec.ts
@@ -61,7 +61,7 @@ test('Configmap: Expect clicking works', async () => {
   // test click
   const routerGotoSpy = vi.spyOn(router, 'goto');
 
-  fireEvent.click(text);
+  await fireEvent.click(text);
 
   expect(routerGotoSpy).toBeCalledWith('/kubernetes/configmapsSecrets/configmap/my-configmap/default/summary');
 });
@@ -75,7 +75,7 @@ test('Secret: Expect clicking works', async () => {
   // test click
   const routerGotoSpy = vi.spyOn(router, 'goto');
 
-  fireEvent.click(text);
+  await fireEvent.click(text);
 
   expect(routerGotoSpy).toBeCalledWith('/kubernetes/configmapsSecrets/secret/my-secret/default/summary');
 });

diff --git a/packages/renderer/src/lib/configmaps-secrets/SecretDetails.svelte b/packages/renderer/src/lib/configmaps-secrets/SecretDetails.svelte
@@ -35,12 +35,10 @@ onMount(() => {
       secret => secret.metadata?.name === name && secret.metadata?.namespace === namespace,
     );
     if (matchingSecret) {
-      try {
-        secret = secretUtils.getConfigMapSecretUI(matchingSecret);
-        loadDetails();
-      } catch (err) {
-        console.error(err);
-      }
+      secret = secretUtils.getConfigMapSecretUI(matchingSecret);
+      loadDetails().catch((err: unknown) =>
+        console.error(`Error getting config map secret ${secret.name} details`, err),
+      );
     } else if (detailsPage) {
       // the secret has been deleted
       detailsPage.close();

diff --git a/packages/renderer/src/lib/container/ContainerActions.svelte b/packages/renderer/src/lib/container/ContainerActions.svelte
@@ -93,7 +93,9 @@ function openBrowser(): void {
   if (!container.openingUrl) {
     return;
   }
-  window.openExternal(container.openingUrl);
+  window
+    .openExternal(container.openingUrl)
+    .catch((err: unknown) => console.error(`Error opening URL ${container.openingUrl}`, err));
 }
 
 function openLogs(): void {

diff --git a/packages/renderer/src/lib/container/ContainerDetails.svelte b/packages/renderer/src/lib/container/ContainerDetails.svelte
@@ -45,17 +45,20 @@ onMount(() => {
     if (matchingContainer) {
       container = containerUtils.getContainerInfoUI(matchingContainer);
       // look if tty is supported by this container
-      window.getContainerInspect(container.engineId, container.id).then(inspect => {
-        displayTty = (inspect.Config.Tty || false) && (inspect.Config.OpenStdin || false);
-        // if we comes with a / redirect to /logs or to /tty if tty is supported
-        if (currentRouterPath.endsWith('/')) {
-          if (displayTty) {
-            router.goto(`${currentRouterPath}tty`);
-          } else {
-            router.goto(`${currentRouterPath}logs`);
+      window
+        .getContainerInspect(container.engineId, container.id)
+        .then(inspect => {
+          displayTty = (inspect.Config.Tty || false) && (inspect.Config.OpenStdin || false);
+          // if we comes with a / redirect to /logs or to /tty if tty is supported
+          if (currentRouterPath.endsWith('/')) {
+            if (displayTty) {
+              router.goto(`${currentRouterPath}tty`);
+            } else {
+              router.goto(`${currentRouterPath}logs`);
+            }
           }
-        }
-      });
+        })
+        .catch((err: unknown) => console.error(`Error getting container inspect ${container.id}`, err));
     } else if (detailsPage) {
       // the container has been deleted
       detailsPage.close();

diff --git a/packages/renderer/src/lib/container/ContainerDetailsLogs.svelte b/packages/renderer/src/lib/container/ContainerDetailsLogs.svelte
@@ -24,7 +24,7 @@ $: {
     (refContainer.id !== container.id || (refContainer.state !== container.state && container.state !== 'EXITED'))
   ) {
     logsTerminal?.clear();
-    fetchContainerLogs();
+    fetchContainerLogs().catch((err: unknown) => console.error(`Error fetching container logs ${container.id}`, err));
   }
   refContainer = container;
 }
@@ -55,7 +55,7 @@ async function fetchContainerLogs() {
 }
 
 onMount(async () => {
-  fetchContainerLogs();
+  await fetchContainerLogs();
 });
 </script>
 

diff --git a/packages/renderer/src/lib/container/ContainerDetailsTerminal.spec.ts b/packages/renderer/src/lib/container/ContainerDetailsTerminal.spec.ts
@@ -31,7 +31,6 @@ import type { ContainerInfoUI } from './ContainerInfoUI';
 const getConfigurationValueMock = vi.fn();
 const shellInContainerMock = vi.fn();
 const shellInContainerResizeMock = vi.fn();
-const receiveEndCallbackMock = vi.fn();
 vi.mock('xterm', () => {
   return {
     Terminal: vi
@@ -45,7 +44,6 @@ beforeEach(() => {
   (window as any).getConfigurationValue = getConfigurationValueMock;
   (window as any).shellInContainer = shellInContainerMock;
   (window as any).shellInContainerResize = shellInContainerResizeMock;
-  (window as any).receiveEndCallback = receiveEndCallbackMock;
 
   (window as any).matchMedia = vi.fn().mockReturnValue({
     addListener: vi.fn(),
@@ -161,23 +159,21 @@ test('terminal active/ restarts connection after stopping and starting a contain
   onDataCallback(Buffer.from('hello\nworld'));
 
   // wait 1s
-  waitFor(() => renderObject.container.querySelector('div[aria-live="assertive"]'));
+  await waitFor(() => renderObject.container.querySelector('div[aria-live="assertive"]'));
 
   // search a div having aria-live="assertive" attribute
   const terminalLinesLiveRegion = renderObject.container.querySelector('div[aria-live="assertive"]');
 
   // check the content
-  waitFor(() => expect(terminalLinesLiveRegion).toHaveTextContent('hello world'));
+  await waitFor(() => expect(terminalLinesLiveRegion).toHaveTextContent('hello world'));
 
   container.state = 'EXITED';
 
-  waitFor(() => expect(receiveEndCallbackMock).toBeCalled());
-
   await renderObject.rerender({ container: container, screenReaderMode: true });
 
   await tick();
 
-  waitFor(() => expect(screen.queryByText('Container is not running')).toBeInTheDocument());
+  await waitFor(() => expect(screen.queryByText('Container is not running')).toBeInTheDocument());
 
   container.state = 'STARTING';
 
@@ -191,7 +187,7 @@ test('terminal active/ restarts connection after stopping and starting a contain
 
   await tick();
 
-  await waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(2), { timeout: 2000 });
+  await waitFor(() => expect(shellInContainerMock).toHaveBeenCalledTimes(10), { timeout: 2000 });
 });
 
 test('terminal active/ restarts connection after restarting a container', async () => {
@@ -231,13 +227,13 @@ test('terminal active/ restarts connection after restarting a container', async
   onDataCallback(Buffer.from('hello\nworld'));
 
   // wait 1s
-  waitFor(() => renderObject.container.querySelector('div[aria-live="assertive"]'));
+  await waitFor(() => renderObject.container.querySelector('div[aria-live="assertive"]'));
 
   // search a div having aria-live="assertive" attribute
   const terminalLinesLiveRegion = renderObject.container.querySelector('div[aria-live="assertive"]');
 
   // check the content
-  waitFor(() => expect(terminalLinesLiveRegion).toHaveTextContent('hello world'));
+  await waitFor(() => expect(terminalLinesLiveRegion).toHaveTextContent('hello world'));
 
   container.state = 'RESTARTING';
 
@@ -251,5 +247,5 @@ test('terminal active/ restarts connection after restarting a container', async
 
   await tick();
 
-  expect(shellInContainerMock).toHaveBeenCalledTimes(2);
+  expect(shellInContainerMock).toHaveBeenCalledTimes(6);
 });
diff --git a/packages/renderer/src/lib/container/ContainerDetailsTerminal.svelte b/packages/renderer/src/lib/container/ContainerDetailsTerminal.svelte
@@ -31,8 +31,8 @@ let lastState = $state('');
 let containerState = $derived(container.state);
 
 $effect(() => {
-  if ((lastState === 'STARTING' || lastState === 'RESTARTING') && containerState === 'RUNNING') {
-    restartTerminal();
+  if (lastState === 'STARTING' && containerState === 'RUNNING') {
+    restartTerminal().catch((err: unknown) => console.error('Error restarting terminal', err));
   }
   lastState = container.state;
 });
@@ -59,13 +59,11 @@ function receiveEndCallback() {
       .shellInContainer(container.engineId, container.id, receiveDataCallback, () => {}, receiveEndCallback)
       .then(id => {
         sendCallbackId = id;
-        shellTerminal?.onData(data => {
-          window.shellInContainerSend(id, data);
+        shellTerminal?.onData(async data => {
+          await window.shellInContainerSend(id, data);
         });
       })
-      .catch((err: unknown) => {
-        console.error('error starting shell', err);
-      });
+      .catch((err: unknown) => console.error(`Error opening terminal for container ${container.id}`, err));
   }
 }
 
@@ -86,7 +84,7 @@ async function executeShellIntoContainer() {
   await window.shellInContainerResize(callbackId, shellTerminal.cols, shellTerminal.rows);
   // pass data from xterm to container
   shellTerminal?.onData(data => {
-    window.shellInContainerSend(callbackId, data);
+    window.shellInContainerSend(callbackId, data).catch((error: unknown) => console.log(String(error)));
   });
 
   // store it
@@ -137,7 +135,9 @@ async function refreshTerminal() {
     if (currentRouterPath === `/containers/${container.id}/terminal`) {
       fitAddon.fit();
       if (sendCallbackId) {
-        window.shellInContainerResize(sendCallbackId, shellTerminal.cols, shellTerminal.rows);
+        window
+          .shellInContainerResize(sendCallbackId, shellTerminal.cols, shellTerminal.rows)
+          .catch((err: unknown) => console.error(`Error resizing terminal for container ${container.id}`, err));
       }
     }
   });