Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the landing-page QR login flow by making QR expiry and polling failures behave more robustly, reducing stale async overwrites, and ensuring the QR widget refreshes correctly when the auth URL changes.
Changes:
- Add request-versioning to prevent stale async provisioning responses from overwriting newer QR codes.
- Auto-refresh QR codes on expiry and surface a retry UI after repeated polling failures.
- Force QR widget rebuilds on auth URL changes and add targeted cubit tests for the new behaviors.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
lib/ui/landing/bloc/landing_cubit.dart |
Adds injected loaders/streams for testability, request-versioning guards, auto-refresh + retry-state logic, and polling failure handling. |
lib/ui/landing/bloc/landing_state.dart |
Extends copyWith to support clearing/updating errorMessage. |
lib/ui/landing/landing_qrcode.dart |
Keys the QR widget by URL to force rebuild when the auth URL changes. |
test/ui/landing/bloc/landing_qrcode_cubit_test.dart |
Adds tests covering stale request protection, expiry refresh, and retry fallback behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ? 'Failed to refresh QR code: $error' | ||
| : 'Failed to request auth: $error', |
There was a problem hiding this comment.
The user-visible retry Tooltip error message is built from raw exceptions (e.g., "Failed to request auth: $error" / "Failed to refresh QR code: $error"). This can leak internal details and is not localized. Consider logging the detailed exception but emitting a localized, user-friendly message (optionally mapping known error types to specific copy).
| ? 'Failed to refresh QR code: $error' | |
| : 'Failed to request auth: $error', | |
| ? 'Failed to refresh QR code. Please try again.' | |
| : 'Failed to request authentication. Please try again.', |
| if (_pollingFailureCount >= pollingFailureLimit) { | ||
| _cancelPeriodicSubscription(); | ||
| w('landing qr polling failed, entering retry state'); | ||
| emit(state.needReload(_expiredMessageBuilder())); |
There was a problem hiding this comment.
When provisioning polling fails repeatedly, the cubit emits _expiredMessageBuilder() which defaults to Localization.current.qrCodeExpiredDesc. That message is misleading for network/provisioning failures. Consider using a dedicated (localized) message for polling failures, or passing an error-specific builder separate from the QR-expired copy.
| emit(state.needReload(_expiredMessageBuilder())); | |
| emit( | |
| state.needReload( | |
| 'Unable to refresh the QR code due to network or server issues. Please try again.', | |
| ), | |
| ); |
| final (acount, privateKey) = await _verify(secret, keyPair); | ||
| if (requestVersion != _requestVersion) return; | ||
| multiAuthChangeNotifier.signIn( | ||
| AuthState(account: acount, privateKey: privateKey), |
There was a problem hiding this comment.
Typo in destructuring assignment: acount should be account to avoid confusion and keep naming consistent with AuthState(account: ...) and the Account type.
| final (acount, privateKey) = await _verify(secret, keyPair); | |
| if (requestVersion != _requestVersion) return; | |
| multiAuthChangeNotifier.signIn( | |
| AuthState(account: acount, privateKey: privateKey), | |
| final (account, privateKey) = await _verify(secret, keyPair); | |
| if (requestVersion != _requestVersion) return; | |
| multiAuthChangeNotifier.signIn( | |
| AuthState(account: account, privateKey: privateKey), |
| if (secret.isEmpty) return; | ||
|
|
||
| _cancelPeriodicSubscription(); | ||
| if (requestVersion != _requestVersion) return; | ||
| emit(state.copyWith(status: LandingStatus.provisioning)); |
There was a problem hiding this comment.
Potential stale-request side effect: _cancelPeriodicSubscription() is called before the requestVersion != _requestVersion guard. If an older in-flight _checkLanding completes after a newer requestAuthUrl() has started, it can cancel the newer polling subscription before returning. Add a version/subscription guard immediately after the await (and before canceling/resetting state) so stale _checkLanding invocations become no-ops.
|
@claude[agent] revew the change |
What changed
Validation
flutter test test/ui/landing/bloc/landing_qrcode_cubit_test.dartflutter analyze lib/ui/landing/bloc/landing_cubit.dart lib/ui/landing/bloc/landing_state.dart lib/ui/landing/landing_qrcode.dart test/ui/landing/bloc/landing_qrcode_cubit_test.dart