fix: Log server connection errors + tweaks to debug logger (#192)

DH-18161: The pip connection error turned out to be jsapi version specific. Namely we removed some polyfills from the extension that are not needed with newer versions of jsapi. I had a stale pip install which caused the issue. This PR adds better error logging + toast to indicate what actually went wrong in such cases.
deephaven · Dec 12, 2024 · f494ae8 · f494ae8
1 parent fdf87b9
commit f494ae8
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 47 deletions.
diff --git a/src/controllers/UserLoginController.ts b/src/controllers/UserLoginController.ts
@@ -19,6 +19,7 @@ import {
 import { Logger, promptForPsk, runUserLoginWorkflow } from '../util';
 import type {
   CoreAuthenticatedClient,
+  CoreUnauthenticatedClient,
   IAsyncCacheService,
   ICoreClientFactory,
   IDheClientFactory,
@@ -194,42 +195,53 @@ export class UserLoginController extends ControllerBase {
    * Create a core authenticated client.
    */
   onCreateCoreAuthenticatedClient = async (serverUrl: URL): Promise<void> => {
-    const client = await this.coreClientFactory(serverUrl);
-
-    const authConfig = new Set(
-      (await client.getAuthConfigValues()).map(([, value]) => value)
-    );
-
+    let client: CoreUnauthenticatedClient | null = null;
     let credentials: DhcType.LoginCredentials | null = null;
 
-    if (authConfig.has(AUTH_HANDLER_TYPE_ANONYMOUS)) {
-      const dh = await this.coreJsApiCache.get(serverUrl);
-      credentials = {
-        type: dh.CoreClient.LOGIN_TYPE_ANONYMOUS,
-      };
-    } else if (authConfig.has(AUTH_HANDLER_TYPE_PSK)) {
-      const token =
-        (await this.secretService.getPsk(serverUrl)) ??
-        (await promptForPsk('Enter your Pre-Shared Key'));
-
-      if (token == null) {
-        this.toast.info('Login cancelled.');
-        return;
-      }
+    try {
+      client = await this.coreClientFactory(serverUrl);
 
-      credentials = {
-        type: AUTH_HANDLER_TYPE_PSK,
-        token,
-      };
+      const authConfig = new Set(
+        (await client.getAuthConfigValues()).map(([, value]) => value)
+      );
 
-      this.secretService.storePsk(serverUrl, token);
-    } else if (authConfig.has(AUTH_HANDLER_TYPE_DHE)) {
-      credentials = await this.serverManager.getWorkerCredentials(serverUrl);
-      if (credentials == null) {
-        throw new Error(`Failed to get credentials for worker '${serverUrl}'`);
+      if (authConfig.has(AUTH_HANDLER_TYPE_ANONYMOUS)) {
+        const dh = await this.coreJsApiCache.get(serverUrl);
+        credentials = {
+          type: dh.CoreClient.LOGIN_TYPE_ANONYMOUS,
+        };
+      } else if (authConfig.has(AUTH_HANDLER_TYPE_PSK)) {
+        const token =
+          (await this.secretService.getPsk(serverUrl)) ??
+          (await promptForPsk('Enter your Pre-Shared Key'));
+
+        if (token == null) {
+          this.toast.info('Login cancelled.');
+          return;
+        }
+
+        credentials = {
+          type: AUTH_HANDLER_TYPE_PSK,
+          token,
+        };
+
+        this.secretService.storePsk(serverUrl, token);
+      } else if (authConfig.has(AUTH_HANDLER_TYPE_DHE)) {
+        credentials = await this.serverManager.getWorkerCredentials(serverUrl);
+        if (credentials == null) {
+          throw new Error(
+            `Failed to get credentials for worker '${serverUrl}'`
+          );
+        }
+      } else {
+        throw new Error('No supported authentication methods found.');
       }
-    } else {
-      throw new Error('No supported authentication methods found.');
+    } catch (err) {
+      const msg = 'Failed to connect to Deephaven server.';
+      logger.error(msg, err);
+      this.toast.error(msg);
+
+      return;
     }
 
     try {
@@ -238,13 +250,11 @@ export class UserLoginController extends ControllerBase {
         await loginClient(client, credentials)
       );
     } catch (err) {
-      logger.error(
-        'An error occurred while connecting to Deephaven server:',
-        err
-      );
-      this.coreClientCache.delete(serverUrl);
+      const msg = 'Login failed.';
+      logger.error(msg, err);
+      this.toast.error(msg);
 
-      this.toast.error('Login failed.');
+      this.coreClientCache.delete(serverUrl);
 
       if (credentials.type === AUTH_HANDLER_TYPE_PSK) {
         await this.secretService.deletePsk(serverUrl);

diff --git a/src/util/Logger.ts b/src/util/Logger.ts
@@ -44,17 +44,19 @@ export class Logger {
   static addOutputChannelHandler = (
     outputChannel: vscode.OutputChannel
   ): void => {
+    const createHandler =
+      (level: LogLevel): LogLevelHandler =>
+      (label, ...args) =>
+        outputChannel.appendLine(
+          `${label} ${level.toUpperCase()}: ${args.map(a => (a instanceof Error ? (a.stack ?? a.message) : a)).join(' ')}`
+        );
+
     Logger.handlers.add({
-      error: (label, ...args) =>
-        outputChannel.appendLine(`${label} ERROR: ${args.join(', ')}`),
-      warn: (label, ...args) =>
-        outputChannel.appendLine(`${label} WARN: ${args.join(', ')}`),
-      info: (label, ...args) =>
-        outputChannel.appendLine(`${label} INFO: ${args.join(', ')}`),
-      debug: (label, ...args) =>
-        outputChannel.appendLine(`${label} DEBUG: ${args.join(', ')}`),
-      debug2: (label, ...args) =>
-        outputChannel.appendLine(`${label} DEBUG2: ${args.join(', ')}`),
+      error: createHandler('error'),
+      warn: createHandler('warn'),
+      info: createHandler('info'),
+      debug: createHandler('debug'),
+      debug2: createHandler('debug2'),
     });
   };