Merge master into feature/cwltail

aws · Nov 7, 2024 · 50cbeee · 50cbeee
2 parents 069c600 + 4add487
commit 50cbeee
Show file tree

Hide file tree

Showing 33 changed files with 133 additions and 36 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -163,6 +163,7 @@ module.exports = {
         'aws-toolkits/no-string-exec-for-child-process': 'error',
         'aws-toolkits/no-console-log': 'error',
         'aws-toolkits/no-json-stringify-in-log': 'error',
+        'aws-toolkits/no-printf-mismatch': 'error',
         'no-restricted-imports': [
             'error',
             {

diff --git a/packages/core/src/applicationcomposer/messageHandlers/emitTelemetryMessageHandler.ts b/packages/core/src/applicationcomposer/messageHandlers/emitTelemetryMessageHandler.ts
@@ -67,7 +67,7 @@ export function emitTelemetryMessageHandler(message: EmitTelemetryMessage) {
                 return
         }
     } catch (e) {
-        getLogger().error('Could not log telemetry for App Composer', e)
+        getLogger().error('Could not log telemetry for App Composer %O', e)
     }
 }
 

diff --git a/packages/core/src/awsService/appBuilder/explorer/nodes/deployedNode.ts b/packages/core/src/awsService/appBuilder/explorer/nodes/deployedNode.ts
@@ -96,7 +96,7 @@ export async function generateDeployedNode(
                         .Configuration as Lambda.FunctionConfiguration
                     newDeployedResource = new LambdaFunctionNode(lambdaNode, regionCode, configuration)
                 } catch (error: any) {
-                    getLogger().error('Error getting Lambda configuration')
+                    getLogger().error('Error getting Lambda configuration %O', error)
                     throw ToolkitError.chain(error, 'Error getting Lambda configuration', {
                         code: 'lambdaClientError',
                     })
@@ -153,7 +153,7 @@ export async function generateDeployedNode(
             }
             default:
                 newDeployedResource = new DeployedResourceNode(deployedResource)
-                getLogger().info('Details are missing or are incomplete for:', deployedResource)
+                getLogger().info('Details are missing or are incomplete for: %O', deployedResource)
                 return [
                     createPlaceholderItem(
                         localize('AWS.appBuilder.explorerNode.noApps', '[This resource is not yet supported.]')

diff --git a/packages/core/src/awsService/appBuilder/explorer/samProject.ts b/packages/core/src/awsService/appBuilder/explorer/samProject.ts
@@ -42,7 +42,7 @@ export async function getStackName(projectRoot: vscode.Uri): Promise<any> {
     } catch (error: any) {
         switch (error.code) {
             case SamConfigErrorCode.samNoConfigFound:
-                getLogger().info('No stack name or region information available in samconfig.toml', error)
+                getLogger().info('No stack name or region information available in samconfig.toml: %O', error)
                 break
             case SamConfigErrorCode.samConfigParseError:
                 getLogger().error(`Error getting stack name or region information: ${error.message}`, error)

diff --git a/packages/core/src/awsService/iot/commands/createCert.ts b/packages/core/src/awsService/iot/commands/createCert.ts
@@ -115,14 +115,14 @@ async function saveCredentials(
     const publicKeyExists = await fileExists(publicKeyPath)
 
     if (certExists) {
-        getLogger().error('Certificate path {0} already exists', certPath)
+        getLogger().error('Certificate path %s already exists', certPath)
         void vscode.window.showErrorMessage(
             localize('AWS.iot.createCert.error', 'Failed to create certificate. Path {0} already exists.', certPath)
         )
         return false
     }
     if (privateKeyExists) {
-        getLogger().error('Key path {0} already exists', privateKeyPath)
+        getLogger().error('Key path %s already exists', privateKeyPath)
         void vscode.window.showErrorMessage(
             localize(
                 'AWS.iot.createCert.error',
@@ -133,7 +133,7 @@ async function saveCredentials(
         return false
     }
     if (publicKeyExists) {
-        getLogger().error('Key path {0} already exists', publicKeyPath)
+        getLogger().error('Key path %s already exists', publicKeyPath)
         void vscode.window.showErrorMessage(
             localize(
                 'AWS.iot.createCert.error',

diff --git a/packages/core/src/awsService/redshift/explorer/redshiftNode.ts b/packages/core/src/awsService/redshift/explorer/redshiftNode.ts
@@ -127,7 +127,7 @@ export class RedshiftNode extends AWSTreeNodeBase implements LoadMoreNode {
                     newServerlessToken = response.nextToken ?? ''
                 }
             } catch (error) {
-                getLogger().error("Serverless workgroup operation isn't supported or failed:", error)
+                getLogger().error("Serverless workgroup operation isn't supported or failed: %O", error)
                 // Continue without interrupting the provisioned cluster loading
             }
         }

diff --git a/packages/core/src/awsService/s3/commands/uploadFile.ts b/packages/core/src/awsService/s3/commands/uploadFile.ts
@@ -415,7 +415,7 @@ export async function promptUserForBucket(
     try {
         allBuckets = await s3client.listAllBuckets()
     } catch (e) {
-        getLogger().error('Failed to list buckets from client', e)
+        getLogger().error('Failed to list buckets from client %O', e)
         void vscode.window.showErrorMessage(
             localize('AWS.message.error.promptUserForBucket.listBuckets', 'Failed to list buckets from client')
         )

diff --git a/packages/core/src/codewhisperer/activation.ts b/packages/core/src/codewhisperer/activation.ts
@@ -618,6 +618,6 @@ export async function enableDefaultConfigCloud9() {
         await editorSettings.update('acceptSuggestionOnEnter', 'on', vscode.ConfigurationTarget.Global)
         await editorSettings.update('snippetSuggestions', 'top', vscode.ConfigurationTarget.Global)
     } catch (error) {
-        getLogger().error('amazonq: Failed to update user settings', error)
+        getLogger().error('amazonq: Failed to update user settings %O', error)
     }
 }
diff --git a/packages/core/src/codewhisperer/client/agent.ts b/packages/core/src/codewhisperer/client/agent.ts
@@ -82,6 +82,6 @@ export function initializeNetworkAgent(): void {
         }
     } catch (error) {
         // Log any errors in the patching logic
-        getLogger().error('Failed to patch http agent', error)
+        getLogger().error('Failed to patch http agent %O', error)
     }
 }
diff --git a/packages/core/src/codewhisperer/commands/startSecurityScan.ts b/packages/core/src/codewhisperer/commands/startSecurityScan.ts
@@ -225,7 +225,7 @@ export async function startSecurityScan(
 
         logger.verbose(`Security scan completed.`)
     } catch (error) {
-        getLogger().error('Security scan failed.', error)
+        getLogger().error('Security scan failed. %O', error)
         if (error instanceof CodeScanStoppedError) {
             codeScanTelemetryEntry.result = 'Cancelled'
         } else {

diff --git a/packages/core/src/codewhisperer/commands/startTransformByQ.ts b/packages/core/src/codewhisperer/commands/startTransformByQ.ts
@@ -142,7 +142,7 @@ export async function validateSQLMetadataFile(fileContents: string, message: any
             `CodeTransformation: Parsed .sct file with source DB: ${sourceDB}, target DB: ${targetDB}, source host name: ${sourceServerName}, and schema names: ${Array.from(schemaNames)}`
         )
     } catch (err: any) {
-        getLogger().error('CodeTransformation: Error parsing .sct file.', err)
+        getLogger().error('CodeTransformation: Error parsing .sct file. %O', err)
         transformByQState.getChatMessenger()?.sendUnrecoverableErrorResponse('error-parsing-sct-file', message.tabID)
         return false
     }

diff --git a/packages/core/src/codewhisperer/util/zipUtil.ts b/packages/core/src/codewhisperer/util/zipUtil.ts
@@ -259,7 +259,7 @@ export class ZipUtil {
                 language: this._language,
             }
         } catch (error) {
-            getLogger().error('Zip error caused by:', error)
+            getLogger().error('Zip error caused by: %O', error)
             throw error
         }
     }

diff --git a/packages/core/src/lambda/vue/configEditor/samInvokeBackend.ts b/packages/core/src/lambda/vue/configEditor/samInvokeBackend.ts
@@ -269,7 +269,7 @@ export class SamInvokeWebview extends VueWebview {
                 selectedFile: this.getFileName(fileLocations[0].fsPath),
             }
         } catch (e) {
-            getLogger().error('readFileSync: Failed to read file at path %O', fileLocations[0].fsPath, e)
+            getLogger().error('readFileSync: Failed to read file at path %s %O', fileLocations[0].fsPath, e)
             throw ToolkitError.chain(e, 'Failed to read selected file')
         }
     }

diff --git a/packages/core/src/lambda/vue/remoteInvoke/invokeLambda.ts b/packages/core/src/lambda/vue/remoteInvoke/invokeLambda.ts
@@ -128,7 +128,7 @@ export class RemoteInvokeWebview extends VueWebview {
                 selectedFile: this.getFileName(fileLocations[0].fsPath),
             }
         } catch (e) {
-            getLogger().error('readFileSync: Failed to read file at path %O', fileLocations[0].fsPath, e)
+            getLogger().error('readFileSync: Failed to read file at path %s %O', fileLocations[0].fsPath, e)
             throw ToolkitError.chain(e, 'Failed to read selected file')
         }
     }
@@ -151,7 +151,7 @@ export class RemoteInvokeWebview extends VueWebview {
                 selectedFile: this.getFileName(fileLocation.fsPath),
             }
         } catch (e) {
-            getLogger().error('readFileSync: Failed to read file at path %O', fileLocation.fsPath, e)
+            getLogger().error('readFileSync: Failed to read file at path %s %O', fileLocation.fsPath, e)
             throw ToolkitError.chain(e, 'Failed to read selected file')
         }
     }

diff --git a/packages/core/src/lambda/wizards/samDeployWizard.ts b/packages/core/src/lambda/wizards/samDeployWizard.ts
@@ -992,7 +992,7 @@ async function populateS3QuickPick(
                 })
             }
         } catch (e) {
-            getLogger().error('Recent bucket JSON not parseable.', e)
+            getLogger().error('Recent bucket JSON not parseable: %O', e)
         }
 
         if (isCloud9() && recent !== cloud9Bucket) {

diff --git a/packages/core/src/login/webview/vue/toolkit/backend_toolkit.ts b/packages/core/src/login/webview/vue/toolkit/backend_toolkit.ts
@@ -96,7 +96,7 @@ export class ToolkitLoginWebview extends CommonAuthWebview {
                 await setContext('aws.explorer.showAuthView', false)
                 await this.showResourceExplorer()
             } catch (e) {
-                getLogger().error('Failed submitting credentials', e)
+                getLogger().error('Failed submitting credentials %O', e)
                 return { id: this.id, text: e as string }
             }
         }

