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.

Sign up for GitHub

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

Jump to bottom

Capture and instrument cache errors #7021

Merged
merged 3 commits into from
Apr 10, 2024
Merged

Capture and instrument cache errors #7021

merged 3 commits into from
Apr 10, 2024

Conversation

konstantin-msft
Copy link
Collaborator

  • Add CacheError.
  • Capture and throw cache errors as CacheError.
  • Instrument the number of tokens in the cache if CacheError occurs.

Note: Cache errors, like "cache quota exceeded", are not captured and reported as empty errors at the moment.

@github-actions github-actions bot added msal-browser Related to msal-browser package msal-common Related to msal-common package labels Apr 10, 2024
@konstantin-msft
- Add CacheError.
713a5b9 
- Capture and throw cache errors as CacheError.
- Instrument the number of tokens in the cache if CacheError occurs.
@konstantin-msft
Change files
9081c92
@konstantin-msft
- Add try/catch block to saveCacheRecord error handler to make sure w…
939cbeb 
…e never throw a non-cache error inside it.
@konstantin-msft konstantin-msft merged commit b7c2309 into dev Apr 10, 2024
44 checks passed
@konstantin-msft konstantin-msft deleted the capture_cache_errors branch April 10, 2024 21:13
sameerag
sameerag reviewed Apr 10, 2024
View reviewed changes
Copy link
Member

@sameerag sameerag 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 getting this in, like the coverage. I assume this will also mean we can get the percentage of these errors in dashboards too?

@konstantin-msft
Copy link
Collaborator Author

Thanks for getting this in, like the coverage. I assume this will also mean we can get the percentage of these errors in dashboards too?

Yes, we will be able to pivot on cache errors too,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sameerag sameerag sameerag left review comments

@hectormmg hectormmg hectormmg approved these changes

@tnorling tnorling tnorling approved these changes

@jo-arroyo jo-arroyo Awaiting requested review from jo-arroyo jo-arroyo is a code owner

@peterzenz peterzenz Awaiting requested review from peterzenz peterzenz is a code owner

@Robbie-Microsoft Robbie-Microsoft Awaiting requested review from Robbie-Microsoft Robbie-Microsoft is a code owner

Assignees
No one assigned
Labels
msal-browser Related to msal-browser package msal-common Related to msal-common package
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@konstantin-msft @hectormmg @tnorling @sameerag