Skip to content

Commit

Permalink
fix: Handle server disconnects / non-existing server (#14)
Browse files Browse the repository at this point in the history
This PR improves the server connection experience / fixes some bugs:
* Status bar item now shows `Deephaven: Connecting...` while a
connection is being initialized (before it immediately showed as
connected)
* Status bar shows `Deephaven: Disconnected` if connection fails
* If client disconnects due to server stopping or anything else that
results in an Jsapi ide 'disconnect' event, the status bar will be
updated to reflect the status
* Updated status bar / quick pick menu icons

### Testing
**Setup**
If you clone this repo and run `npm install`, you should be able to `f5`
to run the extension in debug mode.

**Test Steps**
* Start a DH server on `localhost:10000`
* Connect to server in extension. Should see status bar "Connecting..."
then "Connected" status (the connecting status may happen too quickly to
really see it).
* Kill server
* Should see status bar item switch to disconnected
* Attempt to re-connect to stopped server
* Should see "Connecting..." status for longer this time and then a
"Failed to initialize Deephaven API" toast. Status should say
"Disconnected"

fixes #4 fixes #5 fixes #6
  • Loading branch information
bmingles authored Jul 5, 2024
1 parent 957044b commit 73818af
Show file tree
Hide file tree
Showing 10 changed files with 145 additions and 29 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
"name": "vscode-deephaven",
"displayName": "Deephaven in VS Code",
"description": "",
"publisher": "bmingles",
"publisher": "Deephaven Data Labs",
"repository": {
"type": "git",
"url": "https://github.com/deephaven/vscode-deephaven"
},
"version": "0.0.1",
"version": "0.0.2",
"engines": {
"vscode": "^1.87.0"
},
Expand Down
Binary file added releases/vscode-deephaven-0.0.2.vsix
Binary file not shown.
Binary file modified releases/vscode-deephaven-latest.vsix
Binary file not shown.
1 change: 1 addition & 0 deletions src/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ export const SELECT_CONNECTION_COMMAND = `${CONFIG_KEY}.selectConnection`;

export const STATUS_BAR_DISCONNECTED_TEXT = 'Deephaven: Disconnected';
export const STATUS_BAR_DISCONNECT_TEXT = 'Deephaven: Disconnect';
export const STATUS_BAR_CONNECTING_TEXT = 'Deephaven: Connecting...';
44 changes: 31 additions & 13 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as vscode from 'vscode';
import {
ConnectionOption,
createConnectStatusBarItem,
createConnectText,
createConnectTextAndTooltip,
createConnectionOptions,
createConnectionQuickPick,
getTempDir,
Expand All @@ -13,6 +13,7 @@ import {
RUN_CODE_COMMAND,
RUN_SELECTION_COMMAND,
SELECT_CONNECTION_COMMAND,
STATUS_BAR_CONNECTING_TEXT,
STATUS_BAR_DISCONNECTED_TEXT,
} from './common';

Expand Down Expand Up @@ -40,6 +41,22 @@ export function activate(context: vscode.ExtensionContext) {

const dhcServiceRegistry = new DhServiceRegistry(DhcService, outputChannel);

dhcServiceRegistry.addEventListener('disconnect', () => {
clearConnection();
});

/*
* Clear connection data
*/
function clearConnection() {
selectedConnectionUrl = null;
selectedDhService = null;
const { text, tooltip } = createConnectTextAndTooltip('disconnected');
connectStatusBarItem.text = text;
connectStatusBarItem.tooltip = tooltip;
dhcServiceRegistry.clearCache();
}

/**
* Get currently active DH service.
* @autoActivate If true, auto-activate a service if none is active.
Expand Down Expand Up @@ -96,26 +113,27 @@ export function activate(context: vscode.ExtensionContext) {

// Disconnect option was selected, or connectionUrl that no longer exists
if (connectionUrl == null || !option) {
selectedConnectionUrl = null;
selectedDhService = null;
connectStatusBarItem.text = createConnectText(
STATUS_BAR_DISCONNECTED_TEXT
);
dhcServiceRegistry.clearCache();
clearConnection();
return;
}

selectedConnectionUrl = connectionUrl;

connectStatusBarItem.text = createConnectText(option.label);
const { text, tooltip } = createConnectTextAndTooltip('connecting', option);
connectStatusBarItem.text = text;
connectStatusBarItem.tooltip = tooltip;

selectedConnectionUrl = connectionUrl;
selectedDhService = await dhcServiceRegistry.get(selectedConnectionUrl);

if (selectedDhService.isInitialized) {
if (selectedDhService.isInitialized || (await selectedDhService.initDh())) {
const { text, tooltip } = createConnectTextAndTooltip(
'connected',
option
);
connectStatusBarItem.text = text;
connectStatusBarItem.tooltip = tooltip;
outputChannel.appendLine(`Initialized: ${selectedConnectionUrl}`);
} else {
await selectedDhService.initDh();
outputChannel.appendLine(`Initialized: ${selectedConnectionUrl}`);
clearConnection();
}
}
}
Expand Down
9 changes: 8 additions & 1 deletion src/services/CacheService.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,16 @@
export class CacheService<T> {
import { EventDispatcher } from './EventDispatcher';

export class CacheService<
T,
TEventName extends string
> extends EventDispatcher<TEventName> {
constructor(
label: string,
loader: (key: string | null) => Promise<T>,
normalizeKey: (key: string | null) => string | null
) {
super();

this.label = label;
this.loader = loader;
this.normalizeKey = normalizeKey;
Expand Down
20 changes: 16 additions & 4 deletions src/services/DhService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { hasErrorCode } from '../util/typeUtils';
import { ConnectionAndSession } from '../common';
import { formatTimestamp } from '../util';
import { PanelFocusManager } from './PanelFocusManager';
import { EventDispatcher } from './EventDispatcher';

/* eslint-disable @typescript-eslint/naming-convention */
const icons = {
Expand All @@ -26,8 +27,13 @@ type CommandResultBase = {
error: string;
};

export abstract class DhService<TDH, TClient> {
export abstract class DhService<
TDH,
TClient
> extends EventDispatcher<'disconnect'> {
constructor(serverUrl: string, outputChannel: vscode.OutputChannel) {
super();

this.serverUrl = serverUrl;
this.outputChannel = outputChannel;
}
Expand Down Expand Up @@ -84,7 +90,7 @@ export abstract class DhService<TDH, TClient> {
return this.cachedInitApi != null;
}

public async initDh() {
public async initDh(): Promise<boolean> {
try {
if (this.cachedInitApi == null) {
this.outputChannel.appendLine(
Expand All @@ -101,10 +107,10 @@ export abstract class DhService<TDH, TClient> {
this.clearCaches();
console.error(err);
this.outputChannel.appendLine(
`Failed to initialize Deephaven API: ${err}`
`Failed to initialize Deephaven API${err == null ? '.' : `: ${err}`}`
);
vscode.window.showErrorMessage('Failed to initialize Deephaven API');
return;
return false;
}

if (this.cachedCreateClient == null) {
Expand All @@ -129,6 +135,8 @@ export abstract class DhService<TDH, TClient> {
vscode.window.showInformationMessage(
`Disconnected from Deephaven server: ${this.serverUrl}`
);

this.dispatchEvent('disconnect');
})
);
}
Expand Down Expand Up @@ -160,10 +168,14 @@ export abstract class DhService<TDH, TClient> {
vscode.window.showErrorMessage(
`Failed to create Deephaven session: ${this.serverUrl}`
);

return false;
} else {
vscode.window.showInformationMessage(
`Created Deephaven session: ${this.serverUrl}`
);

return true;
}
}

Expand Down
13 changes: 11 additions & 2 deletions src/services/DhServiceRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import { CacheService } from './CacheService';
import { DhcService } from './DhcService';
import { ensureHasTrailingSlash } from '../util';

export class DhServiceRegistry<T extends DhcService> extends CacheService<T> {
export class DhServiceRegistry<T extends DhcService> extends CacheService<
T,
'disconnect'
> {
constructor(
serviceFactory: new (
serverUrl: string,
Expand All @@ -18,7 +21,13 @@ export class DhServiceRegistry<T extends DhcService> extends CacheService<T> {
throw new Error(`${serviceFactory.name} server url is null.`);
}

return new serviceFactory(serverUrl, outputChannel);
const dhService = new serviceFactory(serverUrl, outputChannel);

dhService.addEventListener('disconnect', () => {
this.dispatchEvent('disconnect');
});

return dhService;
},
ensureHasTrailingSlash
);
Expand Down
39 changes: 39 additions & 0 deletions src/services/EventDispatcher.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
export type EventListener = <TEvent>(event: TEvent) => void;
export type UnsubscribeEventListener = () => void;

/**
* General purpose event dispatcher for events that can be subscribed to.
*/
export class EventDispatcher<TEventName extends string> {
private listeners: Map<string, Set<EventListener>> = new Map();

/**
* Register an event listener for a given event name.
* @param eventName The name of the event to listen for.
* @param listener The event listener to register.
* @returns A function that can be called to unsubscribe the event listener.
*/
addEventListener = (
eventName: TEventName,
listener: EventListener
): UnsubscribeEventListener => {
if (!this.listeners.has(eventName)) {
this.listeners.set(eventName, new Set());
}

this.listeners.get(eventName)?.add(listener);

return () => {
this.listeners.get(eventName)?.delete(listener);
};
};

/**
* Dispatch an event to all registered listeners.
* @param eventName The name of the event to dispatch.
* @param event The event to dispatch to all listeners
*/
dispatchEvent = <TEvent>(eventName: TEventName, event?: TEvent): void => {
this.listeners.get(eventName)?.forEach(listener => listener(event));
};
}
44 changes: 37 additions & 7 deletions src/util/uiUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as vscode from 'vscode';
import {
ConnectionType,
SELECT_CONNECTION_COMMAND,
STATUS_BAR_CONNECTING_TEXT,
STATUS_BAR_DISCONNECTED_TEXT,
STATUS_BAR_DISCONNECT_TEXT,
} from '../common';
Expand Down Expand Up @@ -33,7 +34,7 @@ export async function createConnectionQuickPick(
selectedUrl?: string | null
): Promise<ConnectionOption | DisconnectOption | undefined> {
function padLabel(label: string, isSelected: boolean) {
return isSelected ? `$(circle-filled) ${label}` : ` ${label}`;
return isSelected ? `$(vm-connect) ${label}` : `$(blank) ${label}`;
}

const options: (ConnectionOption | DisconnectOption)[] = [
Expand Down Expand Up @@ -62,7 +63,9 @@ export function createConnectStatusBarItem() {
100
);
statusBarItem.command = SELECT_CONNECTION_COMMAND;
statusBarItem.text = createConnectText(STATUS_BAR_DISCONNECTED_TEXT);
const { text, tooltip } = createConnectTextAndTooltip('disconnected');
statusBarItem.text = text;
statusBarItem.tooltip = tooltip;
statusBarItem.show();

return statusBarItem;
Expand All @@ -73,7 +76,7 @@ export function createConnectStatusBarItem() {
* @param type The type of connection
*/
export function createConnectionOption(type: ConnectionType) {
return (serverUrl: string) => {
return (serverUrl: string): ConnectionOption => {
const url = new URL(serverUrl ?? '');
const label = `${type}: ${url.hostname}:${url.port}`;

Expand All @@ -95,11 +98,38 @@ export function createConnectionOptions(): ConnectionOption[] {
}

/**
* Create display text for the connection status bar item.
* @param connectionDisplay The connection display text
* Create display text and tooltip for the connection status bar item.
* @param status The connection status
* @param option The connection option
*/
export function createConnectText(connectionDisplay: string) {
return `$(debug-disconnect) ${connectionDisplay.trim()}`;
export function createConnectTextAndTooltip(
status: 'connecting' | 'connected' | 'disconnected',
option?: ConnectionOption
): { text: string; tooltip: string } {
const icon = {
connecting: '$(sync~spin)',
connected: '$(vm-connect)',
disconnected: '$(debug-disconnect)',
}[status];

const label = {
connecting: STATUS_BAR_CONNECTING_TEXT,
connected: option?.label,
disconnected: STATUS_BAR_DISCONNECTED_TEXT,
}[status];

const tooltip = {
connecting: `Connecting to ${option?.url}...`,
connected: `Connected to ${option?.url}`,
disconnected: 'Connect to Deephaven',
}[status];

const text = `${icon} ${label}`;

return {
text,
tooltip,
};
}

// Copied from @deephaven/console `ConsoleUtils`
Expand Down

0 comments on commit 73818af

Please sign in to comment.