Skip to content

Commit 03e449b

Browse files
committed
Fix failing to start shell being hidden from user
Signed-off-by: Sebastian Malton <[email protected]>
1 parent dc1f1ab commit 03e449b

File tree

5 files changed

+57
-18
lines changed

5 files changed

+57
-18
lines changed

src/common/terminal/channels.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export enum TerminalChannels {
1010
CONNECTED = "connected",
1111
RESIZE = "resize",
1212
PING = "ping",
13+
ERROR = "error",
1314
}
1415

1516
export type TerminalMessage = {
@@ -28,4 +29,7 @@ export type TerminalMessage = {
2829
};
2930
} | {
3031
type: TerminalChannels.PING;
32+
} | {
33+
type: TerminalChannels.ERROR;
34+
data: string;
3135
};

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ import type { ClusterId } from "../../../common/cluster-types";
1111
import { ShellSession } from "../shell-session";
1212
import type { Kubectl } from "../../kubectl/kubectl";
1313
import { baseBinariesDir } from "../../../common/vars";
14+
import { TerminalChannels } from "../../../common/terminal/channels";
15+
import logger from "../../logger";
1416

1517
export class LocalShellSession extends ShellSession {
1618
ShellType = "shell";
@@ -36,7 +38,13 @@ export class LocalShellSession extends ShellSession {
3638
const shell = env.PTYSHELL;
3739

3840
if (!shell) {
39-
throw new Error("PTYSHELL is not defined with the environment");
41+
this.send({
42+
type: TerminalChannels.ERROR,
43+
data: "PTYSHELL is not defined with the environment",
44+
});
45+
logger.warn(`[LOCAL-SHELL-SESSION]: PTYSHELL env var is not defined for ${this.terminalId}`);
46+
47+
return;
4048
}
4149

4250
const args = await this.getShellArgs(shell);

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

Lines changed: 35 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -133,23 +133,35 @@ export abstract class ShellSession {
133133

134134
protected abstract get cwd(): string | undefined;
135135

136-
protected ensureShellProcess(shell: string, args: string[], env: Record<string, string | undefined>, cwd: string): { shellProcess: pty.IPty; resume: boolean } {
137-
const resume = ShellSession.processes.has(this.terminalId);
138-
const shellProcess = getOrInsertWith(ShellSession.processes, this.terminalId, () => (
139-
pty.spawn(shell, args, {
140-
rows: 30,
141-
cols: 80,
142-
cwd,
143-
env: env as Record<string, string>,
144-
name: "xterm-256color",
145-
// TODO: Something else is broken here so we need to force the use of winPty on windows
146-
useConpty: false,
147-
})
148-
));
149-
150-
logger.info(`[SHELL-SESSION]: PTY for ${this.terminalId} is ${resume ? "resumed" : "started"} with PID=${shellProcess.pid}`);
136+
protected ensureShellProcess(shell: string, args: string[], env: Record<string, string | undefined>, cwd: string): { shellProcess: pty.IPty; resume: boolean } | null {
137+
try {
138+
const resume = ShellSession.processes.has(this.terminalId);
139+
140+
throw new Error("failed");
141+
const shellProcess = getOrInsertWith(ShellSession.processes, this.terminalId, () => (
142+
pty.spawn(shell, args, {
143+
rows: 30,
144+
cols: 80,
145+
cwd,
146+
env: env as Record<string, string>,
147+
name: "xterm-256color",
148+
// TODO: Something else is broken here so we need to force the use of winPty on windows
149+
useConpty: false,
150+
})
151+
));
152+
153+
logger.info(`[SHELL-SESSION]: PTY for ${this.terminalId} is ${resume ? "resumed" : "started"} with PID=${shellProcess.pid}`);
154+
155+
return { shellProcess, resume };
156+
} catch (error) {
157+
this.send({
158+
type: TerminalChannels.ERROR,
159+
data: `Failed to start shell (${shell}): ${error}`,
160+
});
161+
logger.warn(`[SHELL-SESSION]: Failed to start PTY for ${this.terminalId}: ${error}`, { shell });
151162

152-
return { shellProcess, resume };
163+
return null;
164+
}
153165
}
154166

155167
constructor(protected readonly kubectl: Kubectl, protected readonly websocket: WebSocket, protected readonly cluster: Cluster, terminalId: string) {
@@ -205,7 +217,13 @@ export abstract class ShellSession {
205217

206218
protected async openShellProcess(shell: string, args: string[], env: Record<string, string | undefined>) {
207219
const cwd = await this.getCwd(env);
208-
const { shellProcess, resume } = this.ensureShellProcess(shell, args, env, cwd);
220+
const ensured = this.ensureShellProcess(shell, args, env, cwd);
221+
222+
if (!ensured) {
223+
return;
224+
}
225+
226+
const { shellProcess, resume } = ensured;
209227

210228
if (resume) {
211229
this.send({ type: TerminalChannels.CONNECTED });

src/renderer/api/terminal-api.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export interface TerminalApiQuery extends Record<string, string | undefined> {
3434
export interface TerminalEvents extends WebSocketEvents {
3535
ready: () => void;
3636
connected: () => void;
37+
error: (error: string) => void;
3738
}
3839

3940
export interface TerminalApiDependencies {
@@ -144,6 +145,9 @@ export class TerminalApi extends WebSocketApi<TerminalEvents> {
144145
case TerminalChannels.CONNECTED:
145146
this.emit("connected");
146147
break;
148+
case TerminalChannels.ERROR:
149+
this.emit("error", message.data);
150+
break;
147151
default:
148152
logger.warn(`[TERMINAL-API]: unknown or unhandleable message type`, message);
149153
break;

src/renderer/components/dock/terminal/terminal.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ export class Terminal {
106106
this.api.once("ready", clearOnce);
107107
this.api.once("connected", clearOnce);
108108
this.api.on("data", this.onApiData);
109+
this.api.on("error", this.onApiError);
109110
window.addEventListener("resize", this.onResize);
110111

111112
this.disposer.push(
@@ -153,6 +154,10 @@ export class Terminal {
153154
this.xterm.write(data);
154155
};
155156

157+
onApiError = (data: string) => {
158+
this.xterm.writeln(data);
159+
};
160+
156161
onData = (data: string) => {
157162
if (!this.api.isReady) return;
158163
this.api.sendMessage({

0 commit comments

Comments
 (0)