From f78cc36f302c2136be4883de40701f513d707fe1 Mon Sep 17 00:00:00 2001 From: Nick Mitchell Date: Fri, 9 Sep 2022 14:55:26 -0400 Subject: [PATCH] fix: improved debug output and error handling in tray menu code --- .../src/electron-main.ts | 6 +- .../src/tray/events.ts | 13 +++++ .../plugin-kubectl-tray-menu/src/tray/init.ts | 2 +- .../plugin-kubectl-tray-menu/src/tray/main.ts | 29 +++++++--- .../src/tray/menus/contexts/current.ts | 50 ++++++++++------ .../src/tray/menus/contexts/index.ts | 49 +++++++++++----- .../src/tray/menus/namespaces/current.ts | 58 ++++++++++++------- .../src/tray/menus/namespaces/index.ts | 49 +++++++++++----- 8 files changed, 178 insertions(+), 78 deletions(-) diff --git a/plugins/plugin-kubectl-tray-menu/src/electron-main.ts b/plugins/plugin-kubectl-tray-menu/src/electron-main.ts index 673bd2f2d41..acc654a24c5 100644 --- a/plugins/plugin-kubectl-tray-menu/src/electron-main.ts +++ b/plugins/plugin-kubectl-tray-menu/src/electron-main.ts @@ -17,9 +17,9 @@ import { CreateWindowFunction } from '@kui-shell/core' /** - * This logic will be executed in the electron-main process, and is - * called by Kui core in response to the event issued by - * `./tray/renderer`, whenever a new electron window opens. + * [Main Process]: This logic will be executed in the electron-main + * process, and is called by Kui core in response to the event issued + * by `./tray/renderer`, whenever a new electron window opens. */ export async function initTray(args: { command: string }, _: unknown, createWindow: CreateWindowFunction) { if (args.command === '/tray/init') { diff --git a/plugins/plugin-kubectl-tray-menu/src/tray/events.ts b/plugins/plugin-kubectl-tray-menu/src/tray/events.ts index ba91451c3cb..b0dc0e03d10 100644 --- a/plugins/plugin-kubectl-tray-menu/src/tray/events.ts +++ b/plugins/plugin-kubectl-tray-menu/src/tray/events.ts @@ -14,20 +14,33 @@ * limitations under the License. */ +import Debug from 'debug' import { EventEmitter } from 'events' const refreshEvents = new EventEmitter() +/** + * [Main Process] Emit a kubernetes config change event. Called from + * electron-main, which is the touchpoint for calls from the renderer + * process (below). + */ export function emitRefresh() { + Debug('plugin-kubectl-tray-menu/events')('emitRefreshFromMain') refreshEvents.emit('/refresh') } +/** + * [Main Process] This is how tray menu watchers register for + * kubernetes config change event . + */ export function onRefresh(cb: () => void) { refreshEvents.on('/refresh', cb) } +/** [Renderer Process] */ export async function emitRefreshFromRenderer() { try { + Debug('plugin-kubectl-tray-menu/events')('emitRefreshFromRenderer') const { ipcRenderer } = await import('electron') ipcRenderer.send( '/exec/invoke', diff --git a/plugins/plugin-kubectl-tray-menu/src/tray/init.ts b/plugins/plugin-kubectl-tray-menu/src/tray/init.ts index 7e6b604cca6..166b51e1a7b 100644 --- a/plugins/plugin-kubectl-tray-menu/src/tray/init.ts +++ b/plugins/plugin-kubectl-tray-menu/src/tray/init.ts @@ -16,7 +16,7 @@ import { Capabilities } from '@kui-shell/core' -/** Preloader to initialize tray menu */ +/** [Renderer Process] Preloader to initialize tray menu */ export default async function initTray() { if (Capabilities.inElectron() && !process.env.KUI_NO_TRAY_MENU) { const { ipcRenderer } = await import('electron') diff --git a/plugins/plugin-kubectl-tray-menu/src/tray/main.ts b/plugins/plugin-kubectl-tray-menu/src/tray/main.ts index 0a425ab5c5d..bf0c40ce90d 100644 --- a/plugins/plugin-kubectl-tray-menu/src/tray/main.ts +++ b/plugins/plugin-kubectl-tray-menu/src/tray/main.ts @@ -14,6 +14,14 @@ * limitations under the License. */ +/** + * [Main Process] This is the logic that will be executed in the + * *electron-main* process for tray menu registration. This will be + * invoked by our `electron-main.ts`, via the `renderer` function + * below, which in turn is called from our `preload.ts`. + */ + +import Debug from 'debug' import { CreateWindowFunction } from '@kui-shell/core' import { productName } from '@kui-shell/client/config.d/name.json' @@ -25,6 +33,8 @@ import buildContextMenu from './menus' let tray: null | InstanceType = null class LiveMenu { + private readonly debug = Debug('plugin-kubectl-tray-menu/main') + // serialized form, to avoid unnecessary repaints private currentContextMenu = '' @@ -33,7 +43,9 @@ class LiveMenu { private readonly tray: import('electron').Tray, private readonly createWindow: CreateWindowFunction, private readonly periodic = setInterval(() => this.render(), 10 * 1000) - ) {} + ) { + this.debug('constructor') + } /** Avoid a flurry of re-renders */ private debounce: null | ReturnType = null @@ -43,14 +55,13 @@ class LiveMenu { clearTimeout(this.debounce) } this.debounce = setTimeout(async () => { + this.debug('render') try { // avoid blinking on linux by constantly repainting: only update // the tray if the model has changed const newContextMenu = await buildContextMenu(this.createWindow, this.render.bind(this)) - const newContextMenuSerialized = JSON.stringify( - newContextMenu, - (key, value) => (key === 'menu' || key === 'commandsMap' || key === 'commandId' ? undefined : value), - 2 + const newContextMenuSerialized = JSON.stringify(newContextMenu, (key, value) => + key === 'menu' || key === 'commandsMap' || key === 'commandId' ? undefined : value ) if (this.currentContextMenu !== newContextMenuSerialized) { this.currentContextMenu = newContextMenuSerialized @@ -65,10 +76,10 @@ class LiveMenu { } /** - * This is the logic that will be executed in the *electron-main* - * process for tray menu registration. This will be invoked by our - * `electron-main.ts`, via the `renderer` function below, which in - * turn is called from our `preload.ts`. + * [Main Process] This is the logic that will be executed in the + * *electron-main* process for tray menu registration. This will be + * invoked by our `electron-main.ts`, via the `renderer` function + * below, which in turn is called from our `preload.ts`. */ export default async function main(createWindow: CreateWindowFunction) { if (tray) { diff --git a/plugins/plugin-kubectl-tray-menu/src/tray/menus/contexts/current.ts b/plugins/plugin-kubectl-tray-menu/src/tray/menus/contexts/current.ts index 810e18d7f7c..ef86388b920 100644 --- a/plugins/plugin-kubectl-tray-menu/src/tray/menus/contexts/current.ts +++ b/plugins/plugin-kubectl-tray-menu/src/tray/menus/contexts/current.ts @@ -20,14 +20,19 @@ import { tellRendererToExecute } from '@kui-shell/core' export async function get() { const { execFile } = await import('child_process') return new Promise((resolve, reject) => { - execFile('kubectl', ['config', 'current-context'], { windowsHide: true }, (err, stdout, stderr) => { - if (err) { - console.error(stderr) - reject(err) - } else { - resolve(stdout.trim()) - } - }) + try { + execFile('kubectl', ['config', 'current-context'], { windowsHide: true }, (err, stdout, stderr) => { + if (err) { + console.error(stderr) + reject(err) + } else { + resolve(stdout.trim()) + } + }) + } catch (err) { + console.error('Error starting current namespace process', err) + reject(err) + } }) } @@ -37,18 +42,29 @@ export async function set(context: string, tellRenderer = true) { if (tellRenderer) { // inform the renderer that we have a context-related change - tellRendererToExecute('kubectl ' + args.join(' ')) + setTimeout(() => { + try { + tellRendererToExecute('kubectl ' + args.join(' ')) + } catch (err) { + console.error('Error communicating context change to renderer', err) + } + }, 200) } const { execFile } = await import('child_process') return new Promise((resolve, reject) => { - execFile('kubectl', args, { windowsHide: true }, (err, stdout, stderr) => { - if (err) { - console.error(stderr) - reject(err) - } else { - resolve(stdout.trim()) - } - }) + try { + execFile('kubectl', args, { windowsHide: true }, (err, stdout, stderr) => { + if (err) { + console.error(stderr) + reject(err) + } else { + resolve(stdout.trim()) + } + }) + } catch (err) { + console.error('Error starting use-context process', err) + reject(err) + } }) } diff --git a/plugins/plugin-kubectl-tray-menu/src/tray/menus/contexts/index.ts b/plugins/plugin-kubectl-tray-menu/src/tray/menus/contexts/index.ts index 07bd4532d14..8eba6136b00 100644 --- a/plugins/plugin-kubectl-tray-menu/src/tray/menus/contexts/index.ts +++ b/plugins/plugin-kubectl-tray-menu/src/tray/menus/contexts/index.ts @@ -14,6 +14,7 @@ * limitations under the License. */ +import Debug from 'debug' import { MenuItemConstructorOptions } from 'electron' import { CreateWindowFunction } from '@kui-shell/core' @@ -24,40 +25,61 @@ import UpdateFunction from '../../update' import { invalidate } from '../namespaces' class ContextWatcher { + private readonly debug = Debug('plugin-kubectl-tray-menu/context-watcher') + public constructor( private readonly updateFn: UpdateFunction, private _contexts: MenuItemConstructorOptions[] = [Loading] ) { + this.debug('constructor') setTimeout(() => this.scan()) - onRefresh(() => this.findAndSetCurrentContext(this.contexts)) + onRefresh(this.refresh) + } + + /** Refresh content, e.g. because the model changed in the renderer */ + private readonly refresh = () => { + this.debug('refresh') + setTimeout(() => this.findAndSetCurrentContext(this.contexts)) } + /** Re-generate menu model from current data */ private async findAndSetCurrentContext( contexts: MenuItemConstructorOptions[], currentContextP = get().catch(() => '') ) { - const currentContext = await currentContextP + try { + this.debug('findAndSetCurrentContext', contexts.length) + const currentContext = await currentContextP - const oldCur = contexts.find(_ => _.checked) - const newCur = contexts.find(_ => _.id === currentContext) - if (oldCur) { - oldCur.checked = false - } - if (newCur) { - newCur.checked = true + const oldCur = contexts.find(_ => _.checked) + const newCur = contexts.find(_ => _.id === currentContext) + if (oldCur) { + oldCur.checked = false + } + if (newCur) { + newCur.checked = true + } + + this.contexts = contexts + } catch (err) { + this.debug('findAndSetCurrentContext failure', err.message) } + } - this.contexts = contexts + private none(): MenuItemConstructorOptions[] { + return [{ label: '', enabled: false }] } - private async setAndScan(cluster: string) { - await set(cluster) + private async setAndScan(context: string) { + this.debug('setAndScan') + await set(context) this._contexts = [] invalidate() this.scan() } private async scan() { + this.debug('scan') const currentContextP = get().catch(() => '') const { execFile } = await import('child_process') @@ -66,13 +88,14 @@ class ContextWatcher { ['config', 'get-contexts', '--output=name'], { windowsHide: true }, async (err, stdout, stderr) => { + this.debug('scan done', !!err) if (err) { if (!/ENOENT/.test(err.message)) { // ENOENT if kubectl is not found console.error('Error scanning Kubernetes contexts', err.message) console.error(stderr) } - this.contexts = [{ label: '', enabled: false }] + this.contexts = this.none() } else { const contexts: Record = stdout .split(/\n/) diff --git a/plugins/plugin-kubectl-tray-menu/src/tray/menus/namespaces/current.ts b/plugins/plugin-kubectl-tray-menu/src/tray/menus/namespaces/current.ts index ebbad7c0942..6a31be49d8a 100644 --- a/plugins/plugin-kubectl-tray-menu/src/tray/menus/namespaces/current.ts +++ b/plugins/plugin-kubectl-tray-menu/src/tray/menus/namespaces/current.ts @@ -20,19 +20,24 @@ import { tellRendererToExecute } from '@kui-shell/core' export async function get() { const { execFile } = await import('child_process') return new Promise((resolve, reject) => { - execFile( - 'kubectl', - ['config', 'view', '--minify', '--output=jsonpath={..namespace}'], - { windowsHide: true }, - (err, stdout, stderr) => { - if (err) { - console.error(stderr) - reject(err) - } else { - resolve(stdout.trim()) + try { + execFile( + 'kubectl', + ['config', 'view', '--minify', '--output=jsonpath={..namespace}'], + { windowsHide: true }, + (err, stdout, stderr) => { + if (err) { + console.error(stderr) + reject(err) + } else { + resolve(stdout.trim()) + } } - } - ) + ) + } catch (err) { + console.error('Error starting current context process', err) + reject(err) + } }) } @@ -42,18 +47,29 @@ export async function set(ns: string, tellRenderer = true) { if (tellRenderer) { // inform the renderer that we have a context-related change - tellRendererToExecute('kubectl ' + args.join(' ')) + setTimeout(() => { + try { + tellRendererToExecute('kubectl ' + args.join(' ')) + } catch (err) { + console.error('Error communicating namespace change to renderer', err) + } + }, 200) } const { execFile } = await import('child_process') return new Promise((resolve, reject) => { - execFile('kubectl', args, { windowsHide: true }, (err, stdout, stderr) => { - if (err) { - console.error(stderr) - reject(err) - } else { - resolve(stdout.trim()) - } - }) + try { + execFile('kubectl', args, { windowsHide: true }, (err, stdout, stderr) => { + if (err) { + console.error(stderr) + reject(err) + } else { + resolve(stdout.trim()) + } + }) + } catch (err) { + console.error('Error starting namespace set-current process', err) + reject(err) + } }) } diff --git a/plugins/plugin-kubectl-tray-menu/src/tray/menus/namespaces/index.ts b/plugins/plugin-kubectl-tray-menu/src/tray/menus/namespaces/index.ts index b4dd2ef6b93..9fbe2cc3021 100644 --- a/plugins/plugin-kubectl-tray-menu/src/tray/menus/namespaces/index.ts +++ b/plugins/plugin-kubectl-tray-menu/src/tray/menus/namespaces/index.ts @@ -25,48 +25,66 @@ import { onRefresh } from '../../events' import UpdateFunction from '../../update' class NamespaceWatcher { + private readonly debug = Debug('plugin-kubectl-tray-menu/namespace-watcher') + public constructor( private readonly updateFn: UpdateFunction, private _namespaces: MenuItemConstructorOptions[] = [Loading] ) { + this.debug('constructor') setTimeout(() => this.scan()) onRefresh(this.refresh) } /** Refresh content, e.g. because the model changed in the renderer */ - private readonly refresh = () => this.findAndSetCurrentNamespace(this.namespaces) + private readonly refresh = () => { + this.debug('refresh') + setTimeout(() => this.findAndSetCurrentNamespace(this.namespaces)) + } /** Re-generate menu model from current data */ private async findAndSetCurrentNamespace( namespaces: MenuItemConstructorOptions[], currentNamespaceP = get().catch(() => '') ) { - const currentNamespace = await currentNamespaceP - - const oldCur = namespaces.find(_ => _.checked) - const newCur = namespaces.find(_ => _.label === currentNamespace) - if (oldCur) { - oldCur.checked = false - } - if (newCur) { - newCur.checked = true + try { + this.debug('findAndSetCurrentNamespace', namespaces.length) + const currentNamespace = await currentNamespaceP + + const oldCur = namespaces.find(_ => _.checked) + const newCur = namespaces.find(_ => _.label === currentNamespace) + if (oldCur) { + oldCur.checked = false + } + if (newCur) { + newCur.checked = true + } + + this.namespaces = namespaces + } catch (err) { + this.debug('findAndSetCurrentNamespace failure', err.message) } + } - this.namespaces = namespaces + private none(): MenuItemConstructorOptions[] { + return [{ label: '', enabled: false }] } private async setAndScan(ns: string) { + this.debug('setAndScan') await set(ns) this._namespaces = [] this.scan() } private unauthorized() { + this.debug('unauthorized') this.namespaces = [{ label: 'Your token has probably expired', enabled: false, icon: errorIcon }] } private async scan() { try { + this.debug('scan') const currentNamespaceP = get().catch(() => '') const { spawn } = await import('child_process') @@ -80,6 +98,7 @@ class NamespaceWatcher { ) child.on('exit', async code => { + this.debug('exit', code) if ( code === 1 && (this.namespaces.length === 0 || (this.namespaces.length === 1 && this.namespaces[0] === Loading)) @@ -97,13 +116,14 @@ class NamespaceWatcher { if (!/ENOENT/.test(err.message)) { // ENOENT if kubectl is not found console.error(err.message) + } else { + this.debug('error', err.message) } - this.namespaces = [{ label: '', enabled: false }] + this.namespaces = this.none() } }) - const debug = Debug('plugin-kubectl/tray/menus/namespaces') - child.stderr.on('data', data => debug(data.toString())) + child.stderr.on('data', data => this.debug(data.toString())) // partial line from last batch let leftover = '' @@ -154,6 +174,7 @@ class NamespaceWatcher { } public invalidate() { + this.debug('invalidate') this._namespaces = [] this.scan() }