diff --git a/app/lib/pty.ts b/app/lib/pty.ts index 140d668b4..2ced71c33 100644 --- a/app/lib/pty.ts +++ b/app/lib/pty.ts @@ -143,8 +143,20 @@ export class PTYManager { init (app: Application): void { ipcMain.on('pty:spawn', (event, ...options) => { const id = uuidv4().toString() - event.returnValue = id - this.ptys[id] = new PTY(id, app, ...options) + try { + this.ptys[id] = new PTY(id, app, ...options) + } catch (error) { + // Spawning fails for reasons the user can act on - an invalid + // working directory being by far the most common one. Reporting + // that back beats taking down the main process with an + // uncaught exception. + const cwd = options[2]?.cwd + event.returnValue = { + error: cwd ? `${error.message} (working directory: ${cwd})` : error.message, + } + return + } + event.returnValue = { id } }) ipcMain.on('pty:exists', (event, id) => { diff --git a/tabby-electron/src/pty.ts b/tabby-electron/src/pty.ts index 2d4e13b02..390da13e0 100644 --- a/tabby-electron/src/pty.ts +++ b/tabby-electron/src/pty.ts @@ -15,8 +15,11 @@ try { export class ElectronPTYInterface extends PTYInterface { async spawn (...options: any[]): Promise { - const id = ipcRenderer.sendSync('pty:spawn', ...options) - return new ElectronPTYProxy(id) + const result = ipcRenderer.sendSync('pty:spawn', ...options) + if (result.error) { + throw new Error(result.error) + } + return new ElectronPTYProxy(result.id) } async restore (id: string): Promise { diff --git a/tabby-local/src/services/terminal.service.ts b/tabby-local/src/services/terminal.service.ts index d1d17aec2..0b8f9603b 100644 --- a/tabby-local/src/services/terminal.service.ts +++ b/tabby-local/src/services/terminal.service.ts @@ -1,8 +1,8 @@ -import * as fsSync from 'fs' import { Injectable } from '@angular/core' import { Logger, LogService, ConfigService, ProfilesService, PartialProfile } from 'tabby-core' import { TerminalTabComponent } from '../components/terminalTab.component' import { LocalProfile } from '../api' +import { isDirectorySync } from '../util' @Injectable({ providedIn: 'root' }) export class TerminalService { @@ -39,8 +39,8 @@ export class TerminalService { cwd = cwd ?? fullProfile.options.cwd - if (cwd && !fsSync.existsSync(cwd)) { - console.warn('Ignoring non-existent CWD:', cwd) + if (cwd && !isDirectorySync(cwd)) { + console.warn('Ignoring invalid CWD:', cwd) cwd = null } diff --git a/tabby-local/src/session.ts b/tabby-local/src/session.ts index 6927e1520..46d02cf6a 100644 --- a/tabby-local/src/session.ts +++ b/tabby-local/src/session.ts @@ -1,10 +1,10 @@ import * as fs from 'mz/fs' -import * as fsSync from 'fs' import { Injector } from '@angular/core' import { HostAppService, ConfigService, WIN_BUILD_CONPTY_SUPPORTED, isWindowsBuild, Platform, BootstrapData, BOOTSTRAP_DATA, LogService } from 'tabby-core' import { BaseSession } from 'tabby-terminal' import { SessionOptions, ChildProcess, PTYInterface, PTYProxy } from './api' import { getEnvironment, substituteEnv } from './environment' +import { isDirectory, isDirectorySync } from './util' const windowsDirectoryRegex = /([a-zA-Z]:[^\:\[\]\?\"\<\>\|]+)/mi @@ -89,21 +89,27 @@ export class Session extends BaseSession { // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing let cwd = options.cwd || process.env.HOME - if (!fsSync.existsSync(cwd!)) { - console.warn('Ignoring non-existent CWD:', cwd) + if (!isDirectorySync(cwd)) { + console.warn('Ignoring invalid CWD:', cwd) cwd = undefined } - pty = await this.ptyInterface.spawn(options.command, options.args, { - name: 'xterm-256color', - cols: options.width ?? 80, - rows: options.height ?? 30, - encoding: null, - cwd, - env: env, - // `1` instead of `true` forces ConPTY even if unstable - useConpty: isWindowsBuild(WIN_BUILD_CONPTY_SUPPORTED) && this.config.store.terminal.useConPTY ? 1 : false, - }) + try { + pty = await this.ptyInterface.spawn(options.command, options.args, { + name: 'xterm-256color', + cols: options.width ?? 80, + rows: options.height ?? 30, + encoding: null, + cwd, + env: env, + // `1` instead of `true` forces ConPTY even if unstable + useConpty: isWindowsBuild(WIN_BUILD_CONPTY_SUPPORTED) && this.config.store.terminal.useConPTY ? 1 : false, + }) + } catch (error) { + this.logger.error('Could not spawn the shell:', error) + this.emitOutput(Buffer.from(`\r\nCould not start ${options.command}:\r\n${error.message}\r\n`)) + return + } this.guessedCWD = cwd ?? null } @@ -219,18 +225,23 @@ export class Session extends BaseSession { // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing cwd = cwd || this.guessedCWD - try { - await fs.access(cwd) - } catch { + if (!await isDirectory(cwd)) { return null } return cwd } - private guessWindowsCWD (data: string) { + private async guessWindowsCWD (data: string): Promise { const match = windowsDirectoryRegex.exec(data) - if (match) { - this.guessedCWD = match[0] + if (!match) { + return + } + // The regex also matches file paths (e.g. an echoed command line + // containing `D:\tools\7z.exe`), which are useless as a CWD and + // break process spawning once inherited by another tab. + const guess = match[0].trim() + if (await isDirectory(guess)) { + this.guessedCWD = guess } } } diff --git a/tabby-local/src/util.ts b/tabby-local/src/util.ts new file mode 100644 index 000000000..0f54d01a1 --- /dev/null +++ b/tabby-local/src/util.ts @@ -0,0 +1,32 @@ +import * as fs from 'mz/fs' +import * as fsSync from 'fs' + +/** + * Returns `true` only if `path` exists *and* is a directory. + * + * A plain existence check is not enough for anything that will be used as a + * process' working directory: `CreateProcessW` fails with `ERROR_DIRECTORY` + * (267) when `lpCurrentDirectory` points at a file. + */ +export async function isDirectory (path: string|null|undefined): Promise { + if (!path) { + return false + } + try { + return (await fs.stat(path)).isDirectory() + } catch { + return false + } +} + +/** Synchronous counterpart of [[isDirectory]]. */ +export function isDirectorySync (path: string|null|undefined): boolean { + if (!path) { + return false + } + try { + return fsSync.statSync(path).isDirectory() + } catch { + return false + } +}