From bb862e4b1185e85872b8fbf694bc07fa8b9dff6b Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 28 Oct 2024 16:50:57 -0400 Subject: [PATCH 01/19] implement custom lint rule --- .eslintrc.js | 2 +- plugins/eslint-plugin-aws-toolkits/index.ts | 2 + .../lib/rules/no-json-stringify-in-log.ts | 72 +++++++++++++++++++ .../rules/no-json-stringify-in-log.test.ts | 41 +++++++++++ 4 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 plugins/eslint-plugin-aws-toolkits/lib/rules/no-json-stringify-in-log.ts create mode 100644 plugins/eslint-plugin-aws-toolkits/test/rules/no-json-stringify-in-log.test.ts diff --git a/.eslintrc.js b/.eslintrc.js index c3c7cb602fc..a57d304c341 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -162,7 +162,7 @@ module.exports = { 'aws-toolkits/no-incorrect-once-usage': 'error', 'aws-toolkits/no-string-exec-for-child-process': 'error', 'aws-toolkits/no-console-log': 'error', - + 'aws-toolkits/no-json-stringify-in-log': 'error', 'no-restricted-imports': [ 'error', { diff --git a/plugins/eslint-plugin-aws-toolkits/index.ts b/plugins/eslint-plugin-aws-toolkits/index.ts index 10614583254..df8ab4d2026 100644 --- a/plugins/eslint-plugin-aws-toolkits/index.ts +++ b/plugins/eslint-plugin-aws-toolkits/index.ts @@ -9,6 +9,7 @@ import NoIncorrectOnceUsage from './lib/rules/no-incorrect-once-usage' 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' const rules = { 'no-await-on-vscode-msg': NoAwaitOnVscodeMsg, @@ -17,6 +18,7 @@ const rules = { 'no-only-in-tests': NoOnlyInTests, 'no-string-exec-for-child-process': NoStringExecForChildProcess, 'no-console-log': NoConsoleLog, + 'no-json-stringify-in-log': noJsonStringifyInLog, } 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 new file mode 100644 index 00000000000..5da57185ebb --- /dev/null +++ b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-json-stringify-in-log.ts @@ -0,0 +1,72 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +import { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/utils' +import { Rule } from 'eslint' + +export const errMsg = 'Avoid using JSON.stringify within logging and error messages, prefer %O.' + +function isJsonStringifyCall(node: any) { + return ( + node.type === AST_NODE_TYPES.CallExpression && + node.callee.type === AST_NODE_TYPES.MemberExpression && + node.callee.object.type === AST_NODE_TYPES.Identifier && + node.callee.object.name === 'JSON' && + node.callee.property.type === AST_NODE_TYPES.Identifier && + node.callee.property.name === 'stringify' + ) +} + +export default ESLintUtils.RuleCreator.withoutDocs({ + meta: { + docs: { + description: 'disallow use of JSON.stringify in logs and errors, to encourage use %O', + recommended: 'recommended', + }, + messages: { + errMsg, + }, + type: 'problem', + fixable: 'code', + schema: [], + }, + defaultOptions: [], + create(context) { + return { + CallExpression(node) { + // Look for a getLogger().f() call. + if ( + node.callee.type === AST_NODE_TYPES.MemberExpression && + node.callee.object.type === AST_NODE_TYPES.CallExpression && + node.callee.object.callee.type === AST_NODE_TYPES.Identifier && + node.callee.object.callee.name === 'getLogger' + ) { + // For each argument to the call above, check if it contains a JSON.stringify + node.arguments.forEach((arg) => { + // Check if arg itself if a JSON.stringify call + if (isJsonStringifyCall(arg)) { + return context.report({ + node: node, + messageId: 'errMsg', + }) + } + // Check if the arg contains a template ex. '${...}' + if (arg.type === AST_NODE_TYPES.TemplateLiteral) { + arg.expressions.forEach((e) => { + // Check the template for a JSON.stringify call. + if (isJsonStringifyCall(e)) { + return context.report({ + node: node, + messageId: 'errMsg', + }) + } + }) + } + }) + } + }, + } + }, +}) as unknown as Rule.RuleModule diff --git a/plugins/eslint-plugin-aws-toolkits/test/rules/no-json-stringify-in-log.test.ts b/plugins/eslint-plugin-aws-toolkits/test/rules/no-json-stringify-in-log.test.ts new file mode 100644 index 00000000000..683fff7a545 --- /dev/null +++ b/plugins/eslint-plugin-aws-toolkits/test/rules/no-json-stringify-in-log.test.ts @@ -0,0 +1,41 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +import { rules } from '../../index' +import { errMsg } from '../../lib/rules/no-json-stringify-in-log' +import { getRuleTester } from '../testUtil' + +getRuleTester().run('no-json-stringify-in-log', rules['no-json-stringify-in-log'], { + valid: [ + 'getLogger().debug(`the object is %O`)', + 'return JSON.stringify(d)', + 'getLogger().debug(`this does not include anything`)', + 'getLogger().debug(`another example ${JSON.notString(something)}`)', + 'getLogger().fakeFunction(`another example ${JSON.notString(something)}`)', + ], + + invalid: [ + { + code: 'getLogger().debug(`the object is ${JSON.stringify(obj)}`)', + errors: [errMsg], + }, + { + code: 'getLogger().debug(`the object is ${notThis} but ${JSON.stringify(obj)}`)', + errors: [errMsg], + }, + { + code: 'getLogger().debug(`the object is ${notThis} or ${thisOne} but ${JSON.stringify(obj)}`)', + errors: [errMsg], + }, + { + code: 'getLogger().verbose(`Invalid Request : `, JSON.stringify(request, undefined, EditorContext.getTabSize()))', + errors: [errMsg], + }, + { + code: 'getLogger().verbose(`provideDebugConfigurations: debugconfigs: ${JSON.stringify(configs)}`)', + errors: [errMsg], + }, + ], +}) From 67889299caa933a2adfcec703553e4ae43a3fbbb Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 28 Oct 2024 17:20:41 -0400 Subject: [PATCH 02/19] add more test cases --- .../lib/rules/no-json-stringify-in-log.ts | 48 +++++++++++++++---- .../rules/no-json-stringify-in-log.test.ts | 14 ++++++ 2 files changed, 54 insertions(+), 8 deletions(-) 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 index 5da57185ebb..18f48a7a2f5 100644 --- 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 @@ -8,7 +8,11 @@ import { Rule } from 'eslint' export const errMsg = 'Avoid using JSON.stringify within logging and error messages, prefer %O.' -function isJsonStringifyCall(node: any) { +/** + * Check if a given expression is a JSON.stringify call. + */ + +function isJsonStringifyCall(node: any): boolean { return ( node.type === AST_NODE_TYPES.CallExpression && node.callee.type === AST_NODE_TYPES.MemberExpression && @@ -19,6 +23,39 @@ function isJsonStringifyCall(node: any) { ) } +/** + * Check if node is representing syntax of the form getLogger().f(msg) for some f and msg. + * + */ +function isLoggerCall(node: any): boolean { + return ( + node.callee.type === AST_NODE_TYPES.MemberExpression && + node.callee.object.type === AST_NODE_TYPES.CallExpression && + node.callee.object.callee.type === AST_NODE_TYPES.Identifier && + node.callee.object.callee.name === 'getLogger' + ) +} + +/** + * Use two simple heuristics to detect `disguised` logger calls. This is when we log without getLogger. + * Ex. + * const logger = getLogger() + * logger.debug(m) + * To catch these we try two checks: + * 1) If the left side is an identifier including the word logger + * 2) If the left side is a property of some object, including the word logger. + */ +function isDisguisedLoggerCall(node: any): boolean { + return ( + (node.callee.type === AST_NODE_TYPES.MemberExpression && + node.callee.object.type === AST_NODE_TYPES.Identifier && + node.callee.object.name.toLowerCase().includes('logger')) || + (node.callee.type === AST_NODE_TYPES.MemberExpression && + node.callee.object.type === AST_NODE_TYPES.MemberExpression && + node.callee.object.property.name.toLowerCase().includes('logger')) + ) +} + export default ESLintUtils.RuleCreator.withoutDocs({ meta: { docs: { @@ -36,13 +73,8 @@ export default ESLintUtils.RuleCreator.withoutDocs({ create(context) { return { CallExpression(node) { - // Look for a getLogger().f() call. - if ( - node.callee.type === AST_NODE_TYPES.MemberExpression && - node.callee.object.type === AST_NODE_TYPES.CallExpression && - node.callee.object.callee.type === AST_NODE_TYPES.Identifier && - node.callee.object.callee.name === 'getLogger' - ) { + // Look for a getLogger().f() call or a disguised one, see isDisguisedLoggerCall for more info. + if (isLoggerCall(node) || isDisguisedLoggerCall(node)) { // For each argument to the call above, check if it contains a JSON.stringify node.arguments.forEach((arg) => { // Check if arg itself if a JSON.stringify call diff --git a/plugins/eslint-plugin-aws-toolkits/test/rules/no-json-stringify-in-log.test.ts b/plugins/eslint-plugin-aws-toolkits/test/rules/no-json-stringify-in-log.test.ts index 683fff7a545..a3fdaf6c976 100644 --- a/plugins/eslint-plugin-aws-toolkits/test/rules/no-json-stringify-in-log.test.ts +++ b/plugins/eslint-plugin-aws-toolkits/test/rules/no-json-stringify-in-log.test.ts @@ -14,6 +14,8 @@ getRuleTester().run('no-json-stringify-in-log', rules['no-json-stringify-in-log' 'getLogger().debug(`this does not include anything`)', 'getLogger().debug(`another example ${JSON.notString(something)}`)', 'getLogger().fakeFunction(`another example ${JSON.notString(something)}`)', + 'this.deps.devLogger?.debug("crashMonitoring: CLEAR_STATE: Succeeded")', + 'getLogger().debug(`called startBuilderIdSetup()`)', ], invalid: [ @@ -37,5 +39,17 @@ getRuleTester().run('no-json-stringify-in-log', rules['no-json-stringify-in-log' code: 'getLogger().verbose(`provideDebugConfigurations: debugconfigs: ${JSON.stringify(configs)}`)', errors: [errMsg], }, + { + code: 'devLogger?.debug(`crashMonitoring: CHECKED: Result of cleaning up crashed instances\nBEFORE: ${JSON.stringify(before)}\nAFTER: ${JSON.stringify(after)}\nACTUAL: ${JSON.stringify(afterActual)}`)', + errors: [errMsg, errMsg, errMsg], + }, + { + code: 'getLogger().warn(`skipping invalid item in telemetry cache: ${JSON.stringify(item)}\n`)', + errors: [errMsg], + }, + { + code: 'this.deps.devLogger?.debug(`crashMonitoring: CLEAR_STATE: Succeeded ${JSON.stringify(item)}`)', + errors: [errMsg], + }, ], }) From 6b5218916bb701512c33e5b0d73b3d16c5ec4ee8 Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 28 Oct 2024 17:44:42 -0400 Subject: [PATCH 03/19] add typing, refactor --- .../lib/rules/no-json-stringify-in-log.ts | 58 ++++++++----------- .../rules/no-json-stringify-in-log.test.ts | 2 +- 2 files changed, 26 insertions(+), 34 deletions(-) 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 index 18f48a7a2f5..85383cf2cf2 100644 --- 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 @@ -3,7 +3,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/utils' +import { AST_NODE_TYPES, ESLintUtils, TSESTree } from '@typescript-eslint/utils' import { Rule } from 'eslint' export const errMsg = 'Avoid using JSON.stringify within logging and error messages, prefer %O.' @@ -11,8 +11,7 @@ export const errMsg = 'Avoid using JSON.stringify within logging and error messa /** * Check if a given expression is a JSON.stringify call. */ - -function isJsonStringifyCall(node: any): boolean { +function isJsonStringifyCall(node: TSESTree.CallExpressionArgument): boolean { return ( node.type === AST_NODE_TYPES.CallExpression && node.callee.type === AST_NODE_TYPES.MemberExpression && @@ -23,16 +22,24 @@ function isJsonStringifyCall(node: any): boolean { ) } +function isTemplateWithStringifyCall(node: TSESTree.CallExpressionArgument): boolean { + return ( + node.type === AST_NODE_TYPES.TemplateLiteral && + node.expressions.some((e: TSESTree.Expression) => isJsonStringifyCall(e)) + ) +} + /** * Check if node is representing syntax of the form getLogger().f(msg) for some f and msg. * */ -function isLoggerCall(node: any): boolean { +function isLoggerCall(node: TSESTree.CallExpression): boolean { return ( - node.callee.type === AST_NODE_TYPES.MemberExpression && - node.callee.object.type === AST_NODE_TYPES.CallExpression && - node.callee.object.callee.type === AST_NODE_TYPES.Identifier && - node.callee.object.callee.name === 'getLogger' + (node.callee.type === AST_NODE_TYPES.MemberExpression && + node.callee.object.type === AST_NODE_TYPES.CallExpression && + node.callee.object.callee.type === AST_NODE_TYPES.Identifier && + node.callee.object.callee.name === 'getLogger') || + isDisguisedLoggerCall(node) ) } @@ -45,13 +52,14 @@ function isLoggerCall(node: any): boolean { * 1) If the left side is an identifier including the word logger * 2) If the left side is a property of some object, including the word logger. */ -function isDisguisedLoggerCall(node: any): boolean { +function isDisguisedLoggerCall(node: TSESTree.CallExpression): boolean { return ( (node.callee.type === AST_NODE_TYPES.MemberExpression && node.callee.object.type === AST_NODE_TYPES.Identifier && node.callee.object.name.toLowerCase().includes('logger')) || (node.callee.type === AST_NODE_TYPES.MemberExpression && node.callee.object.type === AST_NODE_TYPES.MemberExpression && + node.callee.object.property.type === AST_NODE_TYPES.Identifier && node.callee.object.property.name.toLowerCase().includes('logger')) ) } @@ -72,30 +80,14 @@ export default ESLintUtils.RuleCreator.withoutDocs({ defaultOptions: [], create(context) { return { - CallExpression(node) { - // Look for a getLogger().f() call or a disguised one, see isDisguisedLoggerCall for more info. - if (isLoggerCall(node) || isDisguisedLoggerCall(node)) { - // For each argument to the call above, check if it contains a JSON.stringify - node.arguments.forEach((arg) => { - // Check if arg itself if a JSON.stringify call - if (isJsonStringifyCall(arg)) { - return context.report({ - node: node, - messageId: 'errMsg', - }) - } - // Check if the arg contains a template ex. '${...}' - if (arg.type === AST_NODE_TYPES.TemplateLiteral) { - arg.expressions.forEach((e) => { - // Check the template for a JSON.stringify call. - if (isJsonStringifyCall(e)) { - return context.report({ - node: node, - messageId: 'errMsg', - }) - } - }) - } + CallExpression(node: TSESTree.CallExpression) { + if ( + isLoggerCall(node) && + node.arguments.some((arg) => isJsonStringifyCall(arg) || isTemplateWithStringifyCall(arg)) + ) { + return context.report({ + node: node, + messageId: 'errMsg', }) } }, diff --git a/plugins/eslint-plugin-aws-toolkits/test/rules/no-json-stringify-in-log.test.ts b/plugins/eslint-plugin-aws-toolkits/test/rules/no-json-stringify-in-log.test.ts index a3fdaf6c976..eb29ebb6cd2 100644 --- a/plugins/eslint-plugin-aws-toolkits/test/rules/no-json-stringify-in-log.test.ts +++ b/plugins/eslint-plugin-aws-toolkits/test/rules/no-json-stringify-in-log.test.ts @@ -41,7 +41,7 @@ getRuleTester().run('no-json-stringify-in-log', rules['no-json-stringify-in-log' }, { code: 'devLogger?.debug(`crashMonitoring: CHECKED: Result of cleaning up crashed instances\nBEFORE: ${JSON.stringify(before)}\nAFTER: ${JSON.stringify(after)}\nACTUAL: ${JSON.stringify(afterActual)}`)', - errors: [errMsg, errMsg, errMsg], + errors: [errMsg], }, { code: 'getLogger().warn(`skipping invalid item in telemetry cache: ${JSON.stringify(item)}\n`)', From 5c7724a15232d0041b814a4574a2bf23f221cc0f Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 28 Oct 2024 17:49:13 -0400 Subject: [PATCH 04/19] adjust comments --- .../lib/rules/no-json-stringify-in-log.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 index 85383cf2cf2..80c39bfe29e 100644 --- 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 @@ -30,8 +30,8 @@ function isTemplateWithStringifyCall(node: TSESTree.CallExpressionArgument): boo } /** - * Check if node is representing syntax of the form getLogger().f(msg) for some f and msg. - * + * 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 { return ( @@ -44,7 +44,7 @@ function isLoggerCall(node: TSESTree.CallExpression): boolean { } /** - * Use two simple heuristics to detect `disguised` logger calls. This is when we log without getLogger. + * Use two simple heuristics to detect `disguised` logger calls. This is when we log without getLogger in the same statement. * Ex. * const logger = getLogger() * logger.debug(m) From 6ff529b32c24495a5a03ba11775165dc6ce7a4de Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 29 Oct 2024 11:35:10 -0400 Subject: [PATCH 05/19] change existing uses --- .../messageHandlers/generateResourceHandler.ts | 17 ++++++++++------- .../service/recommendationHandler.ts | 5 +---- .../controllers/chat/controller.ts | 9 +++------ packages/core/src/shared/crashMonitoring.ts | 7 +++++-- packages/core/src/shared/featureConfig.ts | 2 +- .../src/shared/sam/debugger/awsSamDebugger.ts | 4 ++-- .../src/shared/sam/debugger/pythonSamDebug.ts | 2 +- .../core/src/shared/sam/localLambdaRunner.ts | 15 +++++---------- .../src/shared/telemetry/telemetryService.ts | 2 +- packages/core/src/shared/utilities/map.ts | 2 +- .../src/ssmDocument/commands/publishDocument.ts | 4 ++-- .../core/src/test/codewhisperer/testUtil.ts | 2 +- 12 files changed, 33 insertions(+), 38 deletions(-) diff --git a/packages/core/src/applicationcomposer/messageHandlers/generateResourceHandler.ts b/packages/core/src/applicationcomposer/messageHandlers/generateResourceHandler.ts index 1dacff2a93d..6d3e96b81c3 100644 --- a/packages/core/src/applicationcomposer/messageHandlers/generateResourceHandler.ts +++ b/packages/core/src/applicationcomposer/messageHandlers/generateResourceHandler.ts @@ -86,13 +86,13 @@ async function generateResource(cfnType: string) { // TODO-STARLING - Revisit to see if timeout still needed prior to launch const data = await timeout(amazonqApi.chatApi.chat(request), TIMEOUT) const initialResponseTime = globals.clock.Date.now() - startTime - getLogger().debug(`CW Chat initial response: ${JSON.stringify(data, undefined, 2)}, ${initialResponseTime} ms`) + getLogger().debug(`CW Chat initial response: %O, ${initialResponseTime} ms`, data) if (data['$metadata']) { metadata = data['$metadata'] } if (data.generateAssistantResponseResponse === undefined) { - getLogger().debug(`Error: Unexpected model response: ${JSON.stringify(data, undefined, 2)}`) + getLogger().debug(`Error: Unexpected model response: %O`, data) throw new Error('No model response') } @@ -141,12 +141,15 @@ async function generateResource(cfnType: string) { `CW Chat Debug message: cfnType = "${cfnType}", conversationId = ${conversationId}, - metadata = \n${JSON.stringify(metadata, undefined, 2)}, - supplementaryWebLinks = \n${JSON.stringify(supplementaryWebLinks, undefined, 2)}, - references = \n${JSON.stringify(references, undefined, 2)}, + metadata = %O, + supplementaryWebLinks = %O, + references = %O, response = "${response}", initialResponse = ${initialResponseTime} ms, - elapsed time = ${elapsedTime} ms` + elapsed time = ${elapsedTime} ms`, + metadata, + supplementaryWebLinks, + references ) return { @@ -163,7 +166,7 @@ async function generateResource(cfnType: string) { getLogger().debug(`CW Chat error: ${error.name} - ${error.message}`) if (error.$metadata) { const { requestId, cfId, extendedRequestId } = error.$metadata - getLogger().debug(JSON.stringify({ requestId, cfId, extendedRequestId }, undefined, 2)) + getLogger().debug('%O', { requestId, cfId, extendedRequestId }) } throw error diff --git a/packages/core/src/codewhisperer/service/recommendationHandler.ts b/packages/core/src/codewhisperer/service/recommendationHandler.ts index 1fd46541c11..7ce0d223a63 100644 --- a/packages/core/src/codewhisperer/service/recommendationHandler.ts +++ b/packages/core/src/codewhisperer/service/recommendationHandler.ts @@ -220,10 +220,7 @@ export class RecommendationHandler { * Validate request */ if (!EditorContext.validateRequest(request)) { - getLogger().verbose( - 'Invalid Request : ', - JSON.stringify(request, undefined, EditorContext.getTabSize()) - ) + getLogger().verbose('Invalid Request: %O', request) const languageName = request.fileContext.programmingLanguage.languageName if (!runtimeLanguageContext.isLanguageSupported(languageName)) { errorMessage = `${languageName} is currently not supported by Amazon Q inline suggestions` diff --git a/packages/core/src/codewhispererChat/controllers/chat/controller.ts b/packages/core/src/codewhispererChat/controllers/chat/controller.ts index 98d09ca3c7c..0683cc719d3 100644 --- a/packages/core/src/codewhispererChat/controllers/chat/controller.ts +++ b/packages/core/src/codewhispererChat/controllers/chat/controller.ts @@ -651,11 +651,7 @@ export class ChatController { const request = triggerPayloadToChatRequest(triggerPayload) const session = this.sessionStorage.getSession(tabID) - getLogger().info( - `request from tab: ${tabID} conversationID: ${session.sessionIdentifier} request: ${JSON.stringify( - request - )}` - ) + getLogger().info(`request from tab: ${tabID} conversationID: ${session.sessionIdentifier} request: %O`, request) let response: GenerateAssistantResponseCommandOutput | undefined = undefined session.createNewTokenSource() try { @@ -668,7 +664,8 @@ export class ChatController { getLogger().info( `response to tab: ${tabID} conversationID: ${session.sessionIdentifier} requestID: ${ response.$metadata.requestId - } metadata: ${JSON.stringify(response.$metadata)}` + } metadata: %O`, + response.$metadata ) await this.messenger.sendAIResponse(response, session, tabID, triggerID, triggerPayload) } catch (e: any) { diff --git a/packages/core/src/shared/crashMonitoring.ts b/packages/core/src/shared/crashMonitoring.ts index bdbc66af230..6bc2dbb2b8f 100644 --- a/packages/core/src/shared/crashMonitoring.ts +++ b/packages/core/src/shared/crashMonitoring.ts @@ -297,7 +297,7 @@ class CrashChecker { // Example is if I hit the red square in the debug menu, it is a non-graceful shutdown. But the regular // 'x' button in the Debug IDE instance is a graceful shutdown. if (ext.isDebug) { - devLogger?.debug(`crashMonitoring: DEBUG instance crashed: ${JSON.stringify(ext)}`) + devLogger?.debug(`crashMonitoring: DEBUG instance crashed: %O`, ext) return } @@ -318,7 +318,10 @@ class CrashChecker { // Sanity check: ENSURE THAT AFTER === ACTUAL or this implies that our data is out of sync const afterActual = (await state.getAllExts()).map((i) => truncateUuid(i.sessionId)) devLogger?.debug( - `crashMonitoring: CHECKED: Result of cleaning up crashed instances\nBEFORE: ${JSON.stringify(before)}\nAFTER: ${JSON.stringify(after)}\nACTUAL: ${JSON.stringify(afterActual)}` + `crashMonitoring: CHECKED: Result of cleaning up crashed instances\nBEFORE: %O \nAFTER: %O \nACTUAL: %O`, + before, + after, + afterActual ) } diff --git a/packages/core/src/shared/featureConfig.ts b/packages/core/src/shared/featureConfig.ts index a8f35360355..a94fc25a7b8 100644 --- a/packages/core/src/shared/featureConfig.ts +++ b/packages/core/src/shared/featureConfig.ts @@ -110,7 +110,7 @@ export class FeatureConfigProvider { }) }) }) - getLogger().info('AB Testing Cohort Assignments %s', JSON.stringify(response.featureEvaluations)) + getLogger().info('AB Testing Cohort Assignments %O', response.featureEvaluations) const customizationArnOverride = this.featureConfigs.get(Features.customizationArnOverride)?.value ?.stringValue diff --git a/packages/core/src/shared/sam/debugger/awsSamDebugger.ts b/packages/core/src/shared/sam/debugger/awsSamDebugger.ts index 49b3b186b0f..baf1af7bb19 100644 --- a/packages/core/src/shared/sam/debugger/awsSamDebugger.ts +++ b/packages/core/src/shared/sam/debugger/awsSamDebugger.ts @@ -308,7 +308,7 @@ export class SamDebugConfigProvider implements vscode.DebugConfigurationProvider } } } - getLogger().verbose(`provideDebugConfigurations: debugconfigs: ${JSON.stringify(configs)}`) + getLogger().verbose(`provideDebugConfigurations: debugconfigs: %O`, configs) } return configs @@ -445,7 +445,7 @@ export class SamDebugConfigProvider implements vscode.DebugConfigurationProvider } else if (rv.message) { void vscode.window.showInformationMessage(rv.message) } - getLogger().verbose(`SAM debug: config: ${JSON.stringify(config.name)}`) + getLogger().verbose(`SAM debug: config %s:`, config.name) } const editor = vscode.window.activeTextEditor diff --git a/packages/core/src/shared/sam/debugger/pythonSamDebug.ts b/packages/core/src/shared/sam/debugger/pythonSamDebug.ts index 5b0efb8ceb1..a5ef2bc5864 100644 --- a/packages/core/src/shared/sam/debugger/pythonSamDebug.ts +++ b/packages/core/src/shared/sam/debugger/pythonSamDebug.ts @@ -49,7 +49,7 @@ async function makePythonDebugManifest(params: { if (await fileExists(manfestPath)) { manifestText = await readFileAsString(manfestPath) } - getLogger().debug(`pythonCodeLensProvider.makePythonDebugManifest params: ${JSON.stringify(params, undefined, 2)}`) + getLogger().debug(`pythonCodeLensProvider.makePythonDebugManifest params: %O`, params) // TODO: If another module name includes the string "ikp3db", this will be skipped... // HACK: Cloud9-created Lambdas hardcode ikp3db 1.1.4, which only functions with Python 3.6 (which we don't support) // Remove any ikp3db dependency if it exists and manually add a non-pinned ikp3db dependency. diff --git a/packages/core/src/shared/sam/localLambdaRunner.ts b/packages/core/src/shared/sam/localLambdaRunner.ts index 8705b58debc..b533aa3c7a9 100644 --- a/packages/core/src/shared/sam/localLambdaRunner.ts +++ b/packages/core/src/shared/sam/localLambdaRunner.ts @@ -452,16 +452,11 @@ export async function attachDebugger({ }, ...params }: AttachDebuggerContext): Promise { - getLogger().debug( - `localLambdaRunner.attachDebugger: startDebugging with config: ${JSON.stringify( - { - name: params.debugConfig.name, - invokeTarget: params.debugConfig.invokeTarget, - }, - undefined, - 2 - )}` - ) + const obj = { + name: params.debugConfig.name, + invokeTarget: params.debugConfig.invokeTarget, + } + getLogger().debug(`localLambdaRunner.attachDebugger: startDebugging with config: %O`, obj) getLogger().info(localize('AWS.output.sam.local.attaching', 'Attaching debugger to SAM application...')) diff --git a/packages/core/src/shared/telemetry/telemetryService.ts b/packages/core/src/shared/telemetry/telemetryService.ts index d0d018ab88a..c56ba3d2a24 100644 --- a/packages/core/src/shared/telemetry/telemetryService.ts +++ b/packages/core/src/shared/telemetry/telemetryService.ts @@ -470,7 +470,7 @@ export function filterTelemetryCacheEvents(input: any): MetricDatum[] { !Object.prototype.hasOwnProperty.call(item, 'EpochTimestamp') || !Object.prototype.hasOwnProperty.call(item, 'Unit') ) { - getLogger().warn(`skipping invalid item in telemetry cache: ${JSON.stringify(item)}\n`) + getLogger().warn(`skipping invalid item in telemetry cache: %O\n`, item) return false } diff --git a/packages/core/src/shared/utilities/map.ts b/packages/core/src/shared/utilities/map.ts index 57d5ed7b1cb..1a681235cd4 100644 --- a/packages/core/src/shared/utilities/map.ts +++ b/packages/core/src/shared/utilities/map.ts @@ -43,7 +43,7 @@ export abstract class NestedMap implements delete(key: Key, reason?: string): void { delete this.data[this.hash(key)] if (reason) { - getLogger().debug(`${this.name}: cleared cache, key: ${JSON.stringify(key)}, reason: ${reason}`) + getLogger().debug(`${this.name}: cleared cache, key: %O, reason: ${reason}`, key) } } diff --git a/packages/core/src/ssmDocument/commands/publishDocument.ts b/packages/core/src/ssmDocument/commands/publishDocument.ts index 4c0905f358f..bd1e400cc0f 100644 --- a/packages/core/src/ssmDocument/commands/publishDocument.ts +++ b/packages/core/src/ssmDocument/commands/publishDocument.ts @@ -83,7 +83,7 @@ export async function createDocument( } const createResult = await client.createDocument(request) - logger.info(`Created Systems Manager Document: ${JSON.stringify(createResult.DocumentDescription)}`) + logger.info(`Created Systems Manager Document: %O`, createResult.DocumentDescription) void vscode.window.showInformationMessage(`Created Systems Manager Document: ${wizardResponse.name}`) } catch (err) { const error = err as Error @@ -118,7 +118,7 @@ export async function updateDocument( const updateResult = await client.updateDocument(request) - logger.info(`Updated Systems Manager Document: ${JSON.stringify(updateResult.DocumentDescription)}`) + logger.info(`Updated Systems Manager Document: %O`, updateResult.DocumentDescription) void vscode.window.showInformationMessage(`Updated Systems Manager Document: ${wizardResponse.name}`) const isConfirmed = await showConfirmationMessage({ diff --git a/packages/core/src/test/codewhisperer/testUtil.ts b/packages/core/src/test/codewhisperer/testUtil.ts index e752c7c93b8..4abac73dc39 100644 --- a/packages/core/src/test/codewhisperer/testUtil.ts +++ b/packages/core/src/test/codewhisperer/testUtil.ts @@ -76,7 +76,7 @@ export function createMockTextEditor( insert: sinon.spy(), setEndOfLine: sinon.spy(), delete: function (_location: vscode.Selection | vscode.Range): void { - getLogger().info(`delete ${JSON.stringify(_location)}`) + getLogger().info(`delete %O`, _location) }, } resolve(editor) From 1430a975b833602fcfb81c2a28caf88c7418aa84 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 29 Oct 2024 12:15:54 -0400 Subject: [PATCH 06/19] migrate case in new file --- .../src/inlineChat/provider/inlineChatProvider.ts | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/amazonq/src/inlineChat/provider/inlineChatProvider.ts b/packages/amazonq/src/inlineChat/provider/inlineChatProvider.ts index 99c1e21c321..aad41b3cd1b 100644 --- a/packages/amazonq/src/inlineChat/provider/inlineChatProvider.ts +++ b/packages/amazonq/src/inlineChat/provider/inlineChatProvider.ts @@ -111,20 +111,15 @@ export class InlineChatProvider { const request = triggerPayloadToChatRequest(triggerPayload) const session = this.sessionStorage.getSession(tabID) - getLogger().info( - `request from tab: ${tabID} conversationID: ${session.sessionIdentifier} request: ${JSON.stringify( - request - )}` - ) + getLogger().info(`request from tab: ${tabID} conversationID: ${session.sessionIdentifier} request: %O`, request) let response: GenerateAssistantResponseCommandOutput | undefined = undefined session.createNewTokenSource() try { response = await session.chatSso(request) getLogger().info( - `response to tab: ${tabID} conversationID: ${session.sessionIdentifier} requestID: ${response.$metadata.requestId} metadata: ${JSON.stringify( - response.$metadata - )}` + `response to tab: ${tabID} conversationID: ${session.sessionIdentifier} requestID: ${response.$metadata.requestId} metadata: %O`, + response.$metadata ) } catch (e: any) { this.processException(e, tabID) From 8afa3d8bd8965076c9516b2d6586b9aea74bf8f0 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 29 Oct 2024 14:11:33 -0400 Subject: [PATCH 07/19] start of work --- .../lib/rules/no-json-stringify-in-log.ts | 2 +- .../lib/rules/no-string-sub-mismatch.ts | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-sub-mismatch.ts 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 index 80c39bfe29e..ce101330e22 100644 --- 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 @@ -33,7 +33,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 && node.callee.object.type === AST_NODE_TYPES.CallExpression && diff --git a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-sub-mismatch.ts b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-sub-mismatch.ts new file mode 100644 index 00000000000..43e6a55a51e --- /dev/null +++ b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-sub-mismatch.ts @@ -0,0 +1,22 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ +import { ESLintUtils, TSESTree } from '@typescript-eslint/utils' + +export default ESLintUtils.RuleCreator.withoutDocs({ + meta: { + docs: { + description: 'ensure string substitution args and templates match', + recommended: 'recommended', + }, + messages: {}, + type: 'problem', + fixable: 'code', + schema: [], + }, + defaultOptions: [], + create(context) { + return {} + }, +}) From 0736cf41ddc1e3d6d7f0ec643014843204536134 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 29 Oct 2024 14:12:29 -0400 Subject: [PATCH 08/19] add note about depth limit --- .../lib/rules/no-json-stringify-in-log.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 index 80c39bfe29e..b5cd2efbc8d 100644 --- 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 @@ -6,7 +6,8 @@ import { AST_NODE_TYPES, ESLintUtils, TSESTree } from '@typescript-eslint/utils' import { Rule } from 'eslint' -export const errMsg = 'Avoid using JSON.stringify within logging and error messages, prefer %O.' +export const errMsg = + 'Avoid using JSON.stringify within logging and error messages, prefer %O. Note: %O has a depth limit of 2' /** * Check if a given expression is a JSON.stringify call. From aaad8adf2ce95df10673569481bfa749110af9e9 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 29 Oct 2024 16:01:36 -0400 Subject: [PATCH 09/19] implement rule --- plugins/eslint-plugin-aws-toolkits/index.ts | 2 + .../lib/rules/no-string-sub-mismatch.ts | 48 +++++++++++++++++-- .../test/rules/no-string-sub-mismatch.test.ts | 32 +++++++++++++ 3 files changed, 78 insertions(+), 4 deletions(-) create mode 100644 plugins/eslint-plugin-aws-toolkits/test/rules/no-string-sub-mismatch.test.ts diff --git a/plugins/eslint-plugin-aws-toolkits/index.ts b/plugins/eslint-plugin-aws-toolkits/index.ts index df8ab4d2026..32b6bb30320 100644 --- 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 noStringSubMismatch from './lib/rules/no-string-sub-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-string-sub-mismatch': noStringSubMismatch, } export { rules } diff --git a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-sub-mismatch.ts b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-sub-mismatch.ts index 43e6a55a51e..6f1e42cf3bf 100644 --- a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-sub-mismatch.ts +++ b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-sub-mismatch.ts @@ -2,7 +2,24 @@ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. * SPDX-License-Identifier: Apache-2.0 */ -import { ESLintUtils, TSESTree } from '@typescript-eslint/utils' +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 `String substitution has ${substitutionTokens} substitution tokens, but ${numArgs} arguments.` +} export default ESLintUtils.RuleCreator.withoutDocs({ meta: { @@ -10,13 +27,36 @@ export default ESLintUtils.RuleCreator.withoutDocs({ description: 'ensure string substitution args and templates match', recommended: 'recommended', }, - messages: {}, + messages: { + errMsg: formErrorMsg('{{ substitutionTokens }}', '{{ args }}'), + }, type: 'problem', fixable: 'code', schema: [], }, defaultOptions: [], create(context) { - return {} + 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 diff --git a/plugins/eslint-plugin-aws-toolkits/test/rules/no-string-sub-mismatch.test.ts b/plugins/eslint-plugin-aws-toolkits/test/rules/no-string-sub-mismatch.test.ts new file mode 100644 index 00000000000..3149950fdd4 --- /dev/null +++ b/plugins/eslint-plugin-aws-toolkits/test/rules/no-string-sub-mismatch.test.ts @@ -0,0 +1,32 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +import { rules } from '../../index' +import { formErrorMsg } from '../../lib/rules/no-string-sub-mismatch' +import { getRuleTester } from '../testUtil' + +getRuleTester().run('no-string-sub-mismatch', rules['no-string-sub-mismatch'], { + valid: [ + 'getLogger().debug("this is a string %s and a number %d", "s", 2)', + 'getLogger().debug("this is a number %d", 2)', + 'getLogger().debug("this has no substitutions")', + 'getLogger().debug("1 %s 2 %d 3 %O 4 %o 5 %s", arg1, arg2, arg3, arg4, arg5)', + 'getLogger().debug("not real a sub-token %z")', + ], + invalid: [ + { + code: 'getLogger().debug("this is a string %s and a number %d", "s")', + errors: [formErrorMsg(2, 1)], + }, + { + code: 'getLogger().debug("this is a string %s a string %s a string %s")', + errors: [formErrorMsg(3, 0)], + }, + { + code: 'getLogger().debug("this is a string", err)', + errors: [formErrorMsg(0, 1)], + }, + ], +}) From 13b1a09233c138d73fbdc244d3b9cf609b66ed83 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 29 Oct 2024 16:15:34 -0400 Subject: [PATCH 10/19] activate rule --- .eslintrc.js | 1 + .../messageHandlers/emitTelemetryMessageHandler.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.eslintrc.js b/.eslintrc.js index a57d304c341..0063369daae 100644 --- 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-string-sub-mismatch': 'error', 'no-restricted-imports': [ 'error', { diff --git a/packages/core/src/applicationcomposer/messageHandlers/emitTelemetryMessageHandler.ts b/packages/core/src/applicationcomposer/messageHandlers/emitTelemetryMessageHandler.ts index 9ca92ea445b..9b0506b9196 100644 --- 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) } } From a391f69169f5b26971e1b776d2ee5c92df0c1e9c Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 29 Oct 2024 16:57:01 -0400 Subject: [PATCH 11/19] start migration --- packages/core/src/awsService/iot/commands/createCert.ts | 6 +++--- .../core/src/awsService/redshift/explorer/redshiftNode.ts | 2 +- packages/core/src/awsService/s3/commands/uploadFile.ts | 2 +- packages/core/src/codewhisperer/activation.ts | 2 +- packages/core/src/codewhisperer/client/agent.ts | 2 +- .../core/src/codewhisperer/commands/startSecurityScan.ts | 2 +- packages/core/src/shared/sam/activation.ts | 2 +- .../lib/rules/no-string-sub-mismatch.ts | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/core/src/awsService/iot/commands/createCert.ts b/packages/core/src/awsService/iot/commands/createCert.ts index c3b65cf886d..22db3136831 100644 --- 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 index 4d923192f22..52eb470283b 100644 --- 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 index 4bfcc91e027..ca77617af4e 100644 --- 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 index ff78d8a99ef..a9a74a67e5f 100644 --- 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 index d825e94d8d1..52152d2a616 100644 --- 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 index 58eaee2bf40..5bd0b1552cf 100644 --- 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/shared/sam/activation.ts b/packages/core/src/shared/sam/activation.ts index 22e3016bedf..e525c1db887 100644 --- 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/plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-sub-mismatch.ts b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-sub-mismatch.ts index 6f1e42cf3bf..ad9ef74cefb 100644 --- a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-sub-mismatch.ts +++ b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-sub-mismatch.ts @@ -18,7 +18,7 @@ function countSubTokens(literalNode: TSESTree.StringLiteral) { * Allows us to avoid copy pasting message into test file. */ export function formErrorMsg(substitutionTokens: string | number, numArgs: string | number): string { - return `String substitution has ${substitutionTokens} substitution tokens, but ${numArgs} arguments.` + return `String substitution has ${substitutionTokens} format specifiers, but ${numArgs} arguments.` } export default ESLintUtils.RuleCreator.withoutDocs({ From 07f3682efe8f5af466e4724d96e1de73f0d8217f Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 29 Oct 2024 17:42:27 -0400 Subject: [PATCH 12/19] refactor and enforce one of the log functions --- .../lib/rules/no-json-stringify-in-log.ts | 30 +++++++++++-------- .../rules/no-json-stringify-in-log.test.ts | 1 + 2 files changed, 18 insertions(+), 13 deletions(-) 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 index b5cd2efbc8d..dd6696f00c5 100644 --- 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 @@ -36,11 +36,18 @@ function isTemplateWithStringifyCall(node: TSESTree.CallExpressionArgument): boo */ function isLoggerCall(node: TSESTree.CallExpression): boolean { return ( - (node.callee.type === AST_NODE_TYPES.MemberExpression && - node.callee.object.type === AST_NODE_TYPES.CallExpression && - node.callee.object.callee.type === AST_NODE_TYPES.Identifier && - node.callee.object.callee.name === 'getLogger') || - isDisguisedLoggerCall(node) + node.callee.type === AST_NODE_TYPES.MemberExpression && + (isGetLoggerCall(node.callee.object) || isDisguisedGetLoggerCall(node.callee.object)) && + node.callee.property.type === AST_NODE_TYPES.Identifier && + ['debug', 'verbose', 'info', 'warn', 'error'].includes(node.callee.property.name) + ) +} + +function isGetLoggerCall(node: TSESTree.Expression): boolean { + return ( + node.type === AST_NODE_TYPES.CallExpression && + node.callee.type === AST_NODE_TYPES.Identifier && + node.callee.name === 'getLogger' ) } @@ -53,15 +60,12 @@ function isLoggerCall(node: TSESTree.CallExpression): boolean { * 1) If the left side is an identifier including the word logger * 2) If the left side is a property of some object, including the word logger. */ -function isDisguisedLoggerCall(node: TSESTree.CallExpression): boolean { +function isDisguisedGetLoggerCall(node: TSESTree.Expression): boolean { return ( - (node.callee.type === AST_NODE_TYPES.MemberExpression && - node.callee.object.type === AST_NODE_TYPES.Identifier && - node.callee.object.name.toLowerCase().includes('logger')) || - (node.callee.type === AST_NODE_TYPES.MemberExpression && - node.callee.object.type === AST_NODE_TYPES.MemberExpression && - node.callee.object.property.type === AST_NODE_TYPES.Identifier && - node.callee.object.property.name.toLowerCase().includes('logger')) + (node.type === AST_NODE_TYPES.Identifier && node.name.toLowerCase().includes('logger')) || + (node.type === AST_NODE_TYPES.MemberExpression && + node.property.type === AST_NODE_TYPES.Identifier && + node.property.name.toLowerCase().includes('logger')) ) } diff --git a/plugins/eslint-plugin-aws-toolkits/test/rules/no-json-stringify-in-log.test.ts b/plugins/eslint-plugin-aws-toolkits/test/rules/no-json-stringify-in-log.test.ts index eb29ebb6cd2..408b76e666e 100644 --- a/plugins/eslint-plugin-aws-toolkits/test/rules/no-json-stringify-in-log.test.ts +++ b/plugins/eslint-plugin-aws-toolkits/test/rules/no-json-stringify-in-log.test.ts @@ -16,6 +16,7 @@ getRuleTester().run('no-json-stringify-in-log', rules['no-json-stringify-in-log' 'getLogger().fakeFunction(`another example ${JSON.notString(something)}`)', 'this.deps.devLogger?.debug("crashMonitoring: CLEAR_STATE: Succeeded")', 'getLogger().debug(`called startBuilderIdSetup()`)', + 'this.logger.exit(`${JSON.stringify(obj)}`)', ], invalid: [ From 2204b7786ee4511eb85eb74611a4b5db37c6d879 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 29 Oct 2024 17:53:12 -0400 Subject: [PATCH 13/19] finish migration --- packages/core/src/codewhisperer/util/zipUtil.ts | 2 +- packages/core/src/lambda/commands/deploySamApplication.ts | 2 +- packages/core/src/lambda/vue/remoteInvoke/invokeLambda.ts | 2 +- packages/core/src/lambda/wizards/samDeployWizard.ts | 2 +- packages/core/src/login/webview/vue/toolkit/backend_toolkit.ts | 2 +- packages/core/src/shared/filesystemUtilities.ts | 2 +- packages/core/src/shared/sam/cli/samCliLocalInvoke.ts | 2 +- packages/core/src/shared/ui/picker.ts | 2 +- packages/core/src/shared/utilities/zipStream.ts | 2 +- packages/core/src/ssmDocument/commands/deleteDocument.ts | 2 +- packages/core/src/ssmDocument/commands/openDocumentItem.ts | 2 +- packages/core/src/ssmDocument/commands/updateDocumentVersion.ts | 2 +- 12 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/core/src/codewhisperer/util/zipUtil.ts b/packages/core/src/codewhisperer/util/zipUtil.ts index b0c22eed885..2d41da94f98 100644 --- 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/commands/deploySamApplication.ts b/packages/core/src/lambda/commands/deploySamApplication.ts index 4231b2b614e..57de09fc240 100644 --- a/packages/core/src/lambda/commands/deploySamApplication.ts +++ b/packages/core/src/lambda/commands/deploySamApplication.ts @@ -314,7 +314,7 @@ function outputDeployError(error: Error) { getLogger().error(error) globals.outputChannel.show(true) - getLogger().error('AWS.samcli.deploy.general.error', 'Error deploying a SAM Application. {0}', checklogs()) + getLogger().error('AWS.samcli.deploy.general.error: Error deploying a SAM Application. %O', checklogs()) } function getDefaultWindowFunctions(): WindowFunctions { diff --git a/packages/core/src/lambda/vue/remoteInvoke/invokeLambda.ts b/packages/core/src/lambda/vue/remoteInvoke/invokeLambda.ts index b75dd9477ba..8e1178ed00d 100644 --- a/packages/core/src/lambda/vue/remoteInvoke/invokeLambda.ts +++ b/packages/core/src/lambda/vue/remoteInvoke/invokeLambda.ts @@ -107,7 +107,7 @@ export class RemoteInvokeWebview extends VueWebview { selectedFile: fileLocations[0].path, } } 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 \n %O', fileLocations[0].fsPath, e) void vscode.window.showErrorMessage((e as Error).message) } } diff --git a/packages/core/src/lambda/wizards/samDeployWizard.ts b/packages/core/src/lambda/wizards/samDeployWizard.ts index 0f0ef4d9cf7..a45e07b0b74 100644 --- 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 index ffdad51dfa8..aa81329948f 100644 --- 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/filesystemUtilities.ts b/packages/core/src/shared/filesystemUtilities.ts index b216427634e..224bd9258d3 100644 --- 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" \n %O', err as Error) } } diff --git a/packages/core/src/shared/sam/cli/samCliLocalInvoke.ts b/packages/core/src/shared/sam/cli/samCliLocalInvoke.ts index 519b8c775af..dce6a667e46 100644 --- 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 index 67dbb217196..ed0eacf35c6 100644 --- a/packages/core/src/shared/ui/picker.ts +++ b/packages/core/src/shared/ui/picker.ts @@ -310,7 +310,7 @@ export class IteratingQuickPickController { // 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 index f4d9648957a..a1389676b75 100644 --- 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 index eb30e1d24ad..993413b6761 100644 --- 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 index 434ca7627b8..c794e56ff71 100644 --- 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 index 04a467422f7..576f38a3e75 100644 --- 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 }) } From 2b60e9883bfbade9b7858ba40a0d53c8a8ab19aa Mon Sep 17 00:00:00 2001 From: hkobew Date: Wed, 6 Nov 2024 17:52:55 -0500 Subject: [PATCH 14/19] fix new cases of mismatch sub --- .../src/awsService/appBuilder/explorer/nodes/deployedNode.ts | 2 +- .../core/src/awsService/appBuilder/explorer/samProject.ts | 2 +- packages/core/src/codewhisperer/commands/startTransformByQ.ts | 2 +- packages/core/src/lambda/vue/configEditor/samInvokeBackend.ts | 2 +- packages/core/src/lambda/vue/remoteInvoke/invokeLambda.ts | 4 ++-- packages/core/src/shared/env/resolveEnv.ts | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/core/src/awsService/appBuilder/explorer/nodes/deployedNode.ts b/packages/core/src/awsService/appBuilder/explorer/nodes/deployedNode.ts index 70ac56bb1f6..8af4d84797d 100644 --- a/packages/core/src/awsService/appBuilder/explorer/nodes/deployedNode.ts +++ b/packages/core/src/awsService/appBuilder/explorer/nodes/deployedNode.ts @@ -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 index fdb4b8e2117..fd571cd6be8 100644 --- 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 { } 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/codewhisperer/commands/startTransformByQ.ts b/packages/core/src/codewhisperer/commands/startTransformByQ.ts index edf48ed00c5..7acbf04f32e 100644 --- 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/lambda/vue/configEditor/samInvokeBackend.ts b/packages/core/src/lambda/vue/configEditor/samInvokeBackend.ts index 9e3eed9980b..643ea4631e2 100644 --- 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 index 36ea36a55c5..501304c1a94 100644 --- 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/shared/env/resolveEnv.ts b/packages/core/src/shared/env/resolveEnv.ts index 7413c9aa832..d9b8b408553 100644 --- a/packages/core/src/shared/env/resolveEnv.ts +++ b/packages/core/src/shared/env/resolveEnv.ts @@ -298,7 +298,7 @@ async function doResolveUnixShellEnv(timeout: Timeout): Promise Date: Wed, 6 Nov 2024 18:14:03 -0500 Subject: [PATCH 15/19] fix test cases that inspect the logs --- .../awsService/appBuilder/explorer/nodes/deployedNode.ts | 2 +- .../src/test/awsService/appBuilder/samProject.test.ts | 2 +- .../src/test/lambda/vue/remoteInvoke/invokeLambda.test.ts | 2 +- .../core/src/test/lambda/vue/samInvokeBackend.test.ts | 2 +- .../explorer/nodes/deployedNode.test.ts | 8 ++++---- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/core/src/awsService/appBuilder/explorer/nodes/deployedNode.ts b/packages/core/src/awsService/appBuilder/explorer/nodes/deployedNode.ts index 8af4d84797d..913cdd067e0 100644 --- 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', }) diff --git a/packages/core/src/test/awsService/appBuilder/samProject.test.ts b/packages/core/src/test/awsService/appBuilder/samProject.test.ts index 1b60c06e0ae..1d884ddd694 100644 --- 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 index 823472bfb68..5878dca3bad 100644 --- 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 index 8f3eeff2e21..25b3d1eac8f 100644 --- 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 index 8579d90fdbb..07e7ea04bae 100644 --- 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) From 1d3e68f48abcedb5f660934788150275ad1544c0 Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 7 Nov 2024 11:53:08 -0500 Subject: [PATCH 16/19] remove unrelated change --- .../lambda/commands/deploySamApplication.ts | 326 ------------------ 1 file changed, 326 deletions(-) delete mode 100644 packages/core/src/lambda/commands/deploySamApplication.ts diff --git a/packages/core/src/lambda/commands/deploySamApplication.ts b/packages/core/src/lambda/commands/deploySamApplication.ts deleted file mode 100644 index 57de09fc240..00000000000 --- a/packages/core/src/lambda/commands/deploySamApplication.ts +++ /dev/null @@ -1,326 +0,0 @@ -/*! - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -import * as path from 'path' -import * as vscode from 'vscode' -import * as nls from 'vscode-nls' - -import { asEnvironmentVariables } from '../../auth/credentials/utils' -import { AwsContext } from '../../shared/awsContext' -import globals from '../../shared/extensionGlobals' - -import { makeTemporaryToolkitFolder, tryRemoveFolder } from '../../shared/filesystemUtilities' -import { checklogs } from '../../shared/localizedText' -import { getLogger } from '../../shared/logger' -import { SamCliBuildInvocation } from '../../shared/sam/cli/samCliBuild' -import { SamCliSettings } from '../../shared/sam/cli/samCliSettings' -import { getSamCliContext, SamCliContext, getSamCliVersion } from '../../shared/sam/cli/samCliContext' -import { runSamCliDeploy } from '../../shared/sam/cli/samCliDeploy' -import { SamCliProcessInvoker } from '../../shared/sam/cli/samCliInvokerUtils' -import { runSamCliPackage } from '../../shared/sam/cli/samCliPackage' -import { throwAndNotifyIfInvalid } from '../../shared/sam/cli/samCliValidationUtils' -import { Result } from '../../shared/telemetry/telemetry' -import { addCodiconToString } from '../../shared/utilities/textUtilities' -import { SamDeployWizardResponse } from '../wizards/samDeployWizard' -import { telemetry } from '../../shared/telemetry/telemetry' - -const localize = nls.loadMessageBundle() - -interface DeploySamApplicationParameters { - sourceTemplatePath: string - deployRootFolder: string - environmentVariables: NodeJS.ProcessEnv - region: string - packageBucketName: string - ecrRepo?: string - destinationStackName: string - parameterOverrides: Map -} - -export interface WindowFunctions { - showInformationMessage: typeof vscode.window.showInformationMessage - showErrorMessage: typeof vscode.window.showErrorMessage - setStatusBarMessage(text: string, hideWhenDone: Thenable): vscode.Disposable -} - -export async function deploySamApplication( - { - samCliContext = getSamCliContext(), - samDeployWizard, - }: { - samCliContext?: SamCliContext - samDeployWizard: () => Promise - }, - { - awsContext, - settings, - window = getDefaultWindowFunctions(), - refreshFn = () => { - // no need to await, doesn't need to block further execution (true -> no telemetry) - void vscode.commands.executeCommand('aws.refreshAwsExplorer', true) - }, - }: { - awsContext: Pick - settings: SamCliSettings - window?: WindowFunctions - refreshFn?: () => void - } -): Promise { - let deployResult: Result = 'Succeeded' - let samVersion: string | undefined - let deployFolder: string | undefined - try { - const credentials = await awsContext.getCredentials() - if (!credentials) { - throw new Error('No AWS profile selected') - } - - throwAndNotifyIfInvalid(await samCliContext.validator.detectValidSamCli()) - - const deployWizardResponse = await samDeployWizard() - - if (!deployWizardResponse) { - return - } - - deployFolder = await makeTemporaryToolkitFolder('samDeploy') - samVersion = await getSamCliVersion(samCliContext) - - const deployParameters: DeploySamApplicationParameters = { - deployRootFolder: deployFolder, - destinationStackName: deployWizardResponse.stackName, - packageBucketName: deployWizardResponse.s3Bucket, - ecrRepo: deployWizardResponse.ecrRepo?.repositoryUri, - parameterOverrides: deployWizardResponse.parameterOverrides, - environmentVariables: asEnvironmentVariables(credentials), - region: deployWizardResponse.region, - sourceTemplatePath: deployWizardResponse.template.fsPath, - } - - const deployApplicationPromise = deploy({ - deployParameters, - invoker: samCliContext.invoker, - window, - }) - - window.setStatusBarMessage( - addCodiconToString( - 'cloud-upload', - localize( - 'AWS.samcli.deploy.statusbar.message', - 'Deploying SAM Application to {0}...', - deployWizardResponse.stackName - ) - ), - deployApplicationPromise - ) - - await deployApplicationPromise - refreshFn() - - // successful deploy: retain S3 bucket for quick future access - const profile = awsContext.getCredentialProfileName() - if (profile) { - await settings.updateSavedBuckets(profile, deployWizardResponse.region, deployWizardResponse.s3Bucket) - } else { - getLogger().warn('Profile not provided; cannot write recent buckets.') - } - } catch (err) { - deployResult = 'Failed' - outputDeployError(err as Error) - void vscode.window.showErrorMessage( - localize('AWS.samcli.deploy.workflow.error', 'Failed to deploy SAM application.') - ) - } finally { - await tryRemoveFolder(deployFolder) - telemetry.sam_deploy.emit({ result: deployResult, version: samVersion }) - } -} - -function getBuildRootFolder(deployRootFolder: string): string { - return path.join(deployRootFolder, 'build') -} - -function getBuildTemplatePath(deployRootFolder: string): string { - // Assumption: sam build will always produce a template.yaml file. - // If that is not the case, revisit this logic. - return path.join(getBuildRootFolder(deployRootFolder), 'template.yaml') -} - -function getPackageTemplatePath(deployRootFolder: string): string { - return path.join(deployRootFolder, 'template.yaml') -} - -async function buildOperation(params: { - deployParameters: DeploySamApplicationParameters - invoker: SamCliProcessInvoker -}): Promise { - try { - getLogger().info(localize('AWS.samcli.deploy.workflow.init', 'Building SAM Application...')) - - const buildDestination = getBuildRootFolder(params.deployParameters.deployRootFolder) - - const build = new SamCliBuildInvocation({ - buildDir: buildDestination, - baseDir: undefined, - templatePath: params.deployParameters.sourceTemplatePath, - invoker: params.invoker, - }) - - await build.execute() - - return true - } catch (err) { - getLogger().warn( - localize( - 'AWS.samcli.build.failedBuild', - '"sam build" failed: {0}', - params.deployParameters.sourceTemplatePath - ) - ) - return false - } -} - -async function packageOperation( - params: { - deployParameters: DeploySamApplicationParameters - invoker: SamCliProcessInvoker - }, - buildSuccessful: boolean -): Promise { - if (!buildSuccessful) { - void vscode.window.showInformationMessage( - localize( - 'AWS.samcli.deploy.workflow.packaging.noBuild', - 'Attempting to package source template directory directly since "sam build" failed' - ) - ) - } - - getLogger().info( - localize( - 'AWS.samcli.deploy.workflow.packaging', - 'Packaging SAM Application to S3 Bucket: {0}', - params.deployParameters.packageBucketName - ) - ) - - // HACK: Attempt to package the initial template if the build fails. - const buildTemplatePath = buildSuccessful - ? getBuildTemplatePath(params.deployParameters.deployRootFolder) - : params.deployParameters.sourceTemplatePath - const packageTemplatePath = getPackageTemplatePath(params.deployParameters.deployRootFolder) - - await runSamCliPackage( - { - sourceTemplateFile: buildTemplatePath, - destinationTemplateFile: packageTemplatePath, - environmentVariables: params.deployParameters.environmentVariables, - region: params.deployParameters.region, - s3Bucket: params.deployParameters.packageBucketName, - ecrRepo: params.deployParameters.ecrRepo, - }, - params.invoker - ) -} - -async function deployOperation(params: { - deployParameters: DeploySamApplicationParameters - invoker: SamCliProcessInvoker -}): Promise { - try { - getLogger().info( - localize( - 'AWS.samcli.deploy.workflow.stackName.initiated', - 'Deploying SAM Application to CloudFormation Stack: {0}', - params.deployParameters.destinationStackName - ) - ) - - const packageTemplatePath = getPackageTemplatePath(params.deployParameters.deployRootFolder) - - await runSamCliDeploy( - { - parameterOverrides: params.deployParameters.parameterOverrides, - environmentVariables: params.deployParameters.environmentVariables, - templateFile: packageTemplatePath, - region: params.deployParameters.region, - stackName: params.deployParameters.destinationStackName, - s3Bucket: params.deployParameters.packageBucketName, - ecrRepo: params.deployParameters.ecrRepo, - }, - params.invoker - ) - } catch (err) { - // Handle sam deploy Errors to supplement the error message prior to writing it out - const error = err as Error - - getLogger().error(error) - - const errorMessage = enhanceAwsCloudFormationInstructions(String(err), params.deployParameters) - globals.outputChannel.appendLine(errorMessage) - - throw new Error('Deploy failed') - } -} - -async function deploy(params: { - deployParameters: DeploySamApplicationParameters - invoker: SamCliProcessInvoker - window: WindowFunctions -}): Promise { - globals.outputChannel.show(true) - getLogger().info(localize('AWS.samcli.deploy.workflow.start', 'Starting SAM Application deployment...')) - - const buildSuccessful = await buildOperation(params) - await packageOperation(params, buildSuccessful) - await deployOperation(params) - - getLogger().info( - localize( - 'AWS.samcli.deploy.workflow.success', - 'Deployed SAM Application to CloudFormation Stack: {0}', - params.deployParameters.destinationStackName - ) - ) - - void params.window.showInformationMessage( - localize('AWS.samcli.deploy.workflow.success.general', 'SAM Application deployment succeeded.') - ) -} - -function enhanceAwsCloudFormationInstructions( - message: string, - deployParameters: DeploySamApplicationParameters -): string { - // detect error message from https://github.com/aws/aws-cli/blob/4ff0cbacbac69a21d4dd701921fe0759cf7852ed/awscli/customizations/cloudformation/exceptions.py#L42 - // and append region to assist in troubleshooting the error - // (command uses CLI configured value--users that don't know this and omit region won't see error) - if ( - message.includes( - `aws cloudformation describe-stack-events --stack-name ${deployParameters.destinationStackName}` - ) - ) { - message += ` --region ${deployParameters.region}` - } - - return message -} - -function outputDeployError(error: Error) { - getLogger().error(error) - - globals.outputChannel.show(true) - getLogger().error('AWS.samcli.deploy.general.error: Error deploying a SAM Application. %O', checklogs()) -} - -function getDefaultWindowFunctions(): WindowFunctions { - return { - setStatusBarMessage: vscode.window.setStatusBarMessage, - showErrorMessage: vscode.window.showErrorMessage, - showInformationMessage: vscode.window.showInformationMessage, - } -} From 708249ddadfe3f7acff7b9deed6dbad71fc8e09a Mon Sep 17 00:00:00 2001 From: Hweinstock <42325418+Hweinstock@users.noreply.github.com> Date: Thu, 7 Nov 2024 12:09:16 -0500 Subject: [PATCH 17/19] Update packages/core/src/shared/sam/cli/samCliLocalInvoke.ts Co-authored-by: Justin M. Keyes --- packages/core/src/shared/sam/cli/samCliLocalInvoke.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/shared/sam/cli/samCliLocalInvoke.ts b/packages/core/src/shared/sam/cli/samCliLocalInvoke.ts index 4c5b68daf29..14b51de6b8c 100644 --- 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: %O', `${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]}` From 88d3fa074b13927ec2425020d4d3275fe23558c4 Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 7 Nov 2024 12:22:41 -0500 Subject: [PATCH 18/19] change rule name --- .eslintrc.js | 2 +- packages/core/src/shared/filesystemUtilities.ts | 2 +- plugins/eslint-plugin-aws-toolkits/index.ts | 4 ++-- .../{no-string-sub-mismatch.ts => no-printf-mismatch.ts} | 2 +- .../test/rules/no-string-sub-mismatch.test.ts | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) rename plugins/eslint-plugin-aws-toolkits/lib/rules/{no-string-sub-mismatch.ts => no-printf-mismatch.ts} (95%) diff --git a/.eslintrc.js b/.eslintrc.js index 0063369daae..5d196c59541 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -163,7 +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-string-sub-mismatch': 'error', + 'aws-toolkits/no-printf-mismatch': 'error', 'no-restricted-imports': [ 'error', { diff --git a/packages/core/src/shared/filesystemUtilities.ts b/packages/core/src/shared/filesystemUtilities.ts index 224bd9258d3..578cc677a04 100644 --- 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" \n %O', err as Error) + getLogger().error('Error while setting "aws.downloadPath": %O', err as Error) } } diff --git a/plugins/eslint-plugin-aws-toolkits/index.ts b/plugins/eslint-plugin-aws-toolkits/index.ts index 32b6bb30320..87ee111c095 100644 --- a/plugins/eslint-plugin-aws-toolkits/index.ts +++ b/plugins/eslint-plugin-aws-toolkits/index.ts @@ -10,7 +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 noStringSubMismatch from './lib/rules/no-string-sub-mismatch' +import noPrintfMismatch from './lib/rules/no-printf-mismatch' const rules = { 'no-await-on-vscode-msg': NoAwaitOnVscodeMsg, @@ -20,7 +20,7 @@ const rules = { 'no-string-exec-for-child-process': NoStringExecForChildProcess, 'no-console-log': NoConsoleLog, 'no-json-stringify-in-log': noJsonStringifyInLog, - 'no-string-sub-mismatch': noStringSubMismatch, + 'no-printf-mismatch': noPrintfMismatch, } export { rules } diff --git a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-sub-mismatch.ts b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-printf-mismatch.ts similarity index 95% rename from plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-sub-mismatch.ts rename to plugins/eslint-plugin-aws-toolkits/lib/rules/no-printf-mismatch.ts index ad9ef74cefb..41485540a04 100644 --- a/plugins/eslint-plugin-aws-toolkits/lib/rules/no-string-sub-mismatch.ts +++ b/plugins/eslint-plugin-aws-toolkits/lib/rules/no-printf-mismatch.ts @@ -18,7 +18,7 @@ function countSubTokens(literalNode: TSESTree.StringLiteral) { * Allows us to avoid copy pasting message into test file. */ export function formErrorMsg(substitutionTokens: string | number, numArgs: string | number): string { - return `String substitution has ${substitutionTokens} format specifiers, but ${numArgs} arguments.` + return `printf-style (console.log) call has ${substitutionTokens} format specifiers, but ${numArgs} arguments.` } export default ESLintUtils.RuleCreator.withoutDocs({ diff --git a/plugins/eslint-plugin-aws-toolkits/test/rules/no-string-sub-mismatch.test.ts b/plugins/eslint-plugin-aws-toolkits/test/rules/no-string-sub-mismatch.test.ts index 3149950fdd4..a91e58f2f19 100644 --- a/plugins/eslint-plugin-aws-toolkits/test/rules/no-string-sub-mismatch.test.ts +++ b/plugins/eslint-plugin-aws-toolkits/test/rules/no-string-sub-mismatch.test.ts @@ -4,7 +4,7 @@ */ import { rules } from '../../index' -import { formErrorMsg } from '../../lib/rules/no-string-sub-mismatch' +import { formErrorMsg } from '../../lib/rules/no-printf-mismatch' import { getRuleTester } from '../testUtil' getRuleTester().run('no-string-sub-mismatch', rules['no-string-sub-mismatch'], { From bbf54c571c4377f06958510b92254a9f95df40cc Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 7 Nov 2024 12:23:55 -0500 Subject: [PATCH 19/19] change test file name --- ...o-string-sub-mismatch.test.ts => no-printf-mismatch.test.ts} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename plugins/eslint-plugin-aws-toolkits/test/rules/{no-string-sub-mismatch.test.ts => no-printf-mismatch.test.ts} (92%) diff --git a/plugins/eslint-plugin-aws-toolkits/test/rules/no-string-sub-mismatch.test.ts b/plugins/eslint-plugin-aws-toolkits/test/rules/no-printf-mismatch.test.ts similarity index 92% rename from plugins/eslint-plugin-aws-toolkits/test/rules/no-string-sub-mismatch.test.ts rename to plugins/eslint-plugin-aws-toolkits/test/rules/no-printf-mismatch.test.ts index a91e58f2f19..160f6dc505a 100644 --- a/plugins/eslint-plugin-aws-toolkits/test/rules/no-string-sub-mismatch.test.ts +++ b/plugins/eslint-plugin-aws-toolkits/test/rules/no-printf-mismatch.test.ts @@ -7,7 +7,7 @@ import { rules } from '../../index' import { formErrorMsg } from '../../lib/rules/no-printf-mismatch' import { getRuleTester } from '../testUtil' -getRuleTester().run('no-string-sub-mismatch', rules['no-string-sub-mismatch'], { +getRuleTester().run('no-string-sub-mismatch', rules['no-printf-mismatch'], { valid: [ 'getLogger().debug("this is a string %s and a number %d", "s", 2)', 'getLogger().debug("this is a number %d", 2)',