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

Improving logging message types and error messages #14

Closed
wants to merge 0 commits into from

Conversation

Girishbari
Copy link
Contributor

@Girishbari Girishbari commented Oct 16, 2024

Description

Resolve : #6
Improve log message and tweaked some error message conveying a proper error or suggestion

Type of change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • refactor

Checklist:

@Kushal7788
Copy link
Collaborator

Kushal7788 commented Oct 16, 2024

Some changes are required.

  • I'll suggest to go through the changes once more and to make sure the code functionality is not affected when adding logs.
  • We would also like to add more info logs as the user calls the public methods of the sdk to give more context and help user with debugging their application.
  • With addition of different log types, we need to provider a way to the user to set the logging level in proofOptions when log are enabled. Default to showing all log types but giving an option to the user to set the visible log types if needed.

@adithyaakrishna
Copy link
Contributor

@Girishbari Any updates on these?

@Girishbari
Copy link
Contributor Author

yeah been busy with work and ideas to build in the reclaim hackathon, I will update you tomorrow on this although I have a few points to asks

  1. how much info should these logs have while taking note that I don't entirely know the SDK and their public method or if there are docs for these methods already which I can refer to build user-friendly messages
  2. I didn't get the last point, could you please elaborate on that

@Girishbari
Copy link
Contributor Author

hi @Kushal7788 found this error while doing local build
image
should I change it here or make a new and then push for the change

@Kushal7788
Copy link
Collaborator

Kushal7788 commented Oct 22, 2024

hi @Kushal7788 found this error while doing local build image should I change it here or make a new and then push for the change

@Girishbari this is because you have modified the import function names along with the logs update in this branch. I have added comments for the same.

This error is not there on main branch. You can check by taking latest pull from main.

@Kushal7788
Copy link
Collaborator

yeah been busy with work and ideas to build in the reclaim hackathon, I will update you tomorrow on this although I have a few points to asks

  1. how much info should these logs have while taking note that I don't entirely know the SDK and their public method or if there are docs for these methods already which I can refer to build user-friendly messages
  2. I didn't get the last point, could you please elaborate on that

To answer your queries

  1. You can refer to our docs for more information on public methods. I guess you can go through the Reclaim.ts file and directly look at the codebase too. We want some info messages to help users as well as our internal team to understand the errors and help debug them.
  2. Currently if you look at the codebase, we support only info logs when logging is enabled. After adding error and warn logs, we need to explicitly set the logging level to either one of the info, warn or error. We want to give that option to the end user to chose among the logging level once they enable logs. If user has not set any logging level then we want to show all the logs from all logging levels. This change need to happen in init public method for the SDK. We need to give an additional parameter in options for logging level.

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.

Improve and Standardize Error Handling and Logging Strategy
3 participants