fixup! feat(logging): support logger instances

Author: justinmk3

packages/core/src/codewhisperer/activation.ts

@@ -114,7 +114,7 @@ export async function activate(context: ExtContext): Promise<void> {
 
     // TODO: this is already done in packages/core/src/extensionCommon.ts, why doesn't amazonq use that?
     registerWebviewErrorHandler((error: unknown, webviewId: string, command: string) => {
-        logAndShowWebviewError(localize, error, webviewId, command)
+        return logAndShowWebviewError(localize, error, webviewId, command)
     })
 
     /**

packages/core/src/extension.ts

@@ -83,7 +83,7 @@ export async function activateCommon(
     })
 
     registerWebviewErrorHandler((error: unknown, webviewId: string, command: string) => {
-        logAndShowWebviewError(localize, error, webviewId, command)
+        return logAndShowWebviewError(localize, error, webviewId, command)
     })
 
     // Setup the logger

packages/core/src/shared/utilities/logAndShowUtils.ts

@@ -52,6 +52,8 @@ export async function logAndShowError(
  * @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
+ *
+ * @returns user-facing error
  */
 export function logAndShowWebviewError(localize: nls.LocalizeFunc, err: unknown, webviewId: string, command: string) {
     // HACK: The following implementation is a hack, influenced by the implementation of handleError().
@@ -62,4 +64,6 @@ export function logAndShowWebviewError(localize: nls.LocalizeFunc, err: unknown,
     logAndShowError(localize, userFacingError, `webviewId="${webviewId}"`, 'Webview error').catch((e) => {
         getLogger().error('logAndShowError failed: %s', (e as Error).message)
     })
+
+    return userFacingError
 }

packages/core/src/test/codewhisperer/commands/basicCommands.test.ts

@@ -28,7 +28,6 @@ import { stub } from '../../utilities/stubber'
 import { AuthUtil } from '../../../codewhisperer/util/authUtil'
 import { getTestWindow } from '../../shared/vscode/window'
 import { ExtContext } from '../../../shared/extensions'
-import { getLogger } from '../../../shared/logger/logger'
 import {
     createAutoScans,
     createAutoSuggestions,
@@ -57,6 +56,7 @@ import * as diagnosticsProvider from '../../../codewhisperer/service/diagnostics
 import { SecurityIssueHoverProvider } from '../../../codewhisperer/service/securityIssueHoverProvider'
 import { SecurityIssueCodeActionProvider } from '../../../codewhisperer/service/securityIssueCodeActionProvider'
 import { randomUUID } from '../../../shared/crypto'
+import { assertLogsContain } from '../../globalSetup.test'
 
 describe('CodeWhisperer-basicCommands', function () {
     let targetCommand: Command<any> & vscode.Disposable
@@ -636,7 +636,6 @@ describe('CodeWhisperer-basicCommands', function () {
             openTextDocumentMock.resolves(textDocumentMock)
 
             sandbox.stub(vscode.workspace, 'openTextDocument').value(openTextDocumentMock)
-            const loggerStub = sinon.stub(getLogger(), 'error')
 
             sinon.stub(vscode.WorkspaceEdit.prototype, 'replace').value(replaceMock)
             applyEditMock.resolves(false)
@@ -652,9 +651,7 @@ describe('CodeWhisperer-basicCommands', function () {
             await targetCommand.execute(codeScanIssue, fileName, 'quickfix')
 
             assert.ok(replaceMock.calledOnce)
-            assert.ok(loggerStub.calledOnce)
-            const actual = loggerStub.getCall(0).args[0]
-            assert.strictEqual(actual, 'Apply fix command failed. Error: Failed to apply edit to the workspace.')
+            assertLogsContain('Apply fix command failed. Error: Failed to apply edit to the workspace.', true, 'error')
             assertTelemetry('codewhisperer_codeScanIssueApplyFix', {
                 detectorId: codeScanIssue.detectorId,
                 findingId: codeScanIssue.findingId,

packages/core/src/test/globalSetup.test.ts

@@ -169,18 +169,17 @@ async function writeLogsToFile(testName: string) {
 
 // TODO: merge this with `toolkitLogger.test.ts:checkFile`
 export function assertLogsContain(text: string, exactMatch: boolean, severity: LogLevel) {
+    const logs = getTestLogger().getLoggedEntries(severity)
     assert.ok(
-        getTestLogger()
-            .getLoggedEntries(severity)
-            .some((e) =>
-                e instanceof Error
-                    ? exactMatch
-                        ? e.message === text
-                        : e.message.includes(text)
-                    : exactMatch
-                      ? e === text
-                      : e.includes(text)
-            ),
+        logs.some((e) =>
+            e instanceof Error
+                ? exactMatch
+                    ? e.message === text
+                    : e.message.includes(text)
+                : exactMatch
+                  ? e === text
+                  : e.includes(text)
+        ),
         `Expected to find "${text}" in the logs as type "${severity}"`
     )
 }

packages/core/src/test/lambda/vue/remoteInvoke/invokeLambda.test.ts

@@ -16,11 +16,11 @@ import { getTestWindow } from '../../../shared/vscode/window'
 import { LambdaFunctionNode } from '../../../../lambda/explorer/lambdaFunctionNode'
 import * as utils from '../../../../lambda/utils'
 import { HttpResourceFetcher } from '../../../../shared/resourcefetcher/httpResourceFetcher'
-import { getLogger } from '../../../../shared/logger'
 import { ExtContext } from '../../../../shared/extensions'
 import { FakeExtensionContext } from '../../../fakeExtensionContext'
 import * as samCliRemoteTestEvent from '../../../../shared/sam/cli/samCliRemoteTestEvent'
 import { TestEventsOperation, SamCliRemoteTestEventsParameters } from '../../../../shared/sam/cli/samCliRemoteTestEvent'
+import { assertLogsContain } from '../../../globalSetup.test'
 
 describe('RemoteInvokeWebview', () => {
     let outputChannel: vscode.OutputChannel
@@ -190,20 +190,11 @@ describe('RemoteInvokeWebview', () => {
 
             getTestWindow().onDidShowDialog((d) => d.selectItem(fileUri))
 
-            const loggerErrorStub = sinon.stub(getLogger(), 'error')
-
-            try {
-                await assert.rejects(
-                    async () => await remoteInvokeWebview.promptFile(),
-                    new Error('Failed to read selected file')
-                )
-                assert.strictEqual(loggerErrorStub.calledOnce, true)
-                assert.strictEqual(loggerErrorStub.firstCall.args[0], 'readFileSync: Failed to read file at path %s %O')
-                assert.strictEqual(loggerErrorStub.firstCall.args[1], fileUri.fsPath)
-                assert(loggerErrorStub.firstCall.args[2] instanceof Error)
-            } finally {
-                loggerErrorStub.restore()
-            }
+            await assert.rejects(
+                async () => await remoteInvokeWebview.promptFile(),
+                new Error('Failed to read selected file')
+            )
+            assertLogsContain('readFileSync: Failed to read file at path %s %O', true, 'error')
         })
     })
 

packages/core/src/test/lambda/vue/samInvokeBackend.test.ts

@@ -22,12 +22,12 @@ import path from 'path'
 import { addCodiconToString, fs, makeTemporaryToolkitFolder } from '../../../shared'
 import { LaunchConfiguration } from '../../../shared/debug/launchConfiguration'
 import { getTestWindow } from '../..'
-import { getLogger } from '../../../shared/logger'
 import * as extensionUtilities from '../../../shared/extensionUtilities'
 import * as samInvokeBackend from '../../../lambda/vue/configEditor/samInvokeBackend'
 import { SamDebugConfigProvider } from '../../../shared/sam/debugger/awsSamDebugger'
 import sinon from 'sinon'
 import * as nls from 'vscode-nls'
+import { assertLogsContain } from '../../../test/globalSetup.test'
 
 const localize = nls.loadMessageBundle()
 
@@ -438,19 +438,13 @@ describe('SamInvokeWebview', () => {
 
             getTestWindow().onDidShowDialog((window) => window.selectItem(fileUri))
 
-            const loggerErrorStub = sinon.stub(getLogger(), 'error')
-
             try {
                 await assert.rejects(
                     async () => await samInvokeWebview.promptFile(),
                     new Error('Failed to read selected file')
                 )
-                assert.strictEqual(loggerErrorStub.calledOnce, true)
-                assert.strictEqual(loggerErrorStub.firstCall.args[0], 'readFileSync: Failed to read file at path %s %O')
-                assert.strictEqual(loggerErrorStub.firstCall.args[1], fileUri.fsPath)
-                assert(loggerErrorStub.firstCall.args[2] instanceof Error)
+                assertLogsContain('readFileSync: Failed to read file at path %s %O', true, 'error')
             } finally {
-                loggerErrorStub.restore()
                 await fs.delete(tempFolder, { recursive: true })
             }
         })

packages/core/src/test/shared/applicationBuilder/explorer/nodes/deployedNode.test.ts

@@ -19,23 +19,19 @@ import { LambdaFunctionNode } from '../../../../../lambda/explorer/lambdaFunctio
 import { RestApiNode } from '../../../../../awsService/apigateway/explorer/apiNodes'
 import { S3BucketNode } from '../../../../../awsService/s3/explorer/s3BucketNode'
 import * as LambdaNodeModule from '../../../../../lambda/explorer/lambdaNodes'
-import { getLogger } from '../../../../../shared/logger/logger'
 import { getIcon } from '../../../../../shared/icons'
 import _ from 'lodash'
 import { isTreeNode } from '../../../../../shared/treeview/resourceTreeDataProvider'
 import { Any } from '../../../../../shared/utilities/typeConstructors'
 import { IamConnection, ProfileMetadata } from '../../../../../auth/connection'
 import * as AuthUtils from '../../../../../auth/utils'
+import { assertLogsContain } from '../../../../../test/globalSetup.test'
 
 describe('DeployedResourceNode', () => {
     const expectedStackName = 'myStack'
     const expectedRegionCode = 'us-west-2'
-    let loggerWarnStub: sinon.SinonStub<[message: string | Error, ...meta: any[]], number>
 
-    beforeEach(() => {
-        // Create a stub for the entire logger module
-        loggerWarnStub = sinon.stub(getLogger(), 'warn')
-    })
+    beforeEach(() => {})
 
     afterEach(() => {
         // Restore the original function after each test
@@ -87,8 +83,7 @@ describe('DeployedResourceNode', () => {
                 assert.strictEqual(deployedLambdaNode.id, '')
                 assert.strictEqual(deployedLambdaNode.contextValue, '')
                 assert.deepStrictEqual(deployedLambdaNode.resource, emptyArnDeployedResource)
-                assert(loggerWarnStub.calledOnce)
-                assert(loggerWarnStub.calledWith('Cannot create DeployedResourceNode, the ARN does not exist.'))
+                assertLogsContain('Cannot create DeployedResourceNode, the ARN does not exist.', false, 'warn')
             })
         })
     })
@@ -121,17 +116,13 @@ describe('DeployedResourceNode', () => {
 describe('generateDeployedNode', () => {
     const expectedStackName = 'myStack'
     const expectedRegionCode = 'us-west-2'
-    let loggerErrorStub: sinon.SinonStub<[message: string | Error, ...meta: any[]], number>
-    let loggerInfoStub: sinon.SinonStub<[message: string | Error, ...meta: any[]], number>
 
     let sandbox: sinon.SinonSandbox
 
     beforeEach(() => {
         // Initiate stub sanbox
         sandbox = sinon.createSandbox()
         // Create a stub for the entire logger module
-        loggerErrorStub = sandbox.stub(getLogger(), 'error')
-        loggerInfoStub = sandbox.stub(getLogger(), 'info')
     })
     afterEach(async () => {
         sandbox.restore()
@@ -223,8 +214,7 @@ describe('generateDeployedNode', () => {
                 lambdaDeployedNodeInput.resourceTreeEntity
             )
 
-            assert(loggerErrorStub.calledOnceWith('Error getting Lambda configuration %O'))
-            assert(loggerErrorStub.neverCalledWith('Error getting Lambda V3 configuration %O'))
+            assertLogsContain('Error getting Lambda configuration %O', true, 'error')
             assert(deployedResourceNodes.length === 1)
 
             // Check placeholder propertries
@@ -375,10 +365,7 @@ describe('generateDeployedNode', () => {
                 unsupportTypeInput.resourceTreeEntity
             )
 
-            assert(loggerInfoStub.calledOnceWith('Details are missing or are incomplete for: %O'))
-
-            // Check deployedResourceNodes array propertries
-            assert(loggerErrorStub.neverCalledWith('Error getting Lambda V3 configuration %O'))
+            assertLogsContain('Details are missing or are incomplete for:', false, 'info')
 
             // Check deployedResourceNodes array propertries
             assert(deployedResourceNodes.length === 1)

packages/core/src/test/webview/server.test.ts

@@ -3,44 +3,34 @@
  * 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'
+import { assertLogsContain } from '../globalSetup.test'
 
 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')
-    })
+    beforeEach(function () {})
 
-    afterEach(function () {
-        setLogger(undefined, 'main')
-    })
+    afterEach(function () {})
 
     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)
+        const err = handleWebviewError(inputError, myWebviewId, myCommand)
 
-        assert.strictEqual(logError.callCount, 1)
+        // assertLogsContain('Random Error', false, 'error')
 
         // 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')
+        assertLogsContain('Webview error', false, 'error')
 
         // A higher level context of what caused the error
-        const detailedError = userFacingError.cause
+        const detailedError = err.cause
         assert(detailedError instanceof ToolkitError)
         assert.strictEqual(detailedError.message, `Webview backend command failed: "${myCommand}()"`)
 

packages/core/src/webviews/server.ts

@@ -7,6 +7,7 @@ import * as vscode from 'vscode'
 import { getLogger } from '../shared/logger'
 import { Message } from './client'
 import { AsyncResource } from 'async_hooks'
+import { ToolkitError } from '../shared/errors'
 
 interface Command<T extends any[] = any, R = any> {
     (...args: T): R | never
@@ -90,7 +91,12 @@ export function registerWebviewServer(
     return { dispose: () => (messageListener.dispose(), disposeListeners()) }
 }
 
-let errorHandler: (error: unknown, webviewId: string, command: string) => void
+/**
+ * Logs and handles an error.
+ *
+ * @returns final, chained, user-facing error
+ */
+let errorHandler: (error: unknown, webviewId: string, command: string) => ToolkitError
 /**
  * Registers the handler for errors thrown by the webview backend/server code.
  */
@@ -104,7 +110,7 @@ export function registerWebviewErrorHandler(handler: typeof errorHandler): void
 /**
  * Invokes the registered webview error handler.
  */
-export function handleWebviewError(error: unknown, webviewId: string, command: string): void {
+export function handleWebviewError(error: unknown, webviewId: string, command: string): ToolkitError {
     if (errorHandler === undefined) {
         throw new Error('Webview error handler not registered')
     }