diff --git a/packages/core/src/shared/env/resolveEnv.ts b/packages/core/src/shared/env/resolveEnv.ts
@@ -303,7 +303,7 @@ async function doResolveUnixShellEnv(timeout: Timeout): Promise<typeof process.e
                 getLogger().debug(`getUnixShellEnvironment#result:${env}`)
                 resolve(env)
             } catch (err) {
-                getLogger().error('getUnixShellEnvironment#errorCaught', err)
+                getLogger().error('getUnixShellEnvironment#errorCaught %O', err)
                 reject(err)
             }
         })

diff --git a/packages/core/src/shared/filesystemUtilities.ts b/packages/core/src/shared/filesystemUtilities.ts
@@ -245,7 +245,7 @@ export async function setDefaultDownloadPath(downloadPath: string) {
             await globals.globalState.update('aws.downloadPath', path.dirname(downloadPath))
         }
     } catch (err) {
-        getLogger().error('Error while setting "aws.downloadPath"', err as Error)
+        getLogger().error('Error while setting "aws.downloadPath": %O', err as Error)
     }
 }
 

diff --git a/packages/core/src/shared/sam/activation.ts b/packages/core/src/shared/sam/activation.ts
@@ -188,7 +188,7 @@ async function activateCodeLensRegistry(context: ExtContext) {
                 getIdeProperties().codelenses
             )
         )
