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

feat: Log the response body when the request succeeds with LogLevel.DEBUG #479

Closed
wants to merge 3 commits into from

Conversation

whatasame
Copy link

@whatasame whatasame commented Dec 17, 2023

Issue

The README mentions that the response body will be logged on the DEBUG level, but the actual code currently supports logging only in error response cases. See following excerpt from the README.md

If you're debugging an application, and would like the client to log response bodies, set the logLevel option to LogLevel.DEBUG.

Implementation

Added a log method for success response cases.

Note

This produces a non-pretty JSON string to prevent unintended creation of strings on non-DEBUG levels.

Example

image

@CLAassistant
Copy link

CLAassistant commented Dec 17, 2023

CLA assistant check
All committers have signed the CLA.

src/Client.ts Outdated
@@ -220,6 +220,7 @@ export default class Client {

const responseJson: ResponseBody = JSON.parse(responseText)
this.log(LogLevel.INFO, `request success`, { method, path })
this.log(LogLevel.DEBUG, `succeed response body`, { body: responseText })

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you make this consistent with the previous log?

Suggested change
this.log(LogLevel.DEBUG, `succeed response body`, { body: responseText })
this.log(LogLevel.DEBUG, `response body success`, { body: responseText })

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @varunrau! Thank you for the comment.

I believe your opinion is appropriate, but I have added it to align with the pattern seen in the after log, especially considering the 230-241 lines handling errors.

When an error occurs in the request

  • request fail -> failed response body

When the request is succeeded

  • request success -> succeeded response body (what I added)

@cwang314
Copy link
Contributor

Hey, thanks for opening this PR. We're doing some git history cleanup and need to close the PR as it'll become invalid. Afterwards, please reclone the repo and you can reopen the PR. Thanks for your patience and sincere apologies for the trouble.

@cwang314 cwang314 closed this Jan 24, 2024
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

Successfully merging this pull request may close these issues.

4 participants