-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Format error logs in debug module [issue 9244] #9386
Format error logs in debug module [issue 9244] #9386
Conversation
1756ae7
to
b939b63
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anishbhanwala thank you for your first contribution to the framework 👍
Do you mind updating amending the commit to provide a more meaningful title, at the moment it is your sign-off
which does not mean much in the git history
:
commit b939b63ee6074253c3b809af19bd1f8255f36f75 (HEAD -> anishbhanwala-debug-module-format_logs_9244)
Author: Anish Bhanwala <[email protected]>
Date: Wed Apr 21 15:41:12 2021 +0530
Signed-off-by: Anish Bhanwala <[email protected]>
Related to issue: #9244
In case where error is logged as console.error(error) it gets stringified to [object Object].
So replaced such occurrences with the format console.error('<Method> failed:', e).
@colin-grant-work I have not followed the discussion closely, should we be prefixing the errors with debug -
so errors like setValue
or saveAll
are more meaningful?
In case where error is logged as console.error(error) it gets stringified to [object Object]. So replaced such occurrences with the format console.error('<Method> failed:', e). Signed-off-by: Anish Bhanwala <[email protected]>
b939b63
to
c123624
Compare
@vince-fugnitto I have updated the commit message. Please check if it makes sense now. |
Thank you! |
@vince-fugnitto, that's certainly an option, but I don't think it's necessary. The problem before this change is that the log would say only |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anishbhanwala the changes look good to me, I look forward to many more contributions in the future!
@vince-fugnitto Yes, I will try to find some issue today. If you have any suggestions that would be helpful. |
What it does
Fixes: #9244
In case where error is logged as
console.error(error)
it gets stringified to[object Object]
.So replaced such occurrences with the format
console.error('<Method> failed:', e)
.Made changes only for the
debug
module.How to test
I tested by manually throwing the error.
Review checklist
Reminder for reviewers