-
Notifications
You must be signed in to change notification settings - Fork 295
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
[DDW-231] log expected errors as debug messages only #916
Conversation
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.
Nice work @DominikGuzei!
There are a few things I would like to change:
1). Perhaps we should log handled errors with the Logger.debug
and unhandled/unexpected ones with Logger.error
same as it is done here: https://github.com/input-output-hk/daedalus/blob/develop/source/renderer/app/api/ada/index.js#L631-L635 - I believe that would be the proper way to go!
2). I think we should apply the same logic to ETC Api too.
3). Please add CHANGELOG entry.
Thanks for making these changes!
@nikolaglumac i agree that it would be "better" … unfortunately it's not easily possible without a lot of duplicated logging statements or boilerplate around it. Basically I would have to duplicate the error logging into any if statement or set a flag like |
@DominikGuzei roger that! |
working on those now @nikolaglumac 👍 |
@nikolaglumac changelog and ETC updates are added! |
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.
Looks good now! Approved @DominikGuzei
This PR improves our error handling by logging expected error cases only as debug messages instead of spamming our dev console.
Review Checklist:
Basics
npm run test
)npm run dev
)npm run package
/ CI builds)npm run flow-test
)npm run lint
)npm run manage-translations
produces no changes)npm run storybook
)package-lock.json
andyarn.lock
files are updatedCode Quality
Testing
After Review:
done
on the Youtrack board