fix(auth): persist plan update even when org fetch fails on self-hosted#1896
Merged
richiemcilroy merged 2 commits intoJun 7, 2026
Merged
Conversation
Comment on lines
+108
to
+110
| Err(e) => { | ||
| println!("Failed to fetch organizations: {e}"); | ||
| } |
Contributor
There was a problem hiding this comment.
Use structured logging instead of
println!
The rest of the error-path in this function already uses println! (lines 81, 84), so this is consistent — but production Tauri apps typically route log::warn! / tracing::warn! through the platform logging backend (accessible in the DevTools console and system log), whereas println! only appears on stdout. A swallowed org-fetch error that only appears on stdout can be hard to surface in the field, especially on self-hosted deployments where the terminal is rarely visible.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src-tauri/src/auth.rs
Line: 108-110
Comment:
**Use structured logging instead of `println!`**
The rest of the error-path in this function already uses `println!` (lines 81, 84), so this is consistent — but production Tauri apps typically route `log::warn!` / `tracing::warn!` through the platform logging backend (accessible in the DevTools console and system log), whereas `println!` only appears on stdout. A swallowed org-fetch error that only appears on stdout can be hard to surface in the field, especially on self-hosted deployments where the terminal is rarely visible.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Contributor
Author
|
@greptileai please re-review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The previous auth loop fix stopped the double deep-link delivery, but auth still didn't persist on self-hosted because of a second bug in update_auth_plan.
When fetch_organizations failed (common on self-hosted where the org endpoint returns an unexpected response), the ? operator bailed early and Self::set(app, Some(auth)) was never called, meaning the plan update was computed but never written to disk.
On next app start, organizations was always empty, hasAvailableOrganizationCache returned false, and the UI showed the login screen even though valid credentials were in the store.
Fix
Treat org fetch failure as non-fatal, log the error, keep existing organizations, and still persist the plan update to disk.
Greptile Summary
This PR fixes a bug in
update_auth_planwhere a?-operator early-return onfetch_organizationsfailure preventedSelf::setfrom ever being called, causing the computed plan update to be silently discarded on self-hosted deployments.fetch_organizationscall is now wrapped in amatchso failures are logged viatracing::warn!and the existing (stale) organizations are preserved rather than bailing early.Self::set(app, Some(auth))now always executes after the plan fetch, persisting the plan to disk regardless of whether org refresh succeeded.Confidence Score: 5/5
Safe to merge — the change is a minimal, targeted fix that converts one early-return path to a non-fatal branch, with no impact on the plan-fetch success path.
The fix is small and correct: it ensures the plan is always written to the store regardless of whether org fetch succeeds. When org fetch fails, the in-memory auth already holds the previously stored organizations, so persisting it does not accidentally wipe previously cached org data.
No files require special attention.
Important Files Changed
fetch_organizationsinto a non-fatal warning, ensuringSelf::setis always called and the plan update is always persisted to disk.Reviews (2): Last reviewed commit: "fix(auth): use tracing::warn for org fet..." | Re-trigger Greptile