Skip to content

Commit 4add487

Browse files
authored
build(lint): printf/console.log format specifiers must match args #5895
## Problem currently `logger.error(msg, err)` silently drops `err` if it doesn't map to a `%x` format specifier in msg. we want to either prevent that mistake, or enhance our logger so that it prints useful parts of err instead of silently dropping it. Also, there are cases like: `getLogger().error('Certificate path {0} already exists', certPath)`, but this silently drops the arg `certPath`. Pretty sure this syntax is from Java IIRC and not supported by Node. ## Solution Provide a custom lint rule to ensure the number of extra args matches the number of format specifiers. ### Note About Alternatives - As an alternative, we could have transformed the extra `meta` items to strings and appended them to the log message or add them as separate log messages. This would allow usage of the form `getLogger().error('hit error', e)`. - If we add separate log messages, we break the expectation that each `logger.error` statement corresponds to one message in the log. - Conversely, if we append to the log message, this is somewhat of a hidden feature not exposed to the consumer. By forcing them to add a '%O' format specifier, we are making them clearly aware of how the error (or other object) will show up in the log, ensuring the effect does not "surprise" the user. ## Notes Relevant: code used to format errors: https://github.com/aws/aws-toolkit-vscode/blob/f806ae04f5d56b872282e42d5f765b2b1a9e28dd/packages/core/src/shared/errors.ts#L329-L334 which should now actually be used since errors won't be dropped.
1 parent e817c2d commit 4add487

File tree

33 files changed

+133
-36
lines changed

33 files changed

+133
-36
lines changed

.eslintrc.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ module.exports = {
163163
'aws-toolkits/no-string-exec-for-child-process': 'error',
164164
'aws-toolkits/no-console-log': 'error',
165165
'aws-toolkits/no-json-stringify-in-log': 'error',
166+
'aws-toolkits/no-printf-mismatch': 'error',
166167
'no-restricted-imports': [
167168
'error',
168169
{

packages/core/src/applicationcomposer/messageHandlers/emitTelemetryMessageHandler.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ export function emitTelemetryMessageHandler(message: EmitTelemetryMessage) {
6767
return
6868
}
6969
} catch (e) {
70-
getLogger().error('Could not log telemetry for App Composer', e)
70+
getLogger().error('Could not log telemetry for App Composer %O', e)
7171
}
7272
}
7373

packages/core/src/awsService/appBuilder/explorer/nodes/deployedNode.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ export async function generateDeployedNode(
9696
.Configuration as Lambda.FunctionConfiguration
9797
newDeployedResource = new LambdaFunctionNode(lambdaNode, regionCode, configuration)
9898
} catch (error: any) {
99-
getLogger().error('Error getting Lambda configuration')
99+
getLogger().error('Error getting Lambda configuration %O', error)
100100
throw ToolkitError.chain(error, 'Error getting Lambda configuration', {
101101
code: 'lambdaClientError',
102102
})
@@ -153,7 +153,7 @@ export async function generateDeployedNode(
153153
}
154154
default:
155155
newDeployedResource = new DeployedResourceNode(deployedResource)
156-
getLogger().info('Details are missing or are incomplete for:', deployedResource)
156+
getLogger().info('Details are missing or are incomplete for: %O', deployedResource)
157157
return [
158158
createPlaceholderItem(
159159
localize('AWS.appBuilder.explorerNode.noApps', '[This resource is not yet supported.]')

packages/core/src/awsService/appBuilder/explorer/samProject.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export async function getStackName(projectRoot: vscode.Uri): Promise<any> {
4242
} catch (error: any) {
4343
switch (error.code) {
4444
case SamConfigErrorCode.samNoConfigFound:
45-
getLogger().info('No stack name or region information available in samconfig.toml', error)
45+
getLogger().info('No stack name or region information available in samconfig.toml: %O', error)
4646
break
4747
case SamConfigErrorCode.samConfigParseError:
4848
getLogger().error(`Error getting stack name or region information: ${error.message}`, error)

packages/core/src/awsService/iot/commands/createCert.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,14 +115,14 @@ async function saveCredentials(
115115
const publicKeyExists = await fileExists(publicKeyPath)
116116

117117
if (certExists) {
118-
getLogger().error('Certificate path {0} already exists', certPath)
118+
getLogger().error('Certificate path %s already exists', certPath)
119119
void vscode.window.showErrorMessage(
120120
localize('AWS.iot.createCert.error', 'Failed to create certificate. Path {0} already exists.', certPath)
121121
)
122122
return false
123123
}
124124
if (privateKeyExists) {
125-
getLogger().error('Key path {0} already exists', privateKeyPath)
125+
getLogger().error('Key path %s already exists', privateKeyPath)
126126
void vscode.window.showErrorMessage(
127127
localize(
128128
'AWS.iot.createCert.error',
@@ -133,7 +133,7 @@ async function saveCredentials(
133133
return false
134134
}
135135
if (publicKeyExists) {
136-
getLogger().error('Key path {0} already exists', publicKeyPath)
136+
getLogger().error('Key path %s already exists', publicKeyPath)
137137
void vscode.window.showErrorMessage(
138138
localize(
139139
'AWS.iot.createCert.error',

packages/core/src/awsService/redshift/explorer/redshiftNode.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ export class RedshiftNode extends AWSTreeNodeBase implements LoadMoreNode {
127127
newServerlessToken = response.nextToken ?? ''
128128
}
129129
} catch (error) {
130-
getLogger().error("Serverless workgroup operation isn't supported or failed:", error)
130+
getLogger().error("Serverless workgroup operation isn't supported or failed: %O", error)
131131
// Continue without interrupting the provisioned cluster loading
132132
}
133133
}

packages/core/src/awsService/s3/commands/uploadFile.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ export async function promptUserForBucket(
415415
try {
416416
allBuckets = await s3client.listAllBuckets()
417417
} catch (e) {
418-
getLogger().error('Failed to list buckets from client', e)
418+
getLogger().error('Failed to list buckets from client %O', e)
419419
void vscode.window.showErrorMessage(
420420
localize('AWS.message.error.promptUserForBucket.listBuckets', 'Failed to list buckets from client')
421421
)

packages/core/src/codewhisperer/activation.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,6 @@ export async function enableDefaultConfigCloud9() {
618618
await editorSettings.update('acceptSuggestionOnEnter', 'on', vscode.ConfigurationTarget.Global)
619619
await editorSettings.update('snippetSuggestions', 'top', vscode.ConfigurationTarget.Global)
620620
} catch (error) {
621-
getLogger().error('amazonq: Failed to update user settings', error)
621+
getLogger().error('amazonq: Failed to update user settings %O', error)
622622
}
623623
}

packages/core/src/codewhisperer/client/agent.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,6 @@ export function initializeNetworkAgent(): void {
8282
}
8383
} catch (error) {
8484
// Log any errors in the patching logic
85-
getLogger().error('Failed to patch http agent', error)
85+
getLogger().error('Failed to patch http agent %O', error)
8686
}
8787
}

packages/core/src/codewhisperer/commands/startSecurityScan.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ export async function startSecurityScan(
225225

226226
logger.verbose(`Security scan completed.`)
227227
} catch (error) {
228-
getLogger().error('Security scan failed.', error)
228+
getLogger().error('Security scan failed. %O', error)
229229
if (error instanceof CodeScanStoppedError) {
230230
codeScanTelemetryEntry.result = 'Cancelled'
231231
} else {

0 commit comments

Comments
 (0)