improved port-forward error reporting (#4108)

* improved port-forward error reporting

Signed-off-by: Jim Ehrismann <jehrismann@mirantis.com>

* satisfy lint and some autoformatting

Signed-off-by: Jim Ehrismann <jehrismann@mirantis.com>

src/renderer/components/+network-port-forwards/port-forward-menu.tsx

@@ -26,6 +26,7 @@ import { MenuActions, MenuActionsProps } from "../menu/menu-actions";
 import { MenuItem } from "../menu";
 import { Icon } from "../icon";
 import { PortForwardDialog } from "../../port-forward";
+import { Notifications } from "../notifications";
 
 interface Props extends MenuActionsProps {
   portForward: PortForwardItem;
@@ -35,7 +36,13 @@ interface Props extends MenuActionsProps {
 export class PortForwardMenu extends React.Component<Props> {
   @boundMethod
   remove() {
-    return removePortForward(this.props.portForward);
+    const { portForward } = this.props;
+
+    try {
+      removePortForward(portForward);
+    } catch (error) {
+      Notifications.error(`Error occurred stopping the port-forward from port ${portForward.forwardPort}. The port-forward may still be active.`);
+    }
   }
 
   renderContent() {

src/renderer/components/+network-services/service-port-component.tsx

@@ -30,7 +30,6 @@ import { Notifications } from "../notifications";
 import { Button } from "../button";
 import { addPortForward, getPortForward, openPortForward, PortForwardDialog, portForwardStore, removePortForward } from "../../port-forward";
 import type { ForwardedPort } from "../../port-forward";
-import logger from "../../../common/logger";
 import { Spinner } from "../spinner";
 
 interface Props {
@@ -47,21 +46,15 @@ export class ServicePortComponent extends React.Component<Props> {
   constructor(props: Props) {
     super(props);
     makeObservable(this);
-    this.init();
+    this.checkExistingPortForwarding();
   }
 
   componentDidMount() {
     disposeOnUnmount(this, [
-      reaction(() => [ portForwardStore.portForwards, this.props.service ], () => this.init()),
+      reaction(() => [portForwardStore.portForwards, this.props.service], () => this.checkExistingPortForwarding()),
     ]);
   }
 
-  init() {
-    this.checkExistingPortForwarding().catch(error => {
-      logger.error(error);
-    });
-  }
-
   async checkExistingPortForwarding() {
     const { service, port } = this.props;
     const portForward: ForwardedPort = {
@@ -71,7 +64,16 @@ export class ServicePortComponent extends React.Component<Props> {
       port: port.port,
       forwardPort: this.forwardPort,
     };
-    const activePort = await getPortForward(portForward) ?? 0;
+
+    let activePort: number;
+
+    try {
+      activePort = await getPortForward(portForward) ?? 0;
+    } catch (error) {
+      this.isPortForwarded = false;
+
+      return;
+    }
 
     this.forwardPort = activePort;
     this.isPortForwarded = activePort ? true : false;
@@ -88,7 +90,6 @@ export class ServicePortComponent extends React.Component<Props> {
     };
 
     this.waiting = true;
-    this.isPortForwarded = false;
 
     try {
       this.forwardPort = await addPortForward(portForward);
@@ -99,7 +100,8 @@ export class ServicePortComponent extends React.Component<Props> {
         this.isPortForwarded = true;
       }
     } catch (error) {
-      Notifications.error(error);
+      Notifications.error("Error occurred starting port-forward, the local port may not be available");
+      this.checkExistingPortForwarding();
     } finally {
       this.waiting = false;
     }
@@ -121,7 +123,8 @@ export class ServicePortComponent extends React.Component<Props> {
       await removePortForward(portForward);
       this.isPortForwarded = false;
     } catch (error) {
-      Notifications.error(error);
+      Notifications.error(`Error occurred stopping the port-forward from port ${portForward.forwardPort}.`);
+      this.checkExistingPortForwarding();
     } finally {
       this.waiting = false;
     }

src/renderer/components/+workloads-pods/pod-container-port.tsx

@@ -30,7 +30,6 @@ import { Notifications } from "../notifications";
 import { Button } from "../button";
 import { addPortForward, getPortForward, openPortForward, PortForwardDialog, portForwardStore, removePortForward } from "../../port-forward";
 import type { ForwardedPort } from "../../port-forward";
-import logger from "../../../common/logger";
 import { Spinner } from "../spinner";
 
 interface Props {
@@ -51,21 +50,15 @@ export class PodContainerPort extends React.Component<Props> {
   constructor(props: Props) {
     super(props);
     makeObservable(this);
-    this.init();
+    this.checkExistingPortForwarding();
   }
 
   componentDidMount() {
     disposeOnUnmount(this, [
-      reaction(() => [ portForwardStore.portForwards, this.props.pod ], () => this.init()),
+      reaction(() => [portForwardStore.portForwards, this.props.pod], () => this.checkExistingPortForwarding()),
     ]);
   }
 
-  init() {
-    this.checkExistingPortForwarding().catch(error => {
-      logger.error(error);
-    });
-  }
-
   async checkExistingPortForwarding() {
     const { pod, port } = this.props;
     const portForward: ForwardedPort = {
@@ -75,7 +68,16 @@ export class PodContainerPort extends React.Component<Props> {
       port: port.containerPort,
       forwardPort: this.forwardPort,
     };
-    const activePort = await getPortForward(portForward) ?? 0;
+
+    let activePort: number;
+
+    try {
+      activePort = await getPortForward(portForward) ?? 0;
+    } catch (error) {
+      this.isPortForwarded = false;
+
+      return;
+    }
 
     this.forwardPort = activePort;
     this.isPortForwarded = activePort ? true : false;
@@ -92,7 +94,6 @@ export class PodContainerPort extends React.Component<Props> {
     };
 
     this.waiting = true;
-    this.isPortForwarded = false;
 
     try {
       this.forwardPort = await addPortForward(portForward);
@@ -103,7 +104,8 @@ export class PodContainerPort extends React.Component<Props> {
         this.isPortForwarded = true;
       }
     } catch (error) {
-      Notifications.error(error);
+      Notifications.error("Error occurred starting port-forward, the local port may not be available");
+      this.checkExistingPortForwarding();
     } finally {
       this.waiting = false;
     }
@@ -125,7 +127,8 @@ export class PodContainerPort extends React.Component<Props> {
       await removePortForward(portForward);
       this.isPortForwarded = false;
     } catch (error) {
-      Notifications.error(error);
+      Notifications.error(`Error occurred stopping the port-forward from port ${portForward.forwardPort}.`);
+      this.checkExistingPortForwarding();
     } finally {
       this.waiting = false;
     }

src/renderer/port-forward/port-forward-dialog.tsx

@@ -108,7 +108,7 @@ export class PortForwardDialog extends Component<Props> {
         openPortForward(portForward);
       }
     } catch (err) {
-      Notifications.error(err);
+      Notifications.error("Error occurred starting port-forward, the local port may not be available");
     } finally {
       close();
     }

src/renderer/port-forward/port-forward.store.ts

@@ -111,7 +111,8 @@ export async function addPortForward(portForward: ForwardedPort): Promise<number
       logger.warn(`specified ${portForward.forwardPort} got ${response.port}`);
     }
   } catch (error) {
-    logger.warn(error); // don't care, caller must check 
+    logger.warn("Error adding port-forward:", error, portForward);
+    throw(error);
   }
   portForwardStore.reset();
 
@@ -124,7 +125,8 @@ export async function getPortForward(portForward: ForwardedPort): Promise<number
   try {
     response = await apiBase.get<PortForwardResult>(`/pods/port-forward/${portForward.namespace}/${portForward.kind}/${portForward.name}?port=${portForward.port}&forwardPort=${portForward.forwardPort}`);
   } catch (error) {
-    logger.warn(error); // don't care, caller must check 
+    logger.warn("Error getting port-forward:", error, portForward);
+    throw(error);
   }
 
   return response?.port;
@@ -133,13 +135,10 @@ export async function getPortForward(portForward: ForwardedPort): Promise<number
 export async function modifyPortForward(portForward: ForwardedPort, desiredPort: number): Promise<number> {
   let port = 0;
   
-  try {
+  await removePortForward(portForward);
-    await removePortForward(portForward);
+  portForward.forwardPort = desiredPort;
-    portForward.forwardPort = desiredPort;
+  port = await addPortForward(portForward);
-    port = await addPortForward(portForward);
+
-  } catch (error) {
-    logger.warn(error); // don't care, caller must check 
-  }
   portForwardStore.reset();
 
   return port;
@@ -151,7 +150,8 @@ export async function removePortForward(portForward: ForwardedPort) {
     await apiBase.del(`/pods/port-forward/${portForward.namespace}/${portForward.kind}/${portForward.name}?port=${portForward.port}&forwardPort=${portForward.forwardPort}`);
     await waitUntilFree(+portForward.forwardPort, 200, 1000);
   } catch (error) {
-    logger.warn(error); // don't care, caller must check 
+    logger.warn("Error removing port-forward:", error, portForward);
+    throw(error);
   }
   portForwardStore.reset();
 }
@@ -162,7 +162,7 @@ export async function getPortForwards(): Promise<ForwardedPort[]> {
 
     return response.portForwards;
   } catch (error) {
-    logger.warn(error); // don't care, caller must check 
+    logger.warn("Error getting all port-forwards:", error);
     
     return [];
   }