Cleanup Terminal implementation

- SpawningPool is now part of the template HTML instead of being added
  by modifying the DOM

- preloading of the fonts now uses async import

- factor out of setTheme the color name coversion, switch to fromEntries

- remove try/catch in Terminal.fit() as we are now using xterm 4.10

Signed-off-by: Sebastian Malton <sebastian@malton.name>

src/main/node-shell-session.ts

@@ -1,52 +1,37 @@
 import * as WebSocket from "ws";
-import * as pty from "node-pty";
 import { ShellSession } from "./shell-session";
 import { v4 as uuid } from "uuid";
 import * as k8s from "@kubernetes/client-node";
 import { KubeConfig } from "@kubernetes/client-node";
 import { Cluster } from "./cluster";
 import logger from "./logger";
-import { appEventBus } from "../common/event-bus";
 
 export class NodeShellSession extends ShellSession {
-  protected nodeName: string;
+  protected readonly EventName = "node-shell";
-  protected podId: string;
+  protected podId = `node-shell-${uuid()}`;
   protected kc: KubeConfig;
 
-  constructor(socket: WebSocket, cluster: Cluster, nodeName: string) {
+  constructor(socket: WebSocket, cluster: Cluster, protected nodeName: string) {
     super(socket, cluster);
-    this.nodeName = nodeName;
-    this.podId = `node-shell-${uuid()}`;
     this.kc = cluster.getProxyKubeconfig();
   }
 
   public async open() {
-    const shell = await this.kubectl.getPath();
-    let args = [];
-
     if (this.createNodeShellPod(this.podId, this.nodeName)) {
       await this.waitForRunningPod(this.podId).catch(() => {
         this.exit(1001);
       });
     }
-    args = ["exec", "-i", "-t", "-n", "kube-system", this.podId, "--", "sh", "-c", "((clear && bash) || (clear && ash) || (clear && sh))"];
 
-    const shellEnv = await this.getCachedShellEnv();
+    return this.rawOpen();
+  }
 
-    this.shellProcess = pty.spawn(shell, args, {
+  protected async getShell(): Promise<string> {
-      cols: 80,
+    return this.kubectl.getPath();
-      cwd: this.cwd() || shellEnv["HOME"],
+  }
-      env: shellEnv,
-      name: "xterm-256color",
-      rows: 30,
-    });
-    this.running = true;
-    this.pipeStdout();
-    this.pipeStdin();
-    this.closeWebsocketOnProcessExit();
-    this.exitProcessOnWebsocketClose();
 
-    appEventBus.emit({name: "node-shell", action: "open"});
+  protected async getShellArgs(): Promise<string[]> {
+    return ["exec", "-i", "-t", "-n", "kube-system", this.podId, "--", "sh", "-c", "((clear && bash) || (clear && ash) || (clear && sh))"];
   }
 
   protected exit(code = 1000) {

src/main/shell-session.ts

@@ -6,16 +6,29 @@ import shellEnv from "shell-env";
 import { app } from "electron";
 import { Kubectl } from "./kubectl";
 import { Cluster } from "./cluster";
-import { ClusterPreferences } from "../common/cluster-store";
+import { ClusterId, ClusterPreferences } from "../common/cluster-store";
 import { helmCli } from "./helm/helm-cli";
 import { isWindows } from "../common/vars";
 import { appEventBus } from "../common/event-bus";
 import { userStore } from "../common/user-store";
+import { autobind } from "../common/utils";
+
+/**
+ * Joins all the non-empty elements of `parts` using `sep` between each element
+ * @param parts the potential elements for the new ENV var multi-element value
+ * @param sep The separator to join the elements together
+ */
+function joinEnvParts(parts: (string | null | undefined)[], sep: string): string {
+  return parts.filter(Boolean).join(sep);
+}
+
+type EnvVarMap = Record<string, string>;
 
 export class ShellSession extends EventEmitter {
-  static shellEnvs: Map<string, any> = new Map();
+  protected readonly EventName: string = "shell";
+
+  static shellEnvs: Map<ClusterId, EnvVarMap> = new Map();
 
-  protected websocket: WebSocket;
   protected shellProcess: pty.IPty;
   protected kubeconfigPath: string;
   protected nodeShellPod: string;
@@ -25,30 +38,29 @@ export class ShellSession extends EventEmitter {
   protected helmBinDir: string;
   protected preferences: ClusterPreferences;
   protected running = false;
-  protected clusterId: string;
+  protected clusterId: ClusterId;
 
-  constructor(socket: WebSocket, cluster: Cluster) {
+  protected cwd(env: EnvVarMap): string {
+    return this.preferences?.terminalCWD || env.HOME;
+  }
+
+  constructor(protected websocket: WebSocket, cluster: Cluster) {
     super();
-    this.websocket = socket;
     this.kubeconfigPath =  cluster.getProxyKubeconfigPath();
     this.kubectl = new Kubectl(cluster.version);
     this.preferences = cluster.preferences || {};
     this.clusterId = cluster.id;
   }
 
-  public async open() {
+  protected async rawOpen() {
-    this.kubectlBinDir = await this.kubectl.binDir();
-    const pathFromPreferences = userStore.preferences.kubectlBinariesPath || this.kubectl.getBundledPath();
-
-    this.kubectlPathDir = userStore.preferences.downloadKubectlBinaries ? this.kubectlBinDir : path.dirname(pathFromPreferences);
-    this.helmBinDir = helmCli.getBinaryDir();
     const env = await this.getCachedShellEnv();
-    const shell = env.PTYSHELL;
+    const shell = await this.getShell(env);
     const args = await this.getShellArgs(shell);
+    const cwd = this.cwd(env);
 
     this.shellProcess = pty.spawn(shell, args, {
       cols: 80,
-      cwd: this.cwd() || env.HOME,
+      cwd,
       env,
       name: "xterm-256color",
       rows: 30,
@@ -60,18 +72,24 @@ export class ShellSession extends EventEmitter {
     this.closeWebsocketOnProcessExit();
     this.exitProcessOnWebsocketClose();
 
-    appEventBus.emit({name: "shell", action: "open"});
+    appEventBus.emit({ name: this.EventName, action: "open" });
   }
 
-  protected cwd(): string {
+  public async open() {
-    if(!this.preferences || !this.preferences.terminalCWD || this.preferences.terminalCWD === "") {
+    this.kubectlBinDir = await this.kubectl.binDir();
-      return null;
+    const pathFromPreferences = userStore.preferences.kubectlBinariesPath || this.kubectl.getBundledPath();
-    }
 
-    return this.preferences.terminalCWD;
+    this.kubectlPathDir = userStore.preferences.downloadKubectlBinaries ? this.kubectlBinDir : path.dirname(pathFromPreferences);
+    this.helmBinDir = helmCli.getBinaryDir();
+
+    return this.rawOpen();
   }
 
-  protected async getShellArgs(shell: string): Promise<Array<string>> {
+  protected async getShell(env: EnvVarMap): Promise<string> {
+    return env.PTYSHELL;
+  }
+
+  protected async getShellArgs(shell: string): Promise<string[]> {
     switch(path.basename(shell)) {
       case "powershell.exe":
         return ["-NoExit", "-command", `& {Set-Location $Env:USERPROFILE; $Env:PATH="${this.helmBinDir};${this.kubectlPathDir};$Env:PATH"}`];
@@ -86,43 +104,33 @@ export class ShellSession extends EventEmitter {
     }
   }
 
-  protected async getCachedShellEnv() {
+  protected async getCachedShellEnv(): Promise<EnvVarMap> {
-    let env = ShellSession.shellEnvs.get(this.clusterId);
+    if (!ShellSession.shellEnvs.has(this.clusterId)) {
-
+      ShellSession.shellEnvs.set(this.clusterId, await this.getShellEnv());
-    if (!env) {
-      env = await this.getShellEnv();
-      ShellSession.shellEnvs.set(this.clusterId, env);
     } else {
       // refresh env in the background
-      this.getShellEnv().then((shellEnv: any) => {
+      this.getShellEnv()
-        ShellSession.shellEnvs.set(this.clusterId, shellEnv);
+        .then(shellEnv => {
-      });
+          ShellSession.shellEnvs.set(this.clusterId, shellEnv);
+        });
     }
 
-    return env;
+    return ShellSession.shellEnvs.get(this.clusterId);
   }
 
-  protected async getShellEnv() {
+  protected async getShellEnv(): Promise<EnvVarMap> {
     const env = JSON.parse(JSON.stringify(await shellEnv()));
-    const pathStr = [this.kubectlBinDir, this.helmBinDir, process.env.PATH].join(path.delimiter);
+    const pathStr = joinEnvParts([this.kubectlBinDir, this.helmBinDir, process.env.PATH], path.delimiter);
 
-    if(isWindows) {
+    if (isWindows) {
       env["SystemRoot"] = process.env.SystemRoot;
       env["PTYSHELL"] = process.env.SHELL || "powershell.exe";
       env["PATH"] = pathStr;
       env["LENS_SESSION"] = "true";
-      const lensWslEnv = "KUBECONFIG/up:LENS_SESSION/u";
+      env["WSLENV"] = joinEnvParts([env["WSLENV"], "KUBECONFIG/up:LENS_SESSION/u"], ":");
-
-      if (process.env.WSLENV != undefined) {
-        env["WSLENV"] = `${process.env["WSLENV"]}:${lensWslEnv}`;
-      } else {
-        env["WSLENV"] = lensWslEnv;
-      }
-    } else if(typeof(process.env.SHELL) != "undefined") {
-      env["PTYSHELL"] = process.env.SHELL;
-      env["PATH"] = pathStr;
     } else {
-      env["PTYSHELL"] = ""; // blank runs the system default shell
+      env["PTYSHELL"] = process.env.SHELL ?? ""; // blank runs the system default shell
+      env["PATH"] = pathStr;
     }
 
     if(path.basename(env["PTYSHELL"]) === "zsh") {
@@ -139,22 +147,16 @@ export class ShellSession extends EventEmitter {
     if (this.preferences.httpsProxy) {
       env["HTTPS_PROXY"] = this.preferences.httpsProxy;
     }
-    const no_proxy = ["localhost", "127.0.0.1", env["NO_PROXY"]];
 
-    env["NO_PROXY"] = no_proxy.filter(address => !!address).join();
+    env["WSLENV"] = joinEnvParts(["localhost", "127.0.0.1", env["NO_PROXY"]], ",");
+    delete env["DEBUG"];
 
-    if (env.DEBUG) { // do not pass debug option to bash
+    return env;
-      delete env["DEBUG"];
-    }
-
-    return(env);
   }
 
   protected pipeStdout() {
     // send shell output to websocket
-    this.shellProcess.onData(((data: string) => {
+    this.shellProcess.onData(this.sendResponse);
-      this.sendResponse(data);
-    }));
   }
 
   protected pipeStdin() {
@@ -169,9 +171,9 @@ export class ShellSession extends EventEmitter {
           this.shellProcess.write(message);
           break;
         case "4":
-          const resizeMsgObj = JSON.parse(message);
+          const { Width, Height } = JSON.parse(message);
 
-          this.shellProcess.resize(resizeMsgObj["Width"], resizeMsgObj["Height"]);
+          this.shellProcess.resize(Width, Height);
           break;
         case "9":
           this.emit("newToken", message);
@@ -188,12 +190,12 @@ export class ShellSession extends EventEmitter {
   protected closeWebsocketOnProcessExit() {
     this.shellProcess.onExit(({ exitCode }) => {
       this.running = false;
-      let timeout = 0;
+      const timeout = exitCode > 0 ? 15 * 1000 : 0;
 
       if (exitCode > 0) {
         this.sendResponse("Terminal will auto-close in 15 seconds ...");
-        timeout = 15*1000;
       }
+
       setTimeout(() => {
         this.exit();
       }, timeout);
@@ -207,7 +209,7 @@ export class ShellSession extends EventEmitter {
   }
 
   protected killShellProcess(){
-    if(this.running) {
+    if (this.running) {
       // On Windows we need to kill the shell process by pid, since Lens won't respond after a while if using `this.shellProcess.kill()`
       if (isWindows) {
         try {
@@ -221,6 +223,7 @@ export class ShellSession extends EventEmitter {
     }
   }
 
+  @autobind()
   protected sendResponse(msg: string) {
     this.websocket.send(`1${Buffer.from(msg).toString("base64")}`);
   }

src/renderer/components/app.scss

@@ -62,6 +62,15 @@ html, body {
   overflow: hidden;
 }
 
+#terminal-init {
+  position: absolute;
+  top: 0;
+  left: 0;
+  height: 0;
+  visibility: hidden;
+  overflow: hidden;
+}
+
 #app {
   height: 100%;
   min-height: 100%;

src/renderer/components/dock/terminal.ts

@@ -8,21 +8,11 @@ import { themeStore } from "../../theme.store";
 import { autobind } from "../../utils";
 
 export class Terminal {
-  static spawningPool: HTMLElement;
+  private static readonly ColorPrefix = "terminal";
-
+  private static readonly SpawningPool = document.getElementById("terminal-init");
-  static init() {
-    // terminal element must be in DOM before attaching via xterm.open(elem)
-    // https://xtermjs.org/docs/api/terminal/classes/terminal/#open
-    const pool = document.createElement("div");
-
-    pool.className = "terminal-init";
-    pool.style.cssText = "position: absolute; top: 0; left: 0; height: 0; visibility: hidden; overflow: hidden";
-    document.body.appendChild(pool);
-    Terminal.spawningPool = pool;
-  }
 
   static async preloadFonts() {
-    const fontPath = require("../fonts/roboto-mono-nerd.ttf").default; // eslint-disable-line @typescript-eslint/no-var-requires
+    const { default: fontPath } = await import("../fonts/roboto-mono-nerd.ttf");
     const fontFace = new FontFace("RobotoMono", `url(${fontPath})`);
 
     await fontFace.load();
@@ -34,23 +24,24 @@ export class Terminal {
   public scrollPos = 0;
   public disposers: Function[] = [];
 
+  /**
+   * Removes the ColorPrefix from the start of the string and makes the final
+   * string camelcase.
+   * @param src a color theme entry that starts with `Terminal.ColorPrefix`
+   */
+  private static toColorName(src: string): string {
+    return src.charAt(Terminal.ColorPrefix.length).toLowerCase() + src.substring(Terminal.ColorPrefix.length + 1);
+  }
+
   @autobind()
   protected setTheme(colors: Record<string, string>) {
     // Replacing keys stored in styles to format accepted by terminal
     // E.g. terminalBrightBlack -> brightBlack
-    const colorPrefix = "terminal";
+    const terminalColorEntries = Object.entries(colors)
-    const terminalColors = Object.entries(colors)
+      .filter(([name]) => name.startsWith(Terminal.ColorPrefix))
-      .filter(([name]) => name.startsWith(colorPrefix))
+      .map(([name, color]) => [Terminal.toColorName(name), color]);
-      .reduce<any>((colors, [name, color]) => {
-        const colorName = name.split("").slice(colorPrefix.length);
 
-        colorName[0] = colorName[0].toLowerCase();
+    this.xterm.setOption("theme", Object.fromEntries(terminalColorEntries));
-        colors[colorName.join("")] = color;
-
-        return colors;
-      }, {});
-
-    this.xterm.setOption("theme", terminalColors);
   }
 
   get elem() {
@@ -77,7 +68,7 @@ export class Terminal {
   }
 
   detach() {
-    Terminal.spawningPool.appendChild(this.elem);
+    Terminal.SpawningPool.appendChild(this.elem);
   }
 
   async init() {
@@ -95,7 +86,7 @@ export class Terminal {
     this.fitAddon = new FitAddon();
     this.xterm.loadAddon(this.fitAddon);
 
-    this.xterm.open(Terminal.spawningPool);
+    this.xterm.open(Terminal.SpawningPool);
     this.xterm.registerLinkMatcher(/https?:\/\/[^\s]+/i, this.onClickLink);
     this.xterm.attachCustomKeyEventHandler(this.keyHandler);
 
@@ -131,16 +122,10 @@ export class Terminal {
     // Since this function is debounced we need to read this value as late as possible
     if (!this.isActive) return;
 
-    try {
+    this.fitAddon.fit();
-      this.fitAddon.fit();
+    const { cols, rows } = this.xterm;
-      const { cols, rows } = this.xterm;
 
-      this.api.sendTerminalSize(cols, rows);
+    this.api.sendTerminalSize(cols, rows);
-    } catch(error) {
-      console.error(error);
-
-      return; // see https://github.com/lensapp/lens/issues/1891
-    }
   };
 
   fitLazy = debounce(this.fit, 250);
@@ -207,5 +192,3 @@ export class Terminal {
     return true;
   };
 }
-
-Terminal.init();

src/renderer/template.html

@@ -7,6 +7,7 @@
 <body>
 
 <div id="app"></div>
+<div id="terminal-init"></div>
 
 </body>
-</html>
+</html>