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(logging): Use loglevel library for error handling #1845

Merged
merged 5 commits into from
Feb 10, 2025

Conversation

Ninfa-Jeon
Copy link
Contributor

Done

  • Installed loglevel
  • Replaced console.error with log.error
  • Fixed and updated tests

QA

  • All tests should run

Details

https://warthogs.atlassian.net/browse/WD-18724

@webteam-app
Copy link

@Ninfa-Jeon Ninfa-Jeon requested a review from huwshimi February 4, 2025 14:25
Copy link

github-actions bot commented Feb 4, 2025

Coverage Report

Status Category Percentage Covered / Total
🟢 Lines 97.96% (🎯 95%) 14649 / 14954
🟢 Statements 97.96% (🎯 95%) 14649 / 14954
🟢 Functions 98.18% (🎯 95%) 596 / 607
🟢 Branches 92.03% (🎯 90%) 2948 / 3203
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/index.tsx 93.89% 89.79% 100% 93.89% 33-37, 109, 155-156
src/components/LogIn/UserPassForm/UserPassForm.tsx 100% 100% 100% 100%
src/components/ShareCard/ShareCard.tsx 98.31% 90.9% 83.33% 98.31% 40, 121
src/hooks/useLocalStorage.ts 100% 100% 100% 100%
src/hooks/useLogout.tsx 100% 100% 100% 100%
src/hooks/useQueryParams.ts 100% 100% 100% 100%
src/juju/api.ts 98.76% 94.11% 95.45% 98.76% 159, 252, 478-479, 506-507
src/pages/EntityDetails/Model/Logs/ActionLogs/ActionLogs.tsx 96.01% 85.36% 100% 96.01% 86-87, 95, 117-122, 230-231, 241-242
src/pages/ModelDetails/ModelDetails.tsx 100% 100% 100% 100%
src/panels/ActionsPanel/ActionsPanel.tsx 99.43% 92.5% 100% 99.43% 135
src/panels/ActionsPanel/ConfirmationDialog/ConfirmationDialog.tsx 100% 100% 100% 100%
src/panels/CharmsAndActionsPanel/CharmsAndActionsPanel.tsx 100% 100% 100% 100%
src/panels/ConfigPanel/ConfigPanel.tsx 99.45% 97.8% 100% 99.45% 415-416
src/panels/ConfigPanel/ConfirmationDialog/ConfirmationDialog.tsx 98.72% 94.87% 100% 98.72% 142-143
src/store/store.ts 82.85% 75% 100% 82.85% 17-22
src/store/app/thunks.ts 98.46% 75% 100% 98.46% 34
src/store/juju/thunks.ts 83.72% 66.66% 100% 83.72% 34-35, 57-61
src/store/middleware/check-auth.ts 95.95% 94.73% 100% 95.95% 36-40
src/store/middleware/model-poller.ts 95.38% 91.46% 100% 95.38% 49-50, 111-118, 139, 141-142, 191, 389
src/utils/logger.ts 100% 100% 100% 100%
Generated in workflow #42 for commit ad8d390 by the Vitest Coverage Report Action

Copy link
Contributor

@huwshimi huwshimi left a comment

Choose a reason for hiding this comment

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

Not sure if you want to do this in the same PR, but we should probably do the same for our console.log calls as well.

We might want to default to silent in prod and maybe in tests as well.

For prod we could have a console.log when the app loads to output something like 'Run log.setLevel("trace") to output errors to the console.' and then expose loglevel as a global log variable.

@Ninfa-Jeon
Copy link
Contributor Author

Not sure if you want to do this in the same PR, but we should probably do the same for our console.log calls as well.

We might want to default to silent in prod and maybe in tests as well.

For prod we could have a console.log when the app loads to output something like 'Run log.setLevel("trace") to output errors to the console.' and then expose loglevel as a global log variable.

While I'm working on setting the log levels, I'm not sure I've initialised it right in test setup file.

Copy link
Contributor

@huwshimi huwshimi left a comment

Choose a reason for hiding this comment

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

A couple of suggestions here so that you can remove the remaining logging mocks.

@huwshimi
Copy link
Contributor

huwshimi commented Feb 5, 2025

While I'm working on setting the log levels, I'm not sure I've initialised it right in test setup file.

It seems like it's working for me, there are no errors being displayed in the console and if I add logger.setLevel("DEBUG"); to a test file I see the errors.

@Ninfa-Jeon
Copy link
Contributor Author

While I'm working on setting the log levels, I'm not sure I've initialised it right in test setup file.

It seems like it's working for me, there are no errors being displayed in the console and if I add logger.setLevel("DEBUG"); to a test file I see the errors.

Okay, I've not made further changes to this since it was reported to be working fine. Additionally, I have replaced console.log with logger.log

Copy link
Contributor

@huwshimi huwshimi left a comment

Choose a reason for hiding this comment

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

Thanks for all the work on this.

@huwshimi huwshimi merged commit 5401e10 into canonical:main Feb 10, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants