-
Notifications
You must be signed in to change notification settings - Fork 19.6k
fix: signin page stuck on loading when refresh token valid but access token expired #30675
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
Conversation
… token expired This commit fixes two bugs that caused the signin page to get stuck on loading: 1. Remove `loginData?.logged_in` from isLoading condition in normal-form.tsx - Previously, when logged_in=true, isLoading was always true - Now use explicit `isRedirecting` state to track redirect status - This prevents infinite loading if router.replace fails or delays 2. Fix error handling in useIsLogin hook (use-common.ts) - Previously, non-401 errors (500, 403, network errors) were treated as logged in - Now any error is treated as "not logged in" Fixes #30674 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @lyzno1, 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 resolves a critical issue where the signin page could become unresponsive, displaying a perpetual loading state, particularly when a user's access token had expired but a valid refresh token was present. It achieves this by refining the loading state management on the signin page and enhancing the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request effectively resolves an issue where the sign-in page could get stuck in a loading state. The changes in web/service/use-common.ts correctly simplify the error handling within the useIsLogin hook, ensuring that any failure in fetching the user profile is treated as a 'not logged in' state. This is a crucial correctness fix. Additionally, the refactoring in web/app/signin/normal-form.tsx to use an explicit isRedirecting state for the loading indicator is a great improvement. It makes the component's state management more robust and its behavior more predictable during redirection. The changes are well-implemented and address the problem described.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a bug where the signin page would get stuck on a loading screen when users have a valid refresh token but an expired access token. The fix involves two key changes: improving error handling in the login check hook and refactoring the loading state management to use an explicit redirect state.
- Simplified error handling in
useIsLoginto treat any error as "not logged in" - Removed problematic
loginData?.logged_infrom loading condition and added explicitisRedirectingstate
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| web/service/use-common.ts | Updated useIsLogin error handling to catch all errors instead of only checking for 401 status |
| web/app/signin/normal-form.tsx | Fixed loading state logic by removing loginData?.logged_in from isLoading condition and introducing explicit isRedirecting state to prevent infinite loading |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code |
Summary
loginData?.logged_infromisLoadingcondition and use explicitisRedirectingstateuseIsLoginhook to treat any error as "not logged in"Test plan
Fixes #30674
🤖 Generated with Claude Code