-        getLogger().error('Failed to activate codelens registry', e)
+        getLogger().error('Failed to activate codelens registry %O', e)
         // This prevents us from breaking for any reason later if it fails to load. Since
         // Noop watcher is always empty, we will get back empty arrays with no issues.
         globals.codelensRootRegistry = new NoopWatcher() as unknown as CodelensRootRegistry

diff --git a/packages/core/src/shared/sam/cli/samCliLocalInvoke.ts b/packages/core/src/shared/sam/cli/samCliLocalInvoke.ts
@@ -62,7 +62,7 @@ export class DefaultSamLocalInvokeCommand implements SamLocalInvokeCommand {
         const childProcess = new ChildProcess(params.command, params.args, {
             spawnOptions: await addTelemetryEnvVar(options),
         })
-        getLogger().info('AWS.running.command', 'Command: {0}', `${childProcess}`)
+        getLogger().info('AWS.running.command: Command: %O', childProcess)
         // "sam local invoke", "sam local start-api", etc.
         const samCommandName = `sam ${params.args[0]} ${params.args[1]}`
 

diff --git a/packages/core/src/shared/ui/picker.ts b/packages/core/src/shared/ui/picker.ts
@@ -310,7 +310,7 @@ export class IteratingQuickPickController<TResponse> {
                                 // give quickpick item an error message
                                 // we should not blow away the existing items, they should still be viable
                                 const err = e as Error
-                                getLogger().error('Error while loading items for IteratingQuickPickController:', err)
+                                getLogger().error('Error while loading items for IteratingQuickPickController: %O', err)
                                 resolve({
                                     value: [
                                         {

diff --git a/packages/core/src/shared/utilities/zipStream.ts b/packages/core/src/shared/utilities/zipStream.ts
@@ -135,7 +135,7 @@ export class ZipStream {
         while (!finished) {
             finished = await new Promise((resolve) => {
                 setTimeout(() => {
-                    getLogger().verbose('success is', this._numberOfFilesSucceeded, '/', this._numberOfFilesToStream)
+                    getLogger().verbose(`success is ${this._numberOfFilesSucceeded}/${this._numberOfFilesToStream}`)
                     onProgress?.(Math.floor((100 * this._numberOfFilesSucceeded) / this._numberOfFilesToStream))
                     resolve(this._numberOfFilesToStream <= this._numberOfFilesSucceeded)
                 }, 1000)

diff --git a/packages/core/src/ssmDocument/commands/deleteDocument.ts b/packages/core/src/ssmDocument/commands/deleteDocument.ts
@@ -60,7 +60,7 @@ export async function deleteDocument(node: DocumentItemNodeWriteable) {
     } catch (err) {
         result = 'Failed'
         const error = err as Error
-        logger.error('Error on deleting document: %0', error)
+        logger.error('Error on deleting document: %O', error)
         void showViewLogsMessage(
             localize(
                 'AWS.message.error.ssmDocument.deleteDocument.could_not_delete',

diff --git a/packages/core/src/ssmDocument/commands/openDocumentItem.ts b/packages/core/src/ssmDocument/commands/openDocumentItem.ts
@@ -44,7 +44,7 @@ export async function openDocumentItem(node: DocumentItemNode, awsContext: AwsCo
     } catch (err) {
         result = 'Failed'
         const error = err as Error
-        logger.error('Error on opening document: %0', error)
+        logger.error('Error on opening document: %O', error)
         void showViewLogsMessage(
             localize(
                 'AWS.message.error.ssmDocument.openDocument.could_not_open',

diff --git a/packages/core/src/ssmDocument/commands/updateDocumentVersion.ts b/packages/core/src/ssmDocument/commands/updateDocumentVersion.ts
@@ -70,7 +70,7 @@ export async function updateDocumentVersion(node: DocumentItemNodeWriteable, aws
                 node.documentName
             )
         )
-        logger.error('Error on updating document version: %0', error)
+        logger.error('Error on updating document version: %O', error)
     } finally {
         telemetry.ssm_updateDocumentVersion.emit({ result: result })
     }

diff --git a/packages/core/src/test/awsService/appBuilder/samProject.test.ts b/packages/core/src/test/awsService/appBuilder/samProject.test.ts
@@ -78,7 +78,7 @@ describe('samProject', () => {
             // simulate error when no samconfig.toml file in directory
             const result = await getStackName(projectRoot)
             assert.deepStrictEqual(result, {})
-            assertLogsContain('No stack name or region information available in samconfig.toml', true, 'info')
+            assertLogsContain('No stack name or region information available in samconfig.toml: %O', true, 'info')
         })
 
         it('returns empty object give error parsing samconfig file', async () => {

diff --git a/packages/core/src/test/lambda/vue/remoteInvoke/invokeLambda.test.ts b/packages/core/src/test/lambda/vue/remoteInvoke/invokeLambda.test.ts
@@ -198,7 +198,7 @@ describe('RemoteInvokeWebview', () => {
                     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 %O')
+                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 {

diff --git a/packages/core/src/test/lambda/vue/samInvokeBackend.test.ts b/packages/core/src/test/lambda/vue/samInvokeBackend.test.ts
@@ -446,7 +446,7 @@ describe('SamInvokeWebview', () => {
                     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 %O')
+                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 {

diff --git a/packages/core/src/test/shared/applicationBuilder/explorer/nodes/deployedNode.test.ts b/packages/core/src/test/shared/applicationBuilder/explorer/nodes/deployedNode.test.ts
@@ -223,8 +223,8 @@ describe('generateDeployedNode', () => {
                 lambdaDeployedNodeInput.resourceTreeEntity
             )
 
-            assert(loggerErrorStub.calledOnceWith('Error getting Lambda configuration'))
-            assert(loggerErrorStub.neverCalledWith('Error getting Lambda V3 configuration'))
+            assert(loggerErrorStub.calledOnceWith('Error getting Lambda configuration %O'))
+            assert(loggerErrorStub.neverCalledWith('Error getting Lambda V3 configuration %O'))
             assert(deployedResourceNodes.length === 1)
 
             // Check placeholder propertries
@@ -375,10 +375,10 @@ describe('generateDeployedNode', () => {
                 unsupportTypeInput.resourceTreeEntity
             )
 
-            assert(loggerInfoStub.calledOnceWith('Details are missing or are incomplete for:'))
+            assert(loggerInfoStub.calledOnceWith('Details are missing or are incomplete for: %O'))
 
             // Check deployedResourceNodes array propertries
-            assert(loggerErrorStub.neverCalledWith('Error getting Lambda V3 configuration'))
+            assert(loggerErrorStub.neverCalledWith('Error getting Lambda V3 configuration %O'))
 
             // Check deployedResourceNodes array propertries
             assert(deployedResourceNodes.length === 1)

diff --git a/plugins/eslint-plugin-aws-toolkits/index.ts b/plugins/eslint-plugin-aws-toolkits/index.ts
@@ -10,6 +10,7 @@ import NoOnlyInTests from './lib/rules/no-only-in-tests'
 import NoStringExecForChildProcess from './lib/rules/no-string-exec-for-child-process'
 import NoConsoleLog from './lib/rules/no-console-log'
 import noJsonStringifyInLog from './lib/rules/no-json-stringify-in-log'
+import noPrintfMismatch from './lib/rules/no-printf-mismatch'
 
 const rules = {
     'no-await-on-vscode-msg': NoAwaitOnVscodeMsg,
@@ -19,6 +20,7 @@ const rules = {
     'no-string-exec-for-child-process': NoStringExecForChildProcess,
     'no-console-log': NoConsoleLog,
     'no-json-stringify-in-log': noJsonStringifyInLog,
+    'no-printf-mismatch': noPrintfMismatch,
 }
 
 export { rules }
diff --git a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-json-stringify-in-log.ts b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-json-stringify-in-log.ts
@@ -34,7 +34,7 @@ function isTemplateWithStringifyCall(node: TSESTree.CallExpressionArgument): boo
  * Check if node is representing syntax of the form getLogger().f(msg) for some f and msg or
  * if it is doing so indirectly via a logger variable.
  */
-function isLoggerCall(node: TSESTree.CallExpression): boolean {
+export function isLoggerCall(node: TSESTree.CallExpression): boolean {
     return (
         node.callee.type === AST_NODE_TYPES.MemberExpression &&
         (isGetLoggerCall(node.callee.object) || isDisguisedGetLoggerCall(node.callee.object)) &&

diff --git a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-printf-mismatch.ts b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-printf-mismatch.ts
@@ -0,0 +1,62 @@
+/*!
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import { AST_NODE_TYPES, ESLintUtils, TSESTree } from '@typescript-eslint/utils'
+import { isLoggerCall } from './no-json-stringify-in-log'
+import { Rule } from 'eslint'
+
+/**
+ * reuse solution done in logger itself: https://github.com/winstonjs/winston/blob/195e55c7e7fc58914ae4967ea7b832c9e0ced930/lib/winston/logger.js#L27
+ */
+function countSubTokens(literalNode: TSESTree.StringLiteral) {
+    const formatRegExp = /%[scdjifoO%]/g
+    return literalNode.value.match(formatRegExp)?.length || 0
+}
+/**
+ * Form the error message using templates or actual values.
+ * Allows us to avoid copy pasting message into test file.
+ */
+export function formErrorMsg(substitutionTokens: string | number, numArgs: string | number): string {
+    return `printf-style (console.log) call has ${substitutionTokens} format specifiers, but ${numArgs} arguments.`
+}
+
+export default ESLintUtils.RuleCreator.withoutDocs({
+    meta: {
+        docs: {
+            description: 'ensure string substitution args and templates match',
+            recommended: 'recommended',
+        },
+        messages: {
+            errMsg: formErrorMsg('{{ substitutionTokens }}', '{{ args }}'),
+        },
+        type: 'problem',
+        fixable: 'code',
+        schema: [],
+    },
+    defaultOptions: [],
+    create(context) {
+        return {
+            CallExpression(node: TSESTree.CallExpression) {
+                if (
+                    isLoggerCall(node) &&
+                    node.arguments[0].type === AST_NODE_TYPES.Literal &&
+                    typeof node.arguments[0].value === 'string'
+                ) {
+                    const numSubTokens = countSubTokens(node.arguments[0])
+                    const numExtraArgs = node.arguments.length - 1
+                    if (numSubTokens !== numExtraArgs) {
+                        return context.report({
+                            node: node,
+                            data: {
+                                substitutionTokens: numSubTokens,
+                                args: numExtraArgs,
+                            },
+                            messageId: 'errMsg',
+                        })
+                    }
+                }
+            },
+        }
+    },
+}) as unknown as Rule.RuleModule
-Original file line number
+Diff line change
@@ Expand Up @@
                     return
             }
         } catch (e) {
-            getLogger().error('Could not log telemetry for App Composer', e)
+            getLogger().error('Could not log telemetry for App Composer %O', e)
         }
     }
@@ Expand Down @@