Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Stop using node-pty in Electron renderer process #7458

Closed
wants to merge 11 commits into from
8 changes: 3 additions & 5 deletions packages/core/src/main/spawn-electron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {

import windowDefaults, { popupWindowDefaults } from '../webapp/defaults'
import ISubwindowPrefs from '../models/SubwindowPrefs'
import { webpackPath } from '../plugins/path'

/**
* Keep a global reference of the window object, if you don't, the window will
Expand Down Expand Up @@ -442,10 +443,10 @@ export function createWindow(
debug('invoke', message)

try {
const mod = await import(message.module)
const mod = await import('@kui-shell/plugin-' + webpackPath(message.module) + '/dist/index.js')
debug('invoke got module')

const returnValue = await mod[message.main || 'main'](message.args)
const returnValue = await mod[message.main || 'main'](message.args, event.sender)
debug('invoke got returnValue', returnValue)

event.sender.send(
Expand Down Expand Up @@ -600,9 +601,6 @@ export async function initElectron(
debug('loading electron')
const Electron = await import('electron')
app = Electron.app
if (app) {
app.allowRendererProcessReuse = false
}
}

// deal with multiple processes
Expand Down
77 changes: 42 additions & 35 deletions packages/test/src/api/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,46 @@ const prepareElectron = (popup: string[]) => {
return new Application(opts)
}

/** Add app.client commands */
function addCommands(ctx: ISuite) {
// add an isActive command; isFocused is not what we want
if (ctx.app && ctx.app.client) {
// ref: https://github.com/webdriverio/webdriverio/issues/1362#issuecomment-224042781
ctx.app.client.addCommand('isActive', selector => {
return ctx.app.client.execute(selector => {
const focused = document.activeElement

if (!focused || focused === document.body) {
return false
} else if (document.querySelector) {
return document.querySelector(selector) === focused
}

return false
}, selector)
})
}
}

/** restart the app */
export const restart = async (ctx: ISuite) => {
try {
await ctx.app.restart()
addCommands(ctx)
} catch (err) {
const errorIsNavigatedError: boolean =
err.message.includes('Inspected target navigated or closed') ||
err.message.includes('cannot determine loading status') ||
err.message.includes('Inspected target navigated or closed')

if (!errorIsNavigatedError) {
throw err
}
}

return CLI.waitForSession(ctx)
}

/** reload the app */
export const refresh = async (ctx: ISuite, wait = true, clean = false) => {
try {
Expand Down Expand Up @@ -319,23 +359,8 @@ export const before = (ctx: ISuite, options?: BeforeOptions): HookFunction => {
ctx.timeout(process.env.TIMEOUT || 60000)
await start()

// add an isActive command; isFocused is not what we want
if (ctx.app && ctx.app.client) {
// ref: https://github.com/webdriverio/webdriverio/issues/1362#issuecomment-224042781
ctx.app.client.addCommand('isActive', selector => {
return ctx.app.client.execute(selector => {
const focused = document.activeElement

if (!focused || focused === document.body) {
return false
} else if (document.querySelector) {
return document.querySelector(selector) === focused
}

return false
}, selector)
})
}
// add app.client commands
addCommands(ctx)

// see https://github.com/electron-userland/spectron/issues/763
// and https://github.com/webdriverio/webdriverio/issues/6092
Expand Down Expand Up @@ -505,24 +530,6 @@ export const oops = (ctx: ISuite, wait = false) => async (err: Error) => {
// throw err
}

/** restart the app */
export const restart = async (ctx: ISuite) => {
try {
await ctx.app.restart()
} catch (err) {
const errorIsNavigatedError: boolean =
err.message.includes('Inspected target navigated or closed') ||
err.message.includes('cannot determine loading status') ||
err.message.includes('Inspected target navigated or closed')

if (!errorIsNavigatedError) {
throw err
}
}

return CLI.waitForSession(ctx)
}

/** only execute the test in local */
export const localIt = (msg: string, func: Func) => {
if (process.env.MOCHA_RUN_TARGET !== 'webpack') return it(msg, func)
Expand Down
2 changes: 2 additions & 0 deletions plugins/plugin-bash-like/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,5 @@ export { StdioChannelWebsocketSide } from './pty/stdio-channel'
export { getSessionForTab } from './pty/session'
export { dispatchToShell as doExecWithPty, doExecWithStdoutViaPty } from './lib/cmds/catchall'
export { getTabState } from './tab-state'

export { initMainPty } from './pty/electron-main-channel'
30 changes: 18 additions & 12 deletions plugins/plugin-bash-like/src/pty/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ import Options from './options'
import ChannelId from './channel-id'
import { cleanupTerminalAfterTermination } from './util'
import { getChannelForTab, setChannelForTab, invalidateSession } from './session'
import { Channel, InProcessChannel, WebViewChannelRendererSide } from './channel'
import { Channel, WebViewChannelRendererSide } from './channel'
import ElectronRendererSideChannel from './electron-main-channel'

const debug = Debug('plugins/bash-like/pty/client')

Expand Down Expand Up @@ -682,7 +683,7 @@ const injectFont = (terminal: XTerminal, flush = false) => {
}
}

type ChannelFactory = (tab: Tab) => Promise<Channel>
type ChannelFactory = (tab: Tab, execUUID: string) => Promise<Channel>

/**
* Create a websocket channel to a remote bash
Expand Down Expand Up @@ -714,9 +715,9 @@ const remoteChannelFactory: ChannelFactory = async (tab: Tab) => {
}
}

const electronChannelFactory: ChannelFactory = async () => {
const channel = new InProcessChannel()
channel.init()
const electronChannelFactory: ChannelFactory = async (tab: Tab, execUUID: string) => {
const channel = new ElectronRendererSideChannel()
await channel.init(execUUID)
return channel
}

Expand Down Expand Up @@ -778,21 +779,26 @@ const getOrCreateChannel = async (
// here by ensure that any `await` comes *after* grabbing a
// reference to the channelFactory promise. see
// https://github.com/kubernetes-sigs/kui/issues/7465
const ws = await setChannelForTab(tab, channelFactory(tab))
const ws = await setChannelForTab(tab, channelFactory(tab, uuid))

// when the websocket is ready, handle any queued input; only then
// do we focus the terminal (till then, the CLI module will handle
// queuing, and read out the value via disableInputQueueing()
ws.on('open', () => doExec(ws))

// when the websocket has closed, notify the user
const onClose = function(evt) {
debug('channel has closed', evt.target, uuid)
ws.removeEventListener('close', onClose)
debug('attempting to reestablish connection, because the tab is still open')
invalidateSession(tab)

if (inBrowser()) {
const onClose = function(evt) {
debug('channel has closed', evt.target, uuid)
ws.removeEventListener('close', onClose)
if (!tab.state.closed) {
debug('attempting to reestablish connection, because the tab is still open')
invalidateSession(tab)
}
}
ws.on('close', onClose)
}
ws.on('close', onClose)

return ws
} else {
Expand Down
165 changes: 165 additions & 0 deletions plugins/plugin-bash-like/src/pty/electron-main-channel.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
/*
* Copyright 2021 The Kubernetes Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* This file defines a channel between the Electron renderer process
* and Electron main process.
*
*/

import { EventEmitter } from 'events'
import { IpcMain, IpcRenderer, WebContents } from 'electron'

import { Channel, ReadyState } from './channel'
import { onConnection, disableBashSessions } from './server'

interface Args {
execUUID: string
}

function messageChannelFor(execUUID: string) {
return `/plugin-bash-like/pty/message/${execUUID}`
}

class ElectronMainSideChannel extends EventEmitter implements Channel {
/** is the channel alive? */
public readonly isAlive = true

public readonly readyState = ReadyState.OPEN

private ipcMain: IpcMain
private otherSide: WebContents

private messageChannel: string

private handleMessage = (_, msg: string) => {
this.emit('message', msg)
}

public async init(args: Args, otherSide: WebContents) {
const { ipcMain } = await import('electron')
this.ipcMain = ipcMain
this.otherSide = otherSide

this.messageChannel = messageChannelFor(args.execUUID)
ipcMain.on(this.messageChannel, this.handleMessage)

await disableBashSessions()
await onConnection(() => {
try {
this.ipcMain.off(this.messageChannel, this.handleMessage)
} catch (err) {
console.error('Error turning off PTY events', err)
}
})(this)
}

/** Forcibly close the channel */
// eslint-disable-next-line @typescript-eslint/no-empty-function
public close() {}

/** Send a message over the channel */
public send(msg: string | Buffer) {
try {
if (!this.otherSide.isDestroyed()) {
this.otherSide.send(this.messageChannel, msg)
}
} catch (err) {
console.error('Error sending PTY output to renderer', err)
}
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
public removeEventListener(eventType: string, handler: any) {
this.off(eventType, handler)
}
}

export default class ElectronRendererSideChannel extends EventEmitter implements Channel {
/** is the channel alive? */
public readonly isAlive = true

public readonly readyState = ReadyState.OPEN

private ipcRenderer: IpcRenderer
private messageChannel: string

private handleMessage = (_, msg: string) => {
this.emit('message', msg)
}

public async init(execUUID: string) {
const { ipcRenderer } = await import('electron')
this.ipcRenderer = ipcRenderer
console.error('renderer side init', execUUID)

this.messageChannel = messageChannelFor(execUUID)

window.addEventListener('beforeunload', () => {
this.send(JSON.stringify({ type: 'kill', uuid: execUUID }))
this.ipcRenderer.off(this.messageChannel, this.handleMessage)
})

ipcRenderer.once(`/exec/response/${execUUID}`, (_, _msg: string) => {
const msg = JSON.parse(_msg)
console.error('renderer side init exec response', msg)
if (msg.returnValue === true) {
this.ipcRenderer.on(this.messageChannel, this.handleMessage)
this.emit('open')
}
})

ipcRenderer.send(
'/exec/invoke',
JSON.stringify({
module: 'plugin-bash-like',
main: 'initMainPty',
hash: execUUID,
args: {
execUUID
}
})
)
}

/** Forcibly close the channel */
// eslint-disable-next-line @typescript-eslint/no-empty-function
public close() {}

/** Send a message over the channel */
public send(msg: string | Buffer) {
try {
this.ipcRenderer.send(this.messageChannel, msg.toString())
} catch (err) {
console.error('Error sending PTY message to main process', err)
}
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
public removeEventListener(eventType: string, handler: any) {
this.off(eventType, handler)
}
}

export async function initMainPty(args: Args, otherSide: WebContents) {
try {
await new ElectronMainSideChannel().init(args, otherSide)
return true
} catch (err) {
console.error('Error starting up pty', err)
return false
}
}
2 changes: 1 addition & 1 deletion plugins/plugin-bash-like/src/pty/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ export const onConnection = (exitNow: ExitHandler, uid?: number, gid?: number) =
shell = undefined
if (msg.uuid) delete shells[msg.uuid]
ws.send(JSON.stringify({ type: 'exit', exitCode, uuid: msg.uuid }))
// exitNow(exitCode)
exitNow(exitCode)
})

ws.send(JSON.stringify({ type: 'state', state: 'ready', uuid: msg.uuid }))
Expand Down
2 changes: 1 addition & 1 deletion plugins/plugin-bash-like/src/pty/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export function getChannelForTab(tab: Tab): Promise<Channel> {
if (_exiting) {
// prevent any stagglers re-establishing a channel
throw new Error('Exiting')
} else {
} else if (inBrowser()) {
return _singleChannel
}
}
Expand Down
4 changes: 3 additions & 1 deletion plugins/plugin-bash-like/src/pty/stdio-channel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,13 @@ export class StdioChannelWebsocketSide extends EventEmitter implements Channel {
export class StdioChannelKuiSide extends EventEmitter implements Channel {
public readyState = ReadyState.OPEN

// eslint-disable-next-line @typescript-eslint/no-unused-vars
public async init(onExit: ExitHandler) {
debugK('StdioChannelKuiSide.init')

// await onConnection(await disableBashSessions())(this)
await onConnection(onExit)(this)
// eslint-disable-next-line @typescript-eslint/no-empty-function
await onConnection(() => {})(this)

// leftover helps us manage message chunking/fragmentation:
// https://github.com/IBM/kui/issues/5631
Expand Down
Loading