fix: log token cache errors instead of silently swallowing them#193
Conversation
🦋 Changeset detectedLatest commit: b3a40a1 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical user experience issue where token cache loading failures were silently ignored, leading to users being unable to authenticate without understanding why. By introducing explicit error handling and informative logging, the changes ensure that users receive clear feedback when their token cache is corrupted or inaccessible, guiding them towards a resolution. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request significantly improves the error handling within the load_from_disk function by replacing nested if let Ok statements with explicit match arms. This change ensures that specific failures during token cache loading, such as decryption errors, invalid UTF-8, or JSON parsing issues, are now logged to stderr with informative warnings and hints for the user. This addresses the problem of silently failing token loads, which previously led to confusing authentication issues. The .changeset file accurately documents these improvements.
load_from_disk used four nested if-let-Ok blocks that silently returned an empty HashMap on any failure. When the encryption key rotated or the cache file was corrupted, tokens silently stopped loading and users were forced to re-authenticate with no explanation. Replace with explicit match arms that log specific warnings to stderr for each failure mode: - Decryption failure (key changed, corrupted data) - Invalid UTF-8 in decrypted data - JSON deserialization failure File-not-found is still silent since that's normal on first run.
jpoehnelt
added
area: auth
cla: yes
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #193 +/- ##
==========================================
- Coverage 56.21% 56.17% -0.05%
==========================================
Files 38 38
Lines 13853 13863 +10
==========================================
Hits 7788 7788
- Misses 6065 6075 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
2 participants
Summary
load_from_diskused four nestedif let Okblocks that silently returned an empty HashMap on any failure. When the encryption key rotated or the cache was corrupted, tokens silently stopped loading and users were forced to re-authenticate with no explanation of what went wrong.This is a likely contributor to issues like #151 and #137 where users report "credentials saved successfully" but get 401 on all commands.
Changes
src/token_storage.rs: Replace nestedif let Okwith explicitmatcharms that log specific warnings to stderr:Test plan
cargo test,cargo clippy,cargo fmt