Fix handling of PTYs when renderer is reloaded (#4346)

Co-authored-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>

src/main/shell-session/node-shell-session.ts

@@ -29,6 +29,7 @@ import { get } from "lodash";
 import { Node, NodesApi } from "../../common/k8s-api/endpoints";
 import { KubeJsonApi } from "../../common/k8s-api/kube-json-api";
 import logger from "../logger";
+import { TerminalChannels } from "../../renderer/api/terminal-api";
 
 export class NodeShellSession extends ShellSession {
   ShellType = "node-shell";
@@ -51,7 +52,10 @@ export class NodeShellSession extends ShellSession {
       await this.waitForRunningPod();
     } catch (error) {
       this.deleteNodeShellPod();
-      this.sendResponse(`Error occurred: ${get(error, "response.body.message", error?.toString() || "unknown error")}`);
+      this.send({
+        type: TerminalChannels.STDOUT,
+        data: `Error occurred: ${get(error, "response.body.message", error?.toString() || "unknown error")}`,
+      });
 
       throw new ShellOpenError("failed to create node pod", error);
     }

src/main/shell-session/shell-session.ts

@@ -32,6 +32,8 @@ import { UserStore } from "../../common/user-store";
 import * as pty from "node-pty";
 import { appEventBus } from "../../common/event-bus";
 import logger from "../logger";
+import { TerminalChannels, TerminalMessage } from "../../renderer/api/terminal-api";
+import { deserialize, serialize } from "v8";
 
 export class ShellOpenError extends Error {
   constructor(message: string, public cause: Error) {
@@ -145,18 +147,24 @@ export abstract class ShellSession {
 
   protected abstract get cwd(): string | undefined;
 
-  protected ensureShellProcess(shell: string, args: string[], env: Record<string, string>, cwd: string): pty.IPty {
-    if (!ShellSession.processes.has(this.terminalId)) {
+  protected ensureShellProcess(shell: string, args: string[], env: Record<string, string>, cwd: string): { shellProcess: pty.IPty, resume: boolean } {
+    const resume = ShellSession.processes.has(this.terminalId);
+
+    if (!resume) {
       ShellSession.processes.set(this.terminalId, pty.spawn(shell, args, {
+        rows: 30,
         cols: 80,
         cwd,
         env,
         name: "xterm-256color",
-        rows: 30,
       }));
     }
 
-    return ShellSession.processes.get(this.terminalId);
+    const shellProcess = ShellSession.processes.get(this.terminalId);
+
+    logger.info(`[SHELL-SESSION]: PTY for ${this.terminalId} is ${resume ? "resumed" : "started"} with PID=${shellProcess.pid}`);
+
+    return { shellProcess, resume };
   }
 
   constructor(protected websocket: WebSocket, protected cluster: Cluster, terminalId: string) {
@@ -166,56 +174,88 @@ export abstract class ShellSession {
     this.terminalId = `${cluster.id}:${terminalId}`;
   }
 
+  protected send(message: TerminalMessage): void {
+    this.websocket.send(serialize(message));
+  }
+
   protected async openShellProcess(shell: string, args: string[], env: Record<string, any>) {
     const cwd = (this.cwd && await fse.pathExists(this.cwd))
     	? this.cwd
     	: env.HOME;
-    const shellProcess = this.ensureShellProcess(shell, args, env, cwd);
+    const { shellProcess, resume } = this.ensureShellProcess(shell, args, env, cwd);
+
+    if (resume) {
+      this.send({ type: TerminalChannels.CONNECTED });
+    }
 
     this.running = true;
-    shellProcess.onData(data => this.sendResponse(data));
+    shellProcess.onData(data => this.send({ type: TerminalChannels.STDOUT, data }));
     shellProcess.onExit(({ exitCode }) => {
-      this.running = false;
+      logger.info(`[SHELL-SESSION]: shell has exited for ${this.terminalId} closed with exitcode=${exitCode}`);
 
-      if (exitCode > 0) {
-        this.sendResponse("Terminal will auto-close in 15 seconds ...");
-        setTimeout(() => this.exit(), 15 * 1000);
-      } else {
-        this.exit();
+      // This might already be false because of the kill() within the websocket.on("close") handler
+      if (this.running) {
+        this.running = false;
+
+        if (exitCode > 0) {
+          this.send({ type: TerminalChannels.STDOUT, data: "Terminal will auto-close in 15 seconds ..." });
+          setTimeout(() => this.exit(), 15 * 1000);
+        } else {
+          this.exit();
+        }
       }
     });
 
     this.websocket
-      .on("message", (data: string) => {
+      .on("message", (data: string | Uint8Array) => {
         if (!this.running) {
-          return;
+          return void logger.debug(`[SHELL-SESSION]: received message from ${this.terminalId}, but shellProcess isn't running`);
         }
 
-        const message = Buffer.from(data.slice(1, data.length), "base64").toString();
+        if (typeof data === "string") {
+          return void logger.silly(`[SHELL-SESSION]: Received message from ${this.terminalId}`, { data });
+        }
 
-        switch (data[0]) {
-          case "0":
-            shellProcess.write(message);
-            break;
-          case "4":
-            const { Width, Height } = JSON.parse(message);
+        try {
+          const message: TerminalMessage = deserialize(data);
 
-            shellProcess.resize(Width, Height);
-            break;
+          switch (message.type) {
+            case TerminalChannels.STDIN:
+              shellProcess.write(message.data);
+              break;
+            case TerminalChannels.RESIZE:
+              shellProcess.resize(message.data.width, message.data.height);
+              break;
+            default:
+              logger.warn(`[SHELL-SESSION]: unknown or unhandleable message type for ${this.terminalId}`, message);
+              break;
+          }
+        } catch (error) {
+          logger.error(`[SHELL-SESSION]: failed to handle message for ${this.terminalId}`, error);
         }
       })
-      .on("close", (code) => {
-        logger.debug(`[SHELL-SESSION]: websocket for ${this.terminalId} closed with code=${code}`);
+      .on("close", code => {
+        logger.info(`[SHELL-SESSION]: websocket for ${this.terminalId} closed with code=${WebSocketCloseEvent[code]}(${code})`, { cluster: this.cluster.getMeta() });
 
-        if (this.running && code !== WebSocketCloseEvent.AbnormalClosure) {
-          // This code is the one that gets sent when the network is turned off
-          try {
-            logger.info(`[SHELL-SESSION]: Killing shell process for ${this.terminalId}`);
-            process.kill(shellProcess.pid);
-            ShellSession.processes.delete(this.terminalId);
-          } catch (e) {
-          }
+        const stopShellSession = this.running
+          && (
+            (
+              code !== WebSocketCloseEvent.AbnormalClosure
+              && code !== WebSocketCloseEvent.GoingAway
+            )
+            || this.cluster.disconnected
+          );
+
+        if (stopShellSession) {
           this.running = false;
+
+          try {
+            logger.info(`[SHELL-SESSION]: Killing shell process (pid=${shellProcess.pid}) for ${this.terminalId}`);
+            shellProcess.kill();
+            ShellSession.processes.delete(this.terminalId);
+          } catch (error) {
+            logger.warn(`[SHELL-SESSION]: failed to kill shell process (pid=${shellProcess.pid}) for ${this.terminalId}`, error);
+          }
         }
       });
 
@@ -300,8 +340,4 @@ export abstract class ShellSession {
       this.websocket.close(code);
     }
   }
-
-  protected sendResponse(msg: string) {
-    this.websocket.send(`1${Buffer.from(msg).toString("base64")}`);
-  }
 }