Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

S3 Client customer logger doesn't print debug message properly #1267

Closed
king4sam opened this issue May 9, 2024 · 3 comments
Closed

S3 Client customer logger doesn't print debug message properly #1267

king4sam opened this issue May 9, 2024 · 3 comments

Comments

@king4sam
Copy link
Contributor

king4sam commented May 9, 2024

S3 Client customer logger doesn't print debug message properly #4650

There was a previous issue fixed. However, I found that the following files are still printing only 'endpoints' without useful debug messages.

packages/util-endpoints/utils/evaluateEndpointRule.ts
packages/util-endpoints/utils/evaluateConditions.ts
packages/util-endpoints/utils/evaluateCondition.ts

I would apply the same patch to these files.

king4sam pushed a commit to king4sam/smithy-typescript that referenced this issue May 9, 2024
king4sam pushed a commit to king4sam/smithy-typescript that referenced this issue May 9, 2024
king4sam pushed a commit to king4sam/smithy-typescript that referenced this issue May 9, 2024
@syall
Copy link
Contributor

syall commented May 9, 2024

Generally, the Logger interface methods use rest parameters. If you see only endpoints, it is probably due to the logger implementation only looking at the first argument rather than all of them.

That being said, I think it's ok to make the messages print a single string in the spots you mentioned.

@king4sam
Copy link
Contributor Author

Based on my understanding, pino and winston are two of the most popular logging libraries for Node.js.
While they do not follow strictly the logger interface, it would be beneficial for greater compatibility with these widely-used options.

@syall syall closed this as completed May 10, 2024
@syall
Copy link
Contributor

syall commented May 10, 2024

#1268 was merged, closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants