refactoring and bug fixing, still issue with port-forward-dialog

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

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

@@ -72,10 +72,12 @@ export class PortForwardMenu extends React.Component<Props> {
 
     return (
       <>
-        <MenuItem onClick={() => openPortForward(this.props.portForward)}>
+        { Boolean(portForward.status === "Active") &&
-          <Icon material="open_in_browser" interactive={toolbar} tooltip="Open in browser" />
+          <MenuItem onClick={() => openPortForward(portForward)}>
-          <span className="title">Open</span>
+            <Icon material="open_in_browser" interactive={toolbar} tooltip="Open in browser" />
-        </MenuItem>
+            <span className="title">Open</span>
+          </MenuItem>
+        }
         <MenuItem onClick={() => PortForwardDialog.open(portForward)}>
           <Icon material="edit" tooltip="Change port or protocol" interactive={toolbar} />
           <span className="title">Edit</span>

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

@@ -28,9 +28,10 @@ import { observable, makeObservable, reaction, action } from "mobx";
 import { cssNames } from "../../utils";
 import { Notifications } from "../notifications";
 import { Button } from "../button";
-import { aboutPortForwarding, addPortForward, forwardedPortStatus, getPortForward, getPortForwards, getPortForwardStatus, openPortForward, PortForwardDialog, portForwardStore, predictProtocol, removePortForward } from "../../port-forward";
+import { aboutPortForwarding, addPortForward, getPortForward, getPortForwards, openPortForward, PortForwardDialog, portForwardStore, predictProtocol, removePortForward } from "../../port-forward";
 import type { ForwardedPort } from "../../port-forward";
 import { Spinner } from "../spinner";
+import logger from "../../../common/logger";
 
 interface Props {
   service: Service;
@@ -57,7 +58,7 @@ export class ServicePortComponent extends React.Component<Props> {
 
   async checkExistingPortForwarding() {
     const { service, port } = this.props;
-    const portForward: ForwardedPort = {
+    let portForward: ForwardedPort = {
       kind: "service",
       name: service.getName(),
       namespace: service.getNs(),
@@ -65,32 +66,22 @@ export class ServicePortComponent extends React.Component<Props> {
       forwardPort: this.forwardPort,
     };
 
-    let activePort: number;
-    let status: forwardedPortStatus;
-
-    console.log("checkExistingPortForwarding()");
-
     try {
-      activePort = await getPortForward(portForward) ?? 0;
+      portForward = await getPortForward(portForward);
-      status = await getPortForwardStatus(portForward);
     } catch (error) {
       this.isPortForwarded = false;
 
-      console.log("(catch) isPortForwarded:", this.isPortForwarded, this.forwardPort);
-
       return;
     }
 
-    this.forwardPort = activePort;
+    this.forwardPort = portForward.forwardPort;
-    this.isPortForwarded = (status === "Active" && activePort) ? true : false;
+    this.isPortForwarded = (portForward.status === "Active" && portForward.forwardPort) ? true : false;
-
-    console.log("isPortForwarded:", this.isPortForwarded, this.forwardPort);
   }
 
   @action
   async portForward() {
     const { service, port } = this.props;
-    const portForward: ForwardedPort = {
+    let portForward: ForwardedPort = {
       kind: "service",
       name: service.getName(),
       namespace: service.getNs(),
@@ -106,10 +97,11 @@ export class ServicePortComponent extends React.Component<Props> {
       // determine how many port-forwards already exist
       const { length } = getPortForwards();
 
-      this.forwardPort = await addPortForward(portForward);
+      portForward = await addPortForward(portForward);
 
-      if (this.forwardPort) {
+      this.forwardPort = portForward.forwardPort;
-        portForward.forwardPort = this.forwardPort;
+
+      if (portForward.status === "Active") {
         openPortForward(portForward);
         this.isPortForwarded = true;
 
@@ -117,9 +109,12 @@ export class ServicePortComponent extends React.Component<Props> {
         if (!length) {
           aboutPortForwarding();
         }
+      } else {
+        Notifications.error(`Error occurred starting port-forward, the local port may not be available or the ${portForward.kind} ${portForward.name} may not be reachable`);
+        this.isPortForwarded = false;
       }
     } catch (error) {
-      Notifications.error(`Error occurred starting port-forward, the local port may not be available or the ${portForward.kind} ${portForward.name} may not be reachable`);
+      logger.error("[SERVICE-PORT-COMPONENT]:", error, portForward);
       this.checkExistingPortForwarding();
     } finally {
       this.waiting = false;

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

@@ -28,8 +28,9 @@ import { action, observable, makeObservable, reaction } from "mobx";
 import { cssNames } from "../../utils";
 import { Notifications } from "../notifications";
 import { Button } from "../button";
-import { aboutPortForwarding, addPortForward, forwardedPortStatus, getPortForward, getPortForwards, getPortForwardStatus, openPortForward, PortForwardDialog, portForwardStore, predictProtocol, removePortForward } from "../../port-forward";import type { ForwardedPort } from "../../port-forward";
+import { aboutPortForwarding, addPortForward, getPortForward, getPortForwards, openPortForward, PortForwardDialog, portForwardStore, predictProtocol, removePortForward } from "../../port-forward";import type { ForwardedPort } from "../../port-forward";
 import { Spinner } from "../spinner";
+import logger from "../../../common/logger";
 
 interface Props {
   pod: Pod;
@@ -60,7 +61,7 @@ export class PodContainerPort extends React.Component<Props> {
 
   async checkExistingPortForwarding() {
     const { pod, port } = this.props;
-    const portForward: ForwardedPort = {
+    let portForward: ForwardedPort = {
       kind: "pod",
       name: pod.getName(),
       namespace: pod.getNs(),
@@ -68,26 +69,22 @@ export class PodContainerPort extends React.Component<Props> {
       forwardPort: this.forwardPort,
     };
 
-    let activePort: number;
-    let status: forwardedPortStatus;
-
     try {
-      activePort = await getPortForward(portForward) ?? 0;
+      portForward = await getPortForward(portForward);
-      status = await getPortForwardStatus(portForward);
     } catch (error) {
       this.isPortForwarded = false;
 
       return;
     }
 
-    this.forwardPort = activePort;
+    this.forwardPort = portForward.forwardPort;
-    this.isPortForwarded = (status === "Active" && activePort) ? true : false;
+    this.isPortForwarded = (portForward.status === "Active" && portForward.forwardPort) ? true : false;
   }
 
   @action
   async portForward() {
     const { pod, port } = this.props;
-    const portForward: ForwardedPort = {
+    let portForward: ForwardedPort = {
       kind: "pod",
       name: pod.getName(),
       namespace: pod.getNs(),
@@ -103,10 +100,9 @@ export class PodContainerPort extends React.Component<Props> {
       // determine how many port-forwards already exist
       const { length } = getPortForwards();
 
-      this.forwardPort = await addPortForward(portForward);
+      portForward = await addPortForward(portForward);
 
-      if (this.forwardPort) {
+      if (portForward.status === "Active") {
-        portForward.forwardPort = this.forwardPort;
         openPortForward(portForward);
         this.isPortForwarded = true;
 
@@ -114,9 +110,12 @@ export class PodContainerPort extends React.Component<Props> {
         if (!length) {
           aboutPortForwarding();
         }
+      } else {
+        Notifications.error(`Error occurred starting port-forward, the local port may not be available or the ${portForward.kind} ${portForward.name} may not be reachable`);
+        this.isPortForwarded = false;
       }
     } catch (error) {
-      Notifications.error(`Error occurred starting port-forward, the local port may not be available or the ${portForward.kind} ${portForward.name} may not be reachable`);
+      logger.error("[POD-CONTAINER-PORT]:", error, portForward);
       this.checkExistingPortForwarding();
     } finally {
       this.waiting = false;

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

@@ -33,6 +33,7 @@ import { addPortForward, getPortForwards, modifyPortForward } from "./port-forwa
 import type { ForwardedPort } from "./port-forward-item";
 import { aboutPortForwarding, openPortForward } from ".";
 import { Checkbox } from "../components/checkbox";
+import logger from "../../common/logger";
 
 interface Props extends Partial<DialogProps> {
 }
@@ -92,23 +93,21 @@ export class PortForwardDialog extends Component<Props> {
   };
 
   startPortForward = async () => {
-    const { portForward } = this;
+    let { portForward } = this;
     const { currentPort, desiredPort, close } = this;
 
     try {
       // determine how many port-forwards already exist
       const { length } = getPortForwards();
 
-      let port: number;
-
       portForward.protocol = dialogState.useHttps ? "https" : "http";
       portForward.status = "Active";
 
       if (currentPort) {
-        port = await modifyPortForward(portForward, desiredPort);
+        portForward = await modifyPortForward(portForward, desiredPort);
       } else {
         portForward.forwardPort = desiredPort;
-        port = await addPortForward(portForward);
+        portForward = await addPortForward(portForward);
 
         // if this is the first port-forward show the about notification
         if (!length) {
@@ -116,12 +115,15 @@ export class PortForwardDialog extends Component<Props> {
         }
       }
 
-      if (dialogState.openInBrowser) {
+      if (portForward.status === "Active") {
-        portForward.forwardPort = port;
+        if (dialogState.openInBrowser) {
-        openPortForward(portForward);
+          openPortForward(portForward);
+        }
+      } else {
+        Notifications.error(`Error occurred starting port-forward, the local port may not be available or the ${portForward.kind} ${portForward.name} may not be reachable`);
       }
-    } catch (err) {
+    } catch (error) {
-      Notifications.error(`Error occurred starting port-forward, the local port may not be available or the ${portForward.kind} ${portForward.name} may not be reachable`);
+      logger.error("[PORT-FORWARD-DIALOG]:", error, portForward);
     } finally {
       close();
     }

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

@@ -23,7 +23,7 @@
 import type { ItemObject } from "../../common/item.store";
 import { autoBind } from "../../common/utils";
 
-export type forwardedPortStatus = "Active" | "Disabled";
+export type ForwardedPortStatus = "Active" | "Disabled";
 export interface ForwardedPort {
   kind: string;
   namespace: string;
@@ -31,7 +31,7 @@ export interface ForwardedPort {
   port: number;
   forwardPort: number;
   protocol?: string;
-  status?: forwardedPortStatus;
+  status?: ForwardedPortStatus;
 }
   
 export class PortForwardItem implements ItemObject {
@@ -41,7 +41,7 @@ export class PortForwardItem implements ItemObject {
   port: number;
   forwardPort: number;
   protocol: string;
-  status: forwardedPortStatus;
+  status: ForwardedPortStatus;
   
   constructor(pf: ForwardedPort) {
     this.kind = pf.kind;

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

@@ -23,7 +23,7 @@
 import { action, makeObservable, observable, reaction } from "mobx";
 import { ItemStore } from "../../common/item.store";
 import { autoBind, createStorage, disposer } from "../utils";
-import { ForwardedPort, forwardedPortStatus, PortForwardItem } from "./port-forward-item";
+import { ForwardedPort, ForwardedPortStatus, PortForwardItem } from "./port-forward-item";
 import { apiBase } from "../api";
 import { waitUntilFree } from "tcp-port-used";
 import logger from "../../common/logger";
@@ -68,6 +68,8 @@ export class PortForwardStore extends ItemStore<PortForwardItem> {
   }
 
   loadAll() {
+    console.log("portForwardStore.loadAll()");
+    
     return this.loadItems(() => {
       const portForwards = getPortForwards();
 
@@ -127,14 +129,11 @@ const setPortForward = action((portForward: ForwardedPort) => {
   portForwardStore.portForwards[index] = new PortForwardItem(portForward);
 });
 
-export const startPortForward = action( async (portForward: ForwardedPort): Promise<number> => {
+export const startPortForward = action( async (portForward: ForwardedPort): Promise<ForwardedPort> => {
   const pf = findPortForward(portForward);
   
   if (!pf) {
-    const error = new Error("port-forward not found");
+    throw new Error("cannot start non-existent port-forward");
-
-    logger.warn("[PORT-FORWARD-STORE] Error starting port-forward:", error, portForward);
-    throw(error);    
   }
 
   const { port, forwardPort } = pf;
@@ -156,19 +155,22 @@ export const startPortForward = action( async (portForward: ForwardedPort): Prom
   } catch (error) {
     logger.warn("[PORT-FORWARD-STORE] Error starting port-forward:", error, pf);
     pf.status = "Disabled";
-    throw (error);
-  } finally {
-    setPortForward(pf);
   }
 
-  return response?.port;
+  setPortForward(pf);
+
+  portForward.forwardPort = pf.forwardPort;
+  portForward.protocol = pf.protocol;
+  portForward.status = pf.status;
+
+  return portForward;
 });
 
-export const addPortForward = action(async (portForward: ForwardedPort): Promise<number> => {
+export const addPortForward = action(async (portForward: ForwardedPort): Promise<ForwardedPort> => {
   const pf = findPortForward(portForward);
 
   if (pf) {
-    return pf.forwardPort;
+    return pf;
   }
 
   portForwardStore.portForwards.push(new PortForwardItem(portForward));
@@ -177,21 +179,14 @@ export const addPortForward = action(async (portForward: ForwardedPort): Promise
     portForward.status = "Active";
   }
 
-  try {
+  if (portForward.status === "Active") {
-    if (portForward.status === "Active") {
+    portForward = await startPortForward(portForward);
-      const port = await startPortForward(portForward);
-      
-      portForward.forwardPort = port;
-    }
-  } catch (error) {
-    logger.warn("[PORT-FORWARD-STORE] Error adding port-forward:", error, portForward);
-    throw(error);
   }
 
-  return portForward.forwardPort;
+  return portForward;
 });
 
-export async function getActivePortForward(portForward: ForwardedPort): Promise<number> {
+export async function getActivePortForward(portForward: ForwardedPort): Promise<ForwardedPort> {
   const { port, forwardPort, protocol } = portForward;
   let response: PortForwardResult;
 
@@ -199,35 +194,36 @@ export async function getActivePortForward(portForward: ForwardedPort): Promise<
     response = await apiBase.get<PortForwardResult>(`/pods/port-forward/${portForward.namespace}/${portForward.kind}/${portForward.name}`, { query: { port, forwardPort, protocol }});
   } catch (error) {
     logger.warn("[PORT-FORWARD-STORE] Error getting active port-forward:", error, portForward);
-    throw (error);
   }
 
-  return response?.port;
+  portForward.status = response?.port ? "Active" : "Disabled";
+  portForward.forwardPort = response?.port;
+
+  return portForward;
 }
 
-export async function getPortForward(portForward: ForwardedPort): Promise<number> {
+export async function getPortForward(portForward: ForwardedPort): Promise<ForwardedPort> {
-
+  if (!findPortForward(portForward)) {
-  const pf = findPortForward(portForward);
-  
-  if (!pf) {
     throw new Error("port-forward not found");
   }
 
+  let pf: ForwardedPort;
+
   try {
     // check if the port-forward is active, and if so check if it has the same local port
-    const port = await getActivePortForward(portForward);
+    pf = await getActivePortForward(portForward);
 
-    if (port && port !== pf.forwardPort) {
+    if (pf.forwardPort && pf.forwardPort !== portForward.forwardPort) {
-      logger.warn(`[PORT-FORWARD-STORE] local port, expected ${pf.forwardPort}, got ${port}`);
+      logger.warn(`[PORT-FORWARD-STORE] local port, expected ${pf.forwardPort}, got ${portForward.forwardPort}`);
     }
   } catch (error) {
     // port is not active
   }
 
-  return pf.forwardPort;
+  return pf;
 }
 
-export async function getPortForwardStatus(portForward: ForwardedPort): Promise<forwardedPortStatus> {
+export async function getPortForwardStatus(portForward: ForwardedPort): Promise<ForwardedPortStatus> {
 
   const pf = findPortForward(portForward);
   
@@ -246,14 +242,13 @@ export async function getPortForwardStatus(portForward: ForwardedPort): Promise<
   return pf.status;
 }
 
-export const modifyPortForward = action(async (portForward: ForwardedPort, desiredPort: number): Promise<number> => {
+export const modifyPortForward = action(async (portForward: ForwardedPort, desiredPort: number): Promise<ForwardedPort> => {
-  let port = 0;
 
   await removePortForward(portForward);
   portForward.forwardPort = desiredPort;
-  port = await addPortForward(portForward);
+  
+  return await addPortForward(portForward);
 
-  return port;
 });