-
Notifications
You must be signed in to change notification settings - Fork 634
Closed
Labels
bugThis issue is a bug.This issue is a bug.needs-triageThis issue or PR still needs to be triaged.This issue or PR still needs to be triaged.
Description
Checkboxes for prior research
- I've gone through Developer Guide and API reference
- I've checked AWS Forums and StackOverflow.
- I've searched for previous similar issues and didn't find any solution.
Describe the bug
In the fromHttp
credential provider, the function options.logger.warn
is copied into a local variable, and that function is subsequently called:
const warn: (warning: string) => void = | |
options.logger?.constructor?.name === "NoOpLogger" || !options.logger ? console.warn : options.logger.warn; | |
if (relative && full) { | |
warn( | |
"@aws-sdk/credential-provider-http: " + | |
"you have set both awsContainerCredentialsRelativeUri and awsContainerCredentialsFullUri." | |
); | |
warn("awsContainerCredentialsFullUri will take precedence."); | |
} |
If options.logger
happens to point to a class, that might be implemented with a reference to this
, then the reference to this
is lost and an error will be thrown.
class SomeLogger {
public warn(...args: string[]) {
this.helper(...args); // 💥 'this' is not bound to anything because the function was not called on an object
// Cannot read properties of undefined
}
private helper(...args:string[]) {
// whatever
}
}
The warn function should retain a reference to the original object by using bind()
, otherwise it is not possible to use classes as loggers which is very unexpected behavior.
const warn: (warning: string) => void =
options.logger?.constructor?.name === "NoOpLogger" || !options.logger ? console.warn : options.logger.warn.bind(options.logger);
This pattern might occur in more places than just the place we ran into it.
Regression Issue
- Select this option if this issue appears to be a regression.
SDK version number
Latest
Which JavaScript Runtime is this issue in?
Node.js
Details of the browser/Node.js/ReactNative version
All nodes
Reproduction Steps
See above.
Observed Behavior
See above.
Expected Behavior
See above.
Possible Solution
No response
Additional Information/Context
No response
Metadata
Metadata
Assignees
Labels
bugThis issue is a bug.This issue is a bug.needs-triageThis issue or PR still needs to be triaged.This issue or PR still needs to be triaged.