From 499798fc1752d3d2763d06b91d2c675ea164f15b Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 6 Feb 2019 18:11:31 -0600 Subject: [PATCH] Wrap shared process in retry --- packages/ide/src/fill/notification.ts | 18 ++++++++++++++---- packages/ide/src/retry.ts | 16 ++++++++-------- packages/logger/src/logger.ts | 10 ++++++---- packages/server/src/cli.ts | 8 +------- packages/server/src/vscode/sharedProcess.ts | 10 +++++++--- 5 files changed, 36 insertions(+), 26 deletions(-) diff --git a/packages/ide/src/fill/notification.ts b/packages/ide/src/fill/notification.ts index ebda3eeea..f1c156c9f 100644 --- a/packages/ide/src/fill/notification.ts +++ b/packages/ide/src/fill/notification.ts @@ -41,20 +41,30 @@ export interface IProgressService { } /** - * Temporary notification service. + * Console-based notification service. */ export class NotificationService implements INotificationService { public error(error: Error): void { logger.error(error.message, field("error", error)); } - public prompt(_severity: Severity, message: string, _buttons: INotificationButton[], _onCancel: () => void): INotificationHandle { - throw new Error(`cannot prompt using the console: ${message}`); + public prompt(severity: Severity, message: string, _buttons: INotificationButton[], _onCancel: () => void): INotificationHandle { + switch (severity) { + case Severity.Info: logger.info(message); break; + case Severity.Warning: logger.warn(message); break; + case Severity.Error: logger.error(message); break; + } + + return { + close: (): void => undefined, + updateMessage: (): void => undefined, + updateButtons: (): void => undefined, + }; } } /** - * Temporary progress service. + * Console-based progress service. */ export class ProgressService implements IProgressService { public start(title: string, task: (progress: IProgress) => Promise): Promise { diff --git a/packages/ide/src/retry.ts b/packages/ide/src/retry.ts index 2902c0c41..9cf41af31 100644 --- a/packages/ide/src/retry.ts +++ b/packages/ide/src/retry.ts @@ -1,4 +1,4 @@ -import { logger } from "@coder/logger"; +import { logger, field } from "@coder/logger"; import { NotificationService, INotificationHandle, INotificationService, Severity } from "./fill/notification"; interface IRetryItem { @@ -105,7 +105,7 @@ export class Retry { /** * Retry a service. */ - public run(name: string): void { + public run(name: string, error?: Error): void { if (!this.items.has(name)) { throw new Error(`"${name}" is not registered`); } @@ -117,7 +117,7 @@ export class Retry { item.running = true; // This timeout is for the case when the connection drops; this allows time - // for the Wush service to come in and block everything because some other + // for the socket service to come in and block everything because some other // services might make it here first and try to restart, which will fail. setTimeout(() => { if (this.blocked && this.blocked !== name) { @@ -125,7 +125,7 @@ export class Retry { } if (!item.count || item.count < this.maxImmediateRetries) { - return this.runItem(name, item); + return this.runItem(name, item, error); } if (!item.delay) { @@ -137,10 +137,10 @@ export class Retry { } } - logger.info(`Retrying ${name.toLowerCase()} in ${item.delay}s`); + logger.info(`Retrying ${name.toLowerCase()} in ${item.delay}s`, error && field("error", error.message)); const itemDelayMs = item.delay * 1000; item.end = Date.now() + itemDelayMs; - item.timeout = setTimeout(() => this.runItem(name, item), itemDelayMs); + item.timeout = setTimeout(() => this.runItem(name, item, error), itemDelayMs); this.updateNotification(); }, this.waitDelay); @@ -165,7 +165,7 @@ export class Retry { /** * Run an item. */ - private runItem(name: string, item: IRetryItem): void { + private runItem(name: string, item: IRetryItem, error?: Error): void { if (!item.count) { item.count = 1; } else { @@ -175,7 +175,7 @@ export class Retry { const retryCountText = item.count <= this.maxImmediateRetries ? `[${item.count}/${this.maxImmediateRetries}]` : `[${item.count}]`; - logger.info(`Trying ${name.toLowerCase()} ${retryCountText}...`); + logger.info(`Starting ${name.toLowerCase()} ${retryCountText}...`, error && field("error", error.message)); const endItem = (): void => { this.stopItem(item); diff --git a/packages/logger/src/logger.ts b/packages/logger/src/logger.ts index 0564ff39f..4f3449467 100644 --- a/packages/logger/src/logger.ts +++ b/packages/logger/src/logger.ts @@ -38,8 +38,10 @@ export class Time { ) { } } +// `undefined` is allowed to make it easy to conditionally display a field. +// For example: `error && field("error", error)` // tslint:disable-next-line no-any -export type FieldArray = Array>; +export type FieldArray = Array | undefined>; // Functions can be used to remove the need to perform operations when the // logging level won't output the result anyway. @@ -338,9 +340,9 @@ export class Logger { passedFields = values as FieldArray; } - const fields = this.defaultFields - ? passedFields.concat(this.defaultFields) - : passedFields; + const fields = (this.defaultFields + ? passedFields.filter((f) => !!f).concat(this.defaultFields) + : passedFields.filter((f) => !!f)) as Array>; // tslint:disable-line no-any const now = Date.now(); let times: Array> = []; diff --git a/packages/server/src/cli.ts b/packages/server/src/cli.ts index 761850888..f0c9dc2e3 100644 --- a/packages/server/src/cli.ts +++ b/packages/server/src/cli.ts @@ -92,7 +92,6 @@ export class Entry extends Command { logger.info("Additional documentation: https://coder.com/docs"); logger.info("Initializing", field("data-dir", dataDir), field("working-dir", workingDir), field("log-dir", logDir)); const sharedProcess = new SharedProcess(dataDir, builtInExtensionsDir); - logger.info("Starting shared process...", field("socket", sharedProcess.socketPath)); const sendSharedProcessReady = (socket: WebSocket): void => { const active = new SharedProcessActiveMessage(); active.setSocketPath(sharedProcess.socketPath); @@ -102,12 +101,7 @@ export class Entry extends Command { socket.send(serverMessage.serializeBinary()); }; sharedProcess.onState((event) => { - if (event.state === SharedProcessState.Stopped) { - logger.error("Shared process stopped. Restarting...", field("error", event.error)); - } else if (event.state === SharedProcessState.Starting) { - logger.info("Starting shared process..."); - } else if (event.state === SharedProcessState.Ready) { - logger.info("Shared process is ready!"); + if (event.state === SharedProcessState.Ready) { app.wss.clients.forEach((c) => sendSharedProcessReady(c)); } }); diff --git a/packages/server/src/vscode/sharedProcess.ts b/packages/server/src/vscode/sharedProcess.ts index 41e917dae..49d847b5d 100644 --- a/packages/server/src/vscode/sharedProcess.ts +++ b/packages/server/src/vscode/sharedProcess.ts @@ -6,7 +6,8 @@ import { forkModule } from "./bootstrapFork"; import { StdioIpcHandler } from "../ipc"; import { ParsedArgs } from "vs/platform/environment/common/environment"; import { LogLevel } from "vs/platform/log/common/log"; -import { Emitter, Event } from "@coder/events/src"; +import { Emitter } from "@coder/events/src"; +import { retry } from "@coder/ide/src/retry"; export enum SharedProcessState { Stopped, @@ -28,12 +29,14 @@ export class SharedProcess { private ipcHandler: StdioIpcHandler | undefined; private readonly onStateEmitter = new Emitter(); public readonly onState = this.onStateEmitter.event; + private readonly retryName = "Shared process"; public constructor( private readonly userDataDir: string, private readonly builtInExtensionsDir: string, ) { - this.restart(); + retry.register(this.retryName, () => this.restart()); + retry.run(this.retryName); } public get state(): SharedProcessState { @@ -73,7 +76,7 @@ export class SharedProcess { state: SharedProcessState.Stopped, }); } - this.restart(); + retry.run(this.retryName, new Error(`Exited with ${err}`)); }); this.ipcHandler = new StdioIpcHandler(this.activeProcess); this.ipcHandler.once("handshake:hello", () => { @@ -94,6 +97,7 @@ export class SharedProcess { }); this.ipcHandler.once("handshake:im ready", () => { resolved = true; + retry.recover(this.retryName); this.setState({ state: SharedProcessState.Ready, });