Skip to content

Commit ecffed8

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

File tree

5 files changed

+55
-18
lines changed

5 files changed

+55
-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: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import type { ModifyTerminalShellEnv } from "../shell-env-modifier/modify-termin
1010
import type { JoinPaths } from "../../../common/path/join-paths.injectable";
1111
import type { GetDirnameOfPath } from "../../../common/path/get-dirname.injectable";
1212
import type { GetBasenameOfPath } from "../../../common/path/get-basename.injectable";
13+
import { TerminalChannels } from "../../../common/terminal/channels";
1314

1415
export interface LocalShellSessionDependencies extends ShellSessionDependencies {
1516
readonly directoryForBinaries: string;
@@ -44,7 +45,13 @@ export class LocalShellSession extends ShellSession {
4445
const shell = env.PTYSHELL;
4546

4647
if (!shell) {
47-
throw new Error("PTYSHELL is not defined with the environment");
48+
this.send({
49+
type: TerminalChannels.ERROR,
50+
data: "PTYSHELL is not defined with the environment",
51+
});
52+
this.dependencies.logger.warn(`[LOCAL-SHELL-SESSION]: PTYSHELL env var is not defined for ${this.terminalId}`);
53+
54+
return;
4855
}
4956

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

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

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -148,23 +148,34 @@ export abstract class ShellSession {
148148

149149
protected abstract get cwd(): string | undefined;
150150

151-
protected ensureShellProcess(shell: string, args: string[], env: Record<string, string | undefined>, cwd: string): { shellProcess: pty.IPty; resume: boolean } {
152-
const resume = ShellSession.processes.has(this.terminalId);
153-
const shellProcess = getOrInsertWith(ShellSession.processes, this.terminalId, () => (
154-
pty.spawn(shell, args, {
155-
rows: 30,
156-
cols: 80,
157-
cwd,
158-
env: env as Record<string, string>,
159-
name: "xterm-256color",
160-
// TODO: Something else is broken here so we need to force the use of winPty on windows
161-
useConpty: false,
162-
})
163-
));
164-
165-
this.dependencies.logger.info(`[SHELL-SESSION]: PTY for ${this.terminalId} is ${resume ? "resumed" : "started"} with PID=${shellProcess.pid}`);
151+
protected ensureShellProcess(shell: string, args: string[], env: Record<string, string | undefined>, cwd: string): { shellProcess: pty.IPty; resume: boolean } | null {
152+
try {
153+
const resume = ShellSession.processes.has(this.terminalId);
154+
155+
const shellProcess = getOrInsertWith(ShellSession.processes, this.terminalId, () => (
156+
pty.spawn(shell, args, {
157+
rows: 30,
158+
cols: 80,
159+
cwd,
160+
env: env as Record<string, string>,
161+
name: "xterm-256color",
162+
// TODO: Something else is broken here so we need to force the use of winPty on windows
163+
useConpty: false,
164+
})
165+
));
166+
167+
this.dependencies.logger.info(`[SHELL-SESSION]: PTY for ${this.terminalId} is ${resume ? "resumed" : "started"} with PID=${shellProcess.pid}`);
168+
169+
return { shellProcess, resume };
170+
} catch (error) {
171+
this.send({
172+
type: TerminalChannels.ERROR,
173+
data: `Failed to start shell (${shell}): ${error}`,
174+
});
175+
this.dependencies.logger.warn(`[SHELL-SESSION]: Failed to start PTY for ${this.terminalId}: ${error}`, { shell });
166176

167-
return { shellProcess, resume };
177+
return null;
178+
}
168179
}
169180

170181
constructor(protected readonly dependencies: ShellSessionDependencies, { kubectl, websocket, cluster, tabId: terminalId }: ShellSessionArgs) {
@@ -223,7 +234,13 @@ export abstract class ShellSession {
223234

224235
protected async openShellProcess(shell: string, args: string[], env: Record<string, string | undefined>) {
225236
const cwd = await this.getCwd(env);
226-
const { shellProcess, resume } = this.ensureShellProcess(shell, args, env, cwd);
237+
const ensured = this.ensureShellProcess(shell, args, env, cwd);
238+
239+
if (!ensured) {
240+
return;
241+
}
242+
243+
const { shellProcess, resume } = ensured;
227244

228245
if (resume) {
229246
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
@@ -108,6 +108,7 @@ export class Terminal {
108108
this.api.once("ready", clearOnce);
109109
this.api.once("connected", clearOnce);
110110
this.api.on("data", this.onApiData);
111+
this.api.on("error", this.onApiError);
111112
window.addEventListener("resize", this.onResize);
112113

113114
const linkProvider = new LinkProvider(
@@ -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)