fix: show user message on unhandled exception #3868

Problem:
When webview backend/server code throws an unhandled
exception, it is only logged. The UI doesn't expose the message
or any indication about next steps.

Solution:
When a webview throws an unhandled exception, show an error message
with a "View Logs" button.

Problem:
The "View Logs" feature can get confused on subsequent errors,
the error will not be highlighted even though it's in the logs.
Apparently vscode.TextDocument.getText() caches the text and
only updates once something triggers it. So when we programatically
request the text, we sometimes get stale results.

Solution:
Use the vscode "revert" command to force getText() to read from disk.

Signed-off-by: nkomonen <[email protected]>

Author: nkomonen-amazon

src/auth/ui/vue/show.ts

@@ -43,6 +43,7 @@ import { connectToEnterpriseSso } from '../../../codewhisperer/util/getStartUrl'
 import { trustedDomainCancellation } from '../../sso/model'
 import { FeatureId, CredentialSourceId, Result, telemetry } from '../../../shared/telemetry/telemetry'
 import { AuthFormId, isBuilderIdAuth } from './authForms/types'
+import { handleWebviewError } from '../../../webviews/server'
 
 export class AuthWebview extends VueWebview {
     public override id: string = 'authWebview'
@@ -129,11 +130,11 @@ export class AuthWebview extends VueWebview {
     }
 
     async startCodeWhispererBuilderIdSetup(): Promise<AuthError | undefined> {
-        return this.ssoSetup(() => awsIdSignIn())
+        return this.ssoSetup('startCodeWhispererBuilderIdSetup', () => awsIdSignIn())
     }
 
     async startCodeCatalystBuilderIdSetup(): Promise<AuthError | undefined> {
-        return this.ssoSetup(() => setupCodeCatalystBuilderId(this.codeCatalystAuth))
+        return this.ssoSetup('startCodeCatalystBuilderIdSetup', () => setupCodeCatalystBuilderId(this.codeCatalystAuth))
     }
 
     isCodeWhispererBuilderIdConnected(): boolean {
@@ -193,7 +194,7 @@ export class AuthWebview extends VueWebview {
             const ssoProfile = createSsoProfile(startUrl, regionId)
             await Auth.instance.createConnection(ssoProfile)
         }
-        return this.ssoSetup(setupFunc)
+        return this.ssoSetup('createIdentityCenterConnection', setupFunc)
     }
 
     /**
@@ -203,10 +204,17 @@ export class AuthWebview extends VueWebview {
         const setupFunc = () => {
             return connectToEnterpriseSso(startUrl, regionId)
         }
-        return this.ssoSetup(setupFunc)
+        return this.ssoSetup('startCWIdentityCenterSetup', setupFunc)
     }
 
-    private async ssoSetup(setupFunc: () => Promise<any>): Promise<AuthError | undefined> {
+    /**
+     * This wraps the execution of the given setupFunc() and handles common errors from the SSO setup process.
+     *
+     * @param methodName A value that will help identify which high level function called this method.
+     * @param setupFunc The function which will be executed in a try/catch so that we can handle common errors.
+     * @returns
+     */
+    private async ssoSetup(methodName: string, setupFunc: () => Promise<any>): Promise<AuthError | undefined> {
         try {
             await setupFunc()
             return
@@ -236,7 +244,11 @@ export class AuthWebview extends VueWebview {
                 return { id: 'badStartUrl', text: `Connection failed. Please verify your start URL.` }
             }
 
-            getLogger().error('AuthWebview: Failed to setup: %s', (e as Error).message)
+            // If SSO setup fails we want to be able to show the user an error in the UI, due to this we cannot
+            // throw an error here. So instead this will additionally show an error message that provides more
+            // detailed information.
+            handleWebviewError(e, this.id, methodName)
+
             return { id: 'defaultFailure', text: 'Failed to setup.' }
         }
     }

src/extension.ts

@@ -62,14 +62,15 @@ import globals, { initialize } from './shared/extensionGlobals'
 import { join } from 'path'
 import { Experiments, Settings } from './shared/settings'
 import { isReleaseVersion } from './shared/vscode/env'
-import { Commands, registerErrorHandler } from './shared/vscode/commands2'
-import { ToolkitError, isUserCancelledError, resolveErrorMessageToDisplay } from './shared/errors'
-import { Logging } from './shared/logger/commands'
+import { Commands, registerErrorHandler as registerCommandErrorHandler } from './shared/vscode/commands2'
 import { UriHandler } from './shared/vscode/uriHandler'
 import { telemetry } from './shared/telemetry/telemetry'
 import { Auth } from './auth/auth'
-import { showMessageWithUrl } from './shared/utilities/messages'
 import { openUrl } from './shared/utilities/vsCodeUtils'
+import { isUserCancelledError, resolveErrorMessageToDisplay, ToolkitError } from './shared/errors'
+import { Logging } from './shared/logger/commands'
+import { showMessageWithUrl } from './shared/utilities/messages'
+import { registerWebviewErrorHandler } from './webviews/server'
 
 let localize: nls.LocalizeFunc
 
@@ -89,9 +90,13 @@ export async function activate(context: vscode.ExtensionContext) {
     )
     globals.outputChannel = toolkitOutputChannel
 
-    registerErrorHandler((info, error) => {
+    registerCommandErrorHandler((info, error) => {
         const defaultMessage = localize('AWS.generic.message.error', 'Failed to run command: {0}', info.id)
-        handleError(error, info.id, defaultMessage)
+        logAndShowError(error, info.id, defaultMessage)
+    })
+
+    registerWebviewErrorHandler((error: unknown, webviewId: string, command: string) => {
+        logAndShowWebviewError(error, webviewId, command)
     })
 
     if (isCloud9()) {
@@ -273,29 +278,6 @@ export async function activate(context: vscode.ExtensionContext) {
     }
 }
 
-// This is only being used for errors from commands although there's plenty of other places where it
-// could be used. It needs to be apart of some sort of `core` module that is guaranteed to initialize
-// prior to every other Toolkit component. Logging and telemetry would fit well within this core module.
-async function handleError(error: unknown, topic: string, defaultMessage: string) {
-    if (isUserCancelledError(error)) {
-        getLogger().verbose(`${topic}: user cancelled`)
-        return
-    }
-    const logsItem = localize('AWS.generic.message.viewLogs', 'View Logs...')
-    const logId = getLogger().error(`${topic}: %s`, error)
-    const message = resolveErrorMessageToDisplay(error, defaultMessage)
-
-    if (error instanceof ToolkitError && error.documentationUri) {
-        await showMessageWithUrl(message, error.documentationUri, 'View Documentation', 'error')
-    } else {
-        await vscode.window.showErrorMessage(message, logsItem).then(async resp => {
-            if (resp === logsItem) {
-                await Logging.declared.viewLogsAtMessage.execute(logId)
-            }
-        })
-    }
-}
-
 export async function deactivate() {
     await codewhispererShutdown()
     await globals.telemetry.shutdown()
@@ -365,6 +347,56 @@ function wrapWithProgressForCloud9(channel: vscode.OutputChannel): (typeof vscod
     }
 }
 
+/**
+ * Logs the error. Then determines what kind of error message should be shown, if
+ * at all.
+ *
+ * @param error The error itself
+ * @param topic The prefix of the error message
+ * @param defaultMessage The message to show if once cannot be resolved from the given error
+ *
+ * SIDE NOTE:
+ * This is only being used for errors from commands and webview, there's plenty of other places
+ * (explorer, nodes, ...) where it could be used. It needs to be apart of some sort of `core`
+ * module that is guaranteed to initialize prior to every other Toolkit component.
+ * Logging and telemetry would fit well within this core module.
+ */
+export async function logAndShowError(error: unknown, topic: string, defaultMessage: string) {
+    if (isUserCancelledError(error)) {
+        getLogger().verbose(`${topic}: user cancelled`)
+        return
+    }
+    const logsItem = localize('AWS.generic.message.viewLogs', 'View Logs...')
+    const logId = getLogger().error(`${topic}: %s`, error)
+    const message = resolveErrorMessageToDisplay(error, defaultMessage)
+
+    if (error instanceof ToolkitError && error.documentationUri) {
+        await showMessageWithUrl(message, error.documentationUri, 'View Documentation', 'error')
+    } else {
+        await vscode.window.showErrorMessage(message, logsItem).then(async resp => {
+            if (resp === logsItem) {
+                await Logging.declared.viewLogsAtMessage.execute(logId)
+            }
+        })
+    }
+}
+
+/**
+ * Show a webview related error to the user + button that links to the logged error
+ *
+ * @param err The error that was thrown in the backend
+ * @param webviewId Arbitrary value that identifies which webview had the error
+ * @param command The high level command/function that was run which triggered the error
+ */
+export function logAndShowWebviewError(err: unknown, webviewId: string, command: string) {
+    // HACK: The following implementation is a hack, influenced by the implementation of handleError().
+    // The userFacingError message will be seen in the UI, and the detailedError message will provide the
+    // detailed information in the logs.
+    const detailedError = ToolkitError.chain(err, `Webview backend command failed: "${command}()"`)
+    const userFacingError = ToolkitError.chain(detailedError, 'Webview error')
+    logAndShowError(userFacingError, `webviewId="${webviewId}"`, 'Webview error')
+}
+
 // Unique extension entrypoint names, so that they can be obtained from the webpack bundle
 export const awsToolkitActivate = activate
 export const awsToolkitDeactivate = deactivate

src/shared/logger/commands.ts

@@ -43,6 +43,11 @@ export class Logging {
             return
         }
 
+        // HACK: editor.document.getText() may return "stale" content, then
+        // subsequent calls to openLogId() fail to highlight the specific log.
+        // Invoke "revert" on the current file to force vscode to read from disk.
+        await vscode.commands.executeCommand('workbench.action.files.revert')
+
         // Retrieve where the message starts by counting number of newlines
         const text = editor.document.getText()
         const textStart = text.indexOf(msg)

src/shared/logger/sharedFileTransport.ts

@@ -52,7 +52,7 @@ export class SharedFileTransport extends TransportStream {
 
     /**
      * @returns a promise that resolves once a batch of logs are written to
-     *          the log file. 
+     *          the log file.
      */
     public override log(logEntry: LogEntry, next: () => void): Promise<void> {
         this.bufferedLogEntries.push(logEntry)

src/test/webview/server.test.ts

@@ -0,0 +1,52 @@
+/*!
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { SinonStub, stub } from 'sinon'
+import { Logger, setLogger } from '../../shared/logger/logger'
+import assert from 'assert'
+import { ToolkitError } from '../../shared/errors'
+import { handleWebviewError } from '../../webviews/server'
+
+describe('logAndShowWebviewError()', function () {
+    let logError: SinonStub<[message: string, ...meta: any[]], number>
+    const myWebviewId = 'myWebviewId'
+    const myCommand = 'myCommand'
+
+    beforeEach(function () {
+        logError = stub()
+        const logger = { error: logError } as unknown as Logger
+        setLogger(logger, 'main')
+    })
+
+    afterEach(function () {
+        setLogger(undefined, 'main')
+    })
+
+    it('logs the provided error, but is wrapped in ToolkitErrors for more context', function () {
+        // The method is being tested due to its fragile implementation. This test
+        // protects against changes in the underlying logAndShowError() implementation.
+
+        const inputError = new Error('Random Error')
+
+        handleWebviewError(inputError, myWebviewId, myCommand)
+
+        assert.strictEqual(logError.callCount, 1)
+
+        // A shortened error is shown to the user
+        const userFacingError = logError.getCall(0).args[1]
+        assert(userFacingError instanceof ToolkitError)
+        assert.strictEqual(userFacingError.message, 'Webview error')
+
+        // A higher level context of what caused the error
+        const detailedError = userFacingError.cause
+        assert(detailedError instanceof ToolkitError)
+        assert.strictEqual(detailedError.message, `Webview backend command failed: "${myCommand}()"`)
+
+        // The actual error
+        const rootError = detailedError.cause
+        assert(rootError instanceof Error)
+        assert.strictEqual(rootError, inputError)
+    })
+})

src/webviews/main.ts

@@ -227,7 +227,7 @@ export abstract class VueWebview {
                     webviewJs: this.instance.source,
                     ...params,
                 })
-                const server = registerWebviewServer(panel.webview, this.instance.protocol)
+                const server = registerWebviewServer(panel.webview, this.instance.protocol, this.instance.id)
                 this.instance.onDidDispose(() => {
                     server.dispose()
                     this.panel?.dispose()
@@ -282,7 +282,11 @@ export abstract class VueWebview {
                         if (!this.resolvedView) {
                             this.resolvedView = view
 
-                            const server = registerWebviewServer(this.resolvedView.webview, this.instance.protocol)
+                            const server = registerWebviewServer(
+                                this.resolvedView.webview,
+                                this.instance.protocol,
+                                this.instance.id
+                            )
                             this.resolvedView.onDidDispose(() => {
                                 server.dispose()
                                 this.resolvedView = undefined

src/webviews/server.ts

@@ -21,8 +21,13 @@ export interface Protocol {
  *
  * @param webview Target webview to add the event hook.
  * @param commands Commands to register.
+ * @param webviewId A human readable string that will identify the webview
  */
-export function registerWebviewServer(webview: vscode.Webview, commands: Protocol): vscode.Disposable {
+export function registerWebviewServer(
+    webview: vscode.Webview,
+    commands: Protocol,
+    webviewId: string
+): vscode.Disposable {
     const eventListeners: vscode.Disposable[] = []
     const disposeListeners = () => {
         while (eventListeners.length) {
@@ -64,13 +69,16 @@ export function registerWebviewServer(webview: vscode.Webview, commands: Protoco
                 result = await handler.call(webview, ...data)
             } catch (err) {
                 if (!(err instanceof Error)) {
-                    getLogger().debug(`Webview server threw on comamnd "${command}" but it was not an error: `, err)
+                    getLogger().debug(`Webview server threw on command "${command}" but it was not an error: `, err)
                     return
                 }
                 result = JSON.stringify(err, Object.getOwnPropertyNames(err))
                 delete result.stack // Not relevant to frontend code, we only care about the message
                 metadata.error = true
-                getLogger().debug(`Webview server failed on command "${command}": %s`, err.message)
+
+                // A command failed in the backend/server, this will surface the error to the user as a vscode error message.
+                // This is the base error handler that will end up catching all unhandled errors.
+                handleWebviewError(err, webviewId, command)
             }
 
             // TODO: check if webview has been disposed of before posting message (not necessary but nice)
@@ -81,3 +89,24 @@ export function registerWebviewServer(webview: vscode.Webview, commands: Protoco
 
     return { dispose: () => (messageListener.dispose(), disposeListeners()) }
 }
+
+let errorHandler: (error: unknown, webviewId: string, command: string) => void
+/**
+ * Registers the handler for errors thrown by the webview backend/server code.
+ */
+export function registerWebviewErrorHandler(handler: typeof errorHandler): void {
+    if (errorHandler !== undefined) {
+        throw new TypeError('Webview error handler has already been registered')
+    }
+
+    errorHandler = handler
+}
+/**
+ * Invokes the registered webview error handler.
+ */
+export function handleWebviewError(error: unknown, webviewId: string, command: string): void {
+    if (errorHandler === undefined) {
+        throw new Error('Webview error handler not registered')
+    }
+    return errorHandler(error, webviewId, command)
+}

src/webviews/util.ts

@@ -3,6 +3,13 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+/**
+ * IMPORTANT:
+ *
+ * This class is used in the frontend vue.js webview code.
+ * Do not put backend code in this module.
+ */
+
 import { PropType } from 'vue'
 
 /**