-
Notifications
You must be signed in to change notification settings - Fork 39
Fix: Offline authentication and improved no internet handling #2847
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
base: staging
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds connectivity-aware behavior across auth flows and UI: auth methods now check connectivity and use cached tokens offline; connectivity bloc gains internet reachability checks, dismiss/recheck events and timer handling; offline state tracks reappearance; the no-internet banner becomes animated, state-aware and customizable. Logging updated. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as NoInternetBanner / Login UI
participant Bloc as ConnectivityBloc
participant Net as Connectivity + reachability
UI->>Bloc: ConnectivityCheckRequested
Bloc->>Net: checkConnectivity() + hasInternet()
Net-->>Bloc: result + reachable
alt reachable
Bloc-->>UI: ConnectivityOnline
else not reachable
Bloc-->>UI: ConnectivityOffline(reappeared?)
end
UI->>Bloc: ConnectivityBannerDismissed
Bloc->>Bloc: start 5m timer -> on timeout add ConnectivityCheckRequested
sequenceDiagram
participant Caller as App
participant Auth as AuthImpl
participant Conn as Connectivity
participant API as Auth API
Caller->>Auth: loginWithEmailAndPassword(credentials)
Auth->>Conn: checkConnectivity()
alt offline
Auth->>Hive: read cached token
alt token found
Auth-->>Caller: return cached token
else
Auth-->>Caller: throw offline exception
end
else online
Auth->>API: POST /login
alt 200
Auth->>Hive: save token, userId
Auth-->>Caller: return token
else
Auth-->>Caller: throw decoded server error
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 8
🔭 Outside diff range comments (2)
src/mobile-v3/lib/src/app/auth/repository/auth_repository.dart (1)
168-179: Email verification: avoid assert+null-assert; check token, add timeoutAsserts are stripped in release builds. Guard the token and avoid runtime crash; also set a timeout.
Apply this diff:
- loggy.info('AuthRepository: Verifying email code for $email'); - final apiToken = dotenv.env["AIRQO_MOBILE_TOKEN"]; - assert(apiToken != null, 'AIRQO_MOBILE_TOKEN missing in .env'); + loggy.info('AuthRepository: Verifying email code'); + final apiToken = dotenv.env["AIRQO_MOBILE_TOKEN"]; + if (apiToken == null || apiToken.isEmpty) { + loggy.error('AuthRepository: AIRQO_MOBILE_TOKEN is missing'); + throw Exception('Client misconfiguration: missing API token.'); + } @@ - final verifyResponse = await http.post( + final verifyResponse = await http + .post( Uri.parse("https://api.airqo.net/api/v2/users/verify-email/$token"), headers: { - "Authorization": apiToken!, + "Authorization": apiToken, "Content-Type": "application/json", "Accept": "application/json" }, body: jsonEncode({"email": email}), - ); + ) + .timeout(const Duration(seconds: 15));src/mobile-v3/lib/src/app/shared/bloc/connectivity_bloc.dart (1)
71-78: Add a timeout to the DNS lookup to avoid hanging reachability checksOn some networks, DNS can hang. Apply a short timeout to keep the bloc responsive.
Apply this diff:
Future<bool> _hasInternetConnection() async { try { - final result = await InternetAddress.lookup('google.com'); + final result = await InternetAddress + .lookup('google.com') + .timeout(const Duration(seconds: 3)); return result.isNotEmpty && result[0].rawAddress.isNotEmpty; } catch (_) { return false; } }
🧹 Nitpick comments (4)
src/mobile-v3/lib/src/app/shared/bloc/connectivity_event.dart (1)
19-20: Make zero-arg events const for better immutability and GC efficiencyMinor ergonomics: add a const constructor to zero-payload events so they can be instantiated as const and canonicalized.
Apply this diff:
-class ConnectivityCheckRequested extends ConnectivityEvent {} +class ConnectivityCheckRequested extends ConnectivityEvent { + const ConnectivityCheckRequested(); +}Additionally (outside the selected lines), consider the same for ConnectivityBannerDismissed:
class ConnectivityBannerDismissed extends ConnectivityEvent { const ConnectivityBannerDismissed(); }src/mobile-v3/lib/src/app/shared/bloc/connectivity_state.dart (1)
16-29: Good addition: reappeared flag + copyWith; consider const constructors for other statesThe reappeared flag is a clear, minimal extension and the Equatable props are accurate. Nit: for consistency and a tiny runtime win, make ConnectivityInitial and ConnectivityOnline have explicit const constructors so they can be emitted as const instances.
Example (outside selected lines):
class ConnectivityInitial extends ConnectivityState { const ConnectivityInitial(); } class ConnectivityOnline extends ConnectivityState { const ConnectivityOnline(); }src/mobile-v3/lib/src/app/auth/repository/auth_repository.dart (1)
30-35: Reduce PII in logs (emails/usernames)Usernames and emails are PII; prefer masking or omitting them in logs at info/error level.
Consider:
- Replace occurrences like “for $username/$email” with a generic message, or mask: joh***@domain.com.
- Keep full identifiers only at debug level if absolutely necessary.
I can generate a small helper like:
String maskEmail(String email) { final parts = email.split('@'); if (parts.length != 2 || parts[0].length < 3) return '***'; return '${parts[0].substring(0, 3)}***@${parts[1]}'; }…and update the log calls accordingly. Want me to apply this across the repo?
Also applies to: 41-42, 100-100, 168-168, 243-243
src/mobile-v3/lib/src/app/shared/pages/no_internet_banner.dart (1)
78-86: Use theme’s onError color for icons; keep colors consistent with the bannerHard-coding white can clash with custom themes. Use colorScheme.onError (already used for text), so icons remain legible across themes.
Apply this diff:
- IconButton( - icon: const Icon(Icons.refresh, color: Colors.white), + IconButton( + icon: Icon(Icons.refresh, color: Theme.of(context).colorScheme.onError), tooltip: 'Retry', @@ - IconButton( - icon: const Icon(Icons.close, color: Colors.white), + IconButton( + icon: Icon(Icons.close, color: Theme.of(context).colorScheme.onError), tooltip: 'Dismiss',Optional: The reappeared color is hard-coded (Color.fromARGB(255, 162, 51, 45)). Consider deriving it from Theme.of(context).colorScheme.errorContainer or a slightly darkened error to better support light/dark themes.
Also applies to: 87-97
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (5)
src/mobile-v3/lib/src/app/auth/repository/auth_repository.dart(4 hunks)src/mobile-v3/lib/src/app/shared/bloc/connectivity_bloc.dart(2 hunks)src/mobile-v3/lib/src/app/shared/bloc/connectivity_event.dart(1 hunks)src/mobile-v3/lib/src/app/shared/bloc/connectivity_state.dart(1 hunks)src/mobile-v3/lib/src/app/shared/pages/no_internet_banner.dart(1 hunks)
🔇 Additional comments (3)
src/mobile-v3/lib/src/app/shared/bloc/connectivity_bloc.dart (3)
21-26: Good: derive isConnected from connectivity + actual internet reachabilitySubscribing to onConnectivityChanged and verifying reachability reduces false positives from captive portals or local-only networks.
28-39: Reappearance logic is clear and minimalCorrectly sets reappeared when coming back to offline after a dismissed banner; state stays source-of-truth for the banner.
93-97: Nice cleanup: cancel both subscription and timerProper resource cleanup in close() avoids leaks.
| final Connectivity _connectivity = Connectivity(); | ||
|
|
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.
🛠️ Refactor suggestion
Inject Connectivity to improve testability and flexibility
Hard-coding Connectivity makes unit testing and mocking harder. Prefer DI with a default.
Apply this diff:
-class AuthImpl extends AuthRepository {
- final Connectivity _connectivity = Connectivity();
+class AuthImpl extends AuthRepository {
+ final Connectivity _connectivity;Then, add a constructor (outside the selected lines):
AuthImpl({Connectivity? connectivity}) : _connectivity = connectivity ?? Connectivity();🤖 Prompt for AI Agents
In src/mobile-v3/lib/src/app/auth/repository/auth_repository.dart around lines
23 to 24, the Connectivity instance is hard-coded which hinders testing; replace
the inline instantiation so the field is declared for injection (final
Connectivity _connectivity;) and add the requested constructor outside the
selected lines: AuthImpl({Connectivity? connectivity}) : _connectivity =
connectivity ?? Connectivity(); to allow dependency injection with a default
fallback.
| final connectivityResult = await _connectivity.checkConnectivity(); | ||
| if (connectivityResult == ConnectivityResult.none) { | ||
| loggy.info('AuthRepository: Offline, checking cached token'); | ||
| final cachedToken = | ||
| await HiveRepository.getData('token', HiveBoxNames.authBox); | ||
| if (cachedToken != null && cachedToken.isNotEmpty) { | ||
| loggy.info('AuthRepository: Returning cached token for $username'); | ||
| return cachedToken; // Use cached token when offline | ||
| } | ||
| loggy.error('AuthRepository: No cached token found while offline'); | ||
| throw Exception('Offline: Please try again when connected.'); | ||
| } |
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.
💡 Verification agent
🧩 Analysis chain
Offline login can return a token for a different user (impersonation risk)
When offline, the code returns any cached token regardless of the username entered. This can inadvertently log a different user in on a shared device. Tie the cached token to the username and save the username at successful login.
Apply this diff to validate username during offline login and to persist the username on successful login:
@@
- if (connectivityResult == ConnectivityResult.none) {
- loggy.info('AuthRepository: Offline, checking cached token');
- final cachedToken =
- await HiveRepository.getData('token', HiveBoxNames.authBox);
- if (cachedToken != null && cachedToken.isNotEmpty) {
- loggy.info('AuthRepository: Returning cached token for $username');
- return cachedToken; // Use cached token when offline
- }
+ if (connectivityResult == ConnectivityResult.none) {
+ loggy.info('AuthRepository: Offline, checking cached token');
+ final cachedToken =
+ await HiveRepository.getData('token', HiveBoxNames.authBox);
+ final cachedUsername =
+ await HiveRepository.getData('username', HiveBoxNames.authBox);
+ if (cachedToken != null &&
+ cachedToken.isNotEmpty &&
+ cachedUsername == username) {
+ loggy.info('AuthRepository: Returning cached token (offline)');
+ return cachedToken; // Use cached token when offline for the same user
+ }
loggy.error('AuthRepository: No cached token found while offline');
throw Exception('Offline: Please try again when connected.');
}
@@
- await HiveRepository.saveData(HiveBoxNames.authBox, "token", token);
- await HiveRepository.saveData(HiveBoxNames.authBox, "userId", userId);
+ await HiveRepository.saveData(HiveBoxNames.authBox, "token", token);
+ await HiveRepository.saveData(HiveBoxNames.authBox, "userId", userId);
+ await HiveRepository.saveData(HiveBoxNames.authBox, "username", username);Run this to confirm the HiveRepository API order for getData/saveData:
Also applies to: 61-64
🏁 Script executed:
#!/bin/bash
# Inspect HiveRepository method signatures/usages to ensure correct parameter order.
rg -n "class\s+HiveRepository|static\s+.*getData\s*\(|static\s+.*saveData\s*\(" -A 4 -B 2
echo "---- Usages ----"
rg -n "HiveRepository\.(getData|saveData)\(" -A 0 -B 0Length of output: 4458
Critical — bind offline cached token to the requested username
Offline flow currently returns any cached token regardless of the supplied username (impersonation risk). Confirmed HiveRepository API ordering: saveData(String boxName, String key, dynamic value) is declared at src/mobile-v3/lib/src/app/shared/repository/hive_repository.dart:29, and getData is used throughout as getData(key, boxName). The proposed change is correct and should be applied.
Files to change:
- src/mobile-v3/lib/src/app/auth/repository/auth_repository.dart — offline token retrieval (lines ~28–39)
- src/mobile-v3/lib/src/app/auth/repository/auth_repository.dart — persist username on successful login (lines ~61–64)
Apply this diff:
@@
- if (connectivityResult == ConnectivityResult.none) {
- loggy.info('AuthRepository: Offline, checking cached token');
- final cachedToken =
- await HiveRepository.getData('token', HiveBoxNames.authBox);
- if (cachedToken != null && cachedToken.isNotEmpty) {
- loggy.info('AuthRepository: Returning cached token for $username');
- return cachedToken; // Use cached token when offline
- }
+ if (connectivityResult == ConnectivityResult.none) {
+ loggy.info('AuthRepository: Offline, checking cached token');
+ final cachedToken =
+ await HiveRepository.getData('token', HiveBoxNames.authBox);
+ final cachedUsername =
+ await HiveRepository.getData('username', HiveBoxNames.authBox);
+ if (cachedToken != null &&
+ cachedToken.isNotEmpty &&
+ cachedUsername == username) {
+ loggy.info('AuthRepository: Returning cached token (offline)');
+ return cachedToken; // Use cached token when offline for the same user
+ }
loggy.error('AuthRepository: No cached token found while offline');
throw Exception('Offline: Please try again when connected.');
}
@@
- await HiveRepository.saveData(HiveBoxNames.authBox, "token", token);
- await HiveRepository.saveData(HiveBoxNames.authBox, "userId", userId);
+ await HiveRepository.saveData(HiveBoxNames.authBox, "token", token);
+ await HiveRepository.saveData(HiveBoxNames.authBox, "userId", userId);
+ await HiveRepository.saveData(HiveBoxNames.authBox, "username", username);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final connectivityResult = await _connectivity.checkConnectivity(); | |
| if (connectivityResult == ConnectivityResult.none) { | |
| loggy.info('AuthRepository: Offline, checking cached token'); | |
| final cachedToken = | |
| await HiveRepository.getData('token', HiveBoxNames.authBox); | |
| if (cachedToken != null && cachedToken.isNotEmpty) { | |
| loggy.info('AuthRepository: Returning cached token for $username'); | |
| return cachedToken; // Use cached token when offline | |
| } | |
| loggy.error('AuthRepository: No cached token found while offline'); | |
| throw Exception('Offline: Please try again when connected.'); | |
| } | |
| final connectivityResult = await _connectivity.checkConnectivity(); | |
| if (connectivityResult == ConnectivityResult.none) { | |
| loggy.info('AuthRepository: Offline, checking cached token'); | |
| final cachedToken = | |
| await HiveRepository.getData('token', HiveBoxNames.authBox); | |
| final cachedUsername = | |
| await HiveRepository.getData('username', HiveBoxNames.authBox); | |
| if (cachedToken != null && | |
| cachedToken.isNotEmpty && | |
| cachedUsername == username) { | |
| loggy.info('AuthRepository: Returning cached token (offline)'); | |
| return cachedToken; // Use cached token when offline for the same user | |
| } | |
| loggy.error('AuthRepository: No cached token found while offline'); | |
| throw Exception('Offline: Please try again when connected.'); | |
| } | |
| await HiveRepository.saveData(HiveBoxNames.authBox, "token", token); | |
| await HiveRepository.saveData(HiveBoxNames.authBox, "userId", userId); | |
| await HiveRepository.saveData(HiveBoxNames.authBox, "username", username); |
| loggy.info('AuthRepository: Attempting login for $username'); | ||
| final loginResponse = await http.post( | ||
| Uri.parse("https://api.airqo.net/api/v2/users/loginUser"), | ||
| body: jsonEncode({"userName": username, "password": password}), | ||
| headers: { | ||
| "Authorization": dotenv.env["AIRQO_MOBILE_TOKEN"]!, | ||
| "Accept": "*/*", | ||
| "Content-Type": "application/json" | ||
| }, | ||
| ); | ||
|
|
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.
🛠️ Refactor suggestion
Avoid null-assert on AIRQO_MOBILE_TOKEN; fail fast with a clear message and add a request timeout
Using dotenv.env["AIRQO_MOBILE_TOKEN"]! will crash at runtime if missing. Guard it and apply a network timeout to prevent hangs.
Apply this diff:
- loggy.info('AuthRepository: Attempting login for $username');
- final loginResponse = await http.post(
+ loggy.info('AuthRepository: Attempting login');
+ final apiToken = dotenv.env["AIRQO_MOBILE_TOKEN"];
+ if (apiToken == null || apiToken.isEmpty) {
+ loggy.error('AuthRepository: AIRQO_MOBILE_TOKEN is missing');
+ throw Exception('Client misconfiguration: missing API token.');
+ }
+ final loginResponse = await http
+ .post(
Uri.parse("https://api.airqo.net/api/v2/users/loginUser"),
body: jsonEncode({"userName": username, "password": password}),
headers: {
- "Authorization": dotenv.env["AIRQO_MOBILE_TOKEN"]!,
+ "Authorization": apiToken,
"Accept": "*/*",
"Content-Type": "application/json"
},
- );
+ )
+ .timeout(const Duration(seconds: 15));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| loggy.info('AuthRepository: Attempting login for $username'); | |
| final loginResponse = await http.post( | |
| Uri.parse("https://api.airqo.net/api/v2/users/loginUser"), | |
| body: jsonEncode({"userName": username, "password": password}), | |
| headers: { | |
| "Authorization": dotenv.env["AIRQO_MOBILE_TOKEN"]!, | |
| "Accept": "*/*", | |
| "Content-Type": "application/json" | |
| }, | |
| ); | |
| loggy.info('AuthRepository: Attempting login'); | |
| final apiToken = dotenv.env["AIRQO_MOBILE_TOKEN"]; | |
| if (apiToken == null || apiToken.isEmpty) { | |
| loggy.error('AuthRepository: AIRQO_MOBILE_TOKEN is missing'); | |
| throw Exception('Client misconfiguration: missing API token.'); | |
| } | |
| final loginResponse = await http | |
| .post( | |
| Uri.parse("https://api.airqo.net/api/v2/users/loginUser"), | |
| body: jsonEncode({"userName": username, "password": password}), | |
| headers: { | |
| "Authorization": apiToken, | |
| "Accept": "*/*", | |
| "Content-Type": "application/json" | |
| }, | |
| ) | |
| .timeout(const Duration(seconds: 15)); |
🤖 Prompt for AI Agents
In src/mobile-v3/lib/src/app/auth/repository/auth_repository.dart around lines
41 to 51, the code uses dotenv.env["AIRQO_MOBILE_TOKEN"]! which can crash if the
env var is missing and the http.post has no timeout; change this to read the
token into a local final token = dotenv.env["AIRQO_MOBILE_TOKEN"]; guard it with
an if (token == null || token.isEmpty) { loggy.error(...) and throw or return a
clear AuthConfigurationException } so we fail fast with a descriptive message,
then pass token (non-null) into the headers and apply a request timeout to the
POST (e.g. use http.post(...).timeout(const Duration(seconds: X)) and handle
TimeoutException to return/log an appropriate error/response.
| final data = json.decode(loginResponse.body); | ||
|
|
||
| if (loginResponse.statusCode != 200) { | ||
| loggy.error('AuthRepository: Login failed - ${data['message']}'); | ||
| throw Exception(data['message']); | ||
| } else { | ||
| String userId = data["_id"]; | ||
| String token = data["token"]; | ||
|
|
||
| HiveRepository.saveData(HiveBoxNames.authBox, "token", token); | ||
| HiveRepository.saveData(HiveBoxNames.authBox, "userId", userId); | ||
| } | ||
|
|
||
| return data["token"]; | ||
| final String userId = data["_id"]; | ||
| final String token = data["token"]; | ||
| await HiveRepository.saveData(HiveBoxNames.authBox, "token", token); | ||
| await HiveRepository.saveData(HiveBoxNames.authBox, "userId", userId); | ||
| loggy.info('AuthRepository: Login successful, token saved'); | ||
| return token; | ||
| } |
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.
🛠️ Refactor suggestion
Defensive JSON parsing for login response
json.decode() can throw on invalid or empty bodies; handle parse failures and provide a user-friendly error.
Apply this diff:
- final data = json.decode(loginResponse.body);
+ late final Map<String, dynamic> data;
+ try {
+ final decoded = json.decode(loginResponse.body);
+ data = decoded is Map<String, dynamic> ? decoded : <String, dynamic>{};
+ } catch (e) {
+ loggy.error('AuthRepository: Unable to parse login response - $e');
+ throw Exception('Unexpected server response. Please try again.');
+ }
@@
- final String userId = data["_id"];
- final String token = data["token"];
+ final String userId = (data["_id"] ?? '') as String;
+ final String token = (data["token"] ?? '') as String;
+ if (userId.isEmpty || token.isEmpty) {
+ loggy.error('AuthRepository: Missing token or userId in login response');
+ throw Exception('Login failed: invalid server response.');
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final data = json.decode(loginResponse.body); | |
| if (loginResponse.statusCode != 200) { | |
| loggy.error('AuthRepository: Login failed - ${data['message']}'); | |
| throw Exception(data['message']); | |
| } else { | |
| String userId = data["_id"]; | |
| String token = data["token"]; | |
| HiveRepository.saveData(HiveBoxNames.authBox, "token", token); | |
| HiveRepository.saveData(HiveBoxNames.authBox, "userId", userId); | |
| } | |
| return data["token"]; | |
| final String userId = data["_id"]; | |
| final String token = data["token"]; | |
| await HiveRepository.saveData(HiveBoxNames.authBox, "token", token); | |
| await HiveRepository.saveData(HiveBoxNames.authBox, "userId", userId); | |
| loggy.info('AuthRepository: Login successful, token saved'); | |
| return token; | |
| } | |
| late final Map<String, dynamic> data; | |
| try { | |
| final decoded = json.decode(loginResponse.body); | |
| data = decoded is Map<String, dynamic> ? decoded : <String, dynamic>{}; | |
| } catch (e) { | |
| loggy.error('AuthRepository: Unable to parse login response - $e'); | |
| throw Exception('Unexpected server response. Please try again.'); | |
| } | |
| if (loginResponse.statusCode != 200) { | |
| loggy.error('AuthRepository: Login failed - ${data['message']}'); | |
| throw Exception(data['message']); | |
| } | |
| final String userId = (data["_id"] ?? '') as String; | |
| final String token = (data["token"] ?? '') as String; | |
| if (userId.isEmpty || token.isEmpty) { | |
| loggy.error('AuthRepository: Missing token or userId in login response'); | |
| throw Exception('Login failed: invalid server response.'); | |
| } | |
| await HiveRepository.saveData(HiveBoxNames.authBox, "token", token); | |
| await HiveRepository.saveData(HiveBoxNames.authBox, "userId", userId); | |
| loggy.info('AuthRepository: Login successful, token saved'); | |
| return token; | |
| } |
🤖 Prompt for AI Agents
In src/mobile-v3/lib/src/app/auth/repository/auth_repository.dart around lines
52 to 65, json.decode(loginResponse.body) can throw on empty or invalid JSON;
wrap the decode in a try/catch and validate the body first (e.g., check
loginResponse.body.isNotEmpty), attempt to parse inside the try, and on
FormatException or any parsing error log a clear, user-friendly error and throw
a descriptive Exception (preserving statusCode/message when available). Ensure
the subsequent checks (statusCode != 200) use the safely parsed data only if
parsing succeeded, and handle the case where parsing failed by constructing a
sensible error message to log and throw.
| final connectivityResult = await _connectivity.checkConnectivity(); | ||
| if (connectivityResult == ConnectivityResult.none) { | ||
| loggy.error('AuthRepository: Offline, cannot register'); | ||
| throw Exception('Offline: Registration requires internet.'); | ||
| } | ||
|
|
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.
🛠️ Refactor suggestion
Registration: add timeout, robust JSON parsing, and safer error extraction
Harden the network call and parsing. Also handle when errors is a list/string/object.
Apply this diff:
- loggy.info('AuthRepository: Attempting registration for ${model.email}');
- final registerResponse = await http.post(
+ loggy.info('AuthRepository: Attempting registration');
+ final registerResponse = await http
+ .post(
Uri.parse("https://api.airqo.net/api/v2/users/register"),
body: registerInputModelToJson(model),
headers: {"Accept": "*/*", "Content-Type": "application/json"},
- );
+ )
+ .timeout(const Duration(seconds: 15));
@@
- final data = json.decode(registerResponse.body);
+ late final Map<String, dynamic> data;
+ try {
+ final decoded = json.decode(registerResponse.body);
+ data = decoded is Map<String, dynamic> ? decoded : <String, dynamic>{};
+ } catch (e) {
+ loggy.error('AuthRepository: Unable to parse registration response - $e');
+ throw Exception('Registration failed. Please try again.');
+ }
@@
- final errorMessage = data['errors']?['message'] ?? data['message'];
+ final errs = data['errors'];
+ String errorMessage = data['message']?.toString() ?? 'Registration failed.';
+ if (errs != null) {
+ if (errs is Map) {
+ errorMessage = (errs.values).join(', ');
+ } else if (errs is List) {
+ errorMessage = errs.join(', ');
+ } else {
+ errorMessage = errs.toString();
+ }
+ }Also applies to: 76-81, 82-90
🤖 Prompt for AI Agents
In src/mobile-v3/lib/src/app/auth/repository/auth_repository.dart around lines
69-74 (also apply same changes at 76-81 and 82-90): the registration network
call and error handling need hardening — add a request timeout to the HTTP call
(e.g., wrap the future with .timeout and handle TimeoutException), parse
response body JSON defensively using try/catch and check for null/malformed
JSON, and normalize the "errors" field before logging/throwing so it can handle
when errors is a string, list, or map (e.g., if list join with commas, if map
extract values or known keys, else fallback to toString()). Ensure you extract
HTTP status codes and fallback error messages safely, and throw meaningful
Exceptions with the normalized message for upstream handling.
| loggy.info('AuthRepository: Requesting password reset for $email'); | ||
| final response = await http.post( | ||
| Uri.parse('https://api.airqo.net/api/v2/users/reset-password-request'), | ||
| headers: { | ||
| "Authorization": dotenv.env["AIRQO_MOBILE_TOKEN"]!, | ||
| "Accept": "*/*", | ||
| "Content-Type": "application/json" | ||
| }, | ||
| body: jsonEncode({ | ||
| 'email': email, | ||
| }), | ||
| body: jsonEncode({'email': email}), | ||
| ); |
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.
🛠️ Refactor suggestion
Password reset request: guard API token and add timeout
Same token safety as login; also set a timeout.
Apply this diff:
- loggy.info('AuthRepository: Requesting password reset for $email');
- final response = await http.post(
+ loggy.info('AuthRepository: Requesting password reset');
+ final apiToken = dotenv.env["AIRQO_MOBILE_TOKEN"];
+ if (apiToken == null || apiToken.isEmpty) {
+ loggy.error('AuthRepository: AIRQO_MOBILE_TOKEN is missing');
+ throw Exception('Client misconfiguration: missing API token.');
+ }
+ final response = await http
+ .post(
Uri.parse('https://api.airqo.net/api/v2/users/reset-password-request'),
headers: {
- "Authorization": dotenv.env["AIRQO_MOBILE_TOKEN"]!,
+ "Authorization": apiToken,
"Accept": "*/*",
"Content-Type": "application/json"
},
body: jsonEncode({'email': email}),
- );
+ )
+ .timeout(const Duration(seconds: 15));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| loggy.info('AuthRepository: Requesting password reset for $email'); | |
| final response = await http.post( | |
| Uri.parse('https://api.airqo.net/api/v2/users/reset-password-request'), | |
| headers: { | |
| "Authorization": dotenv.env["AIRQO_MOBILE_TOKEN"]!, | |
| "Accept": "*/*", | |
| "Content-Type": "application/json" | |
| }, | |
| body: jsonEncode({ | |
| 'email': email, | |
| }), | |
| body: jsonEncode({'email': email}), | |
| ); | |
| loggy.info('AuthRepository: Requesting password reset'); | |
| final apiToken = dotenv.env["AIRQO_MOBILE_TOKEN"]; | |
| if (apiToken == null || apiToken.isEmpty) { | |
| loggy.error('AuthRepository: AIRQO_MOBILE_TOKEN is missing'); | |
| throw Exception('Client misconfiguration: missing API token.'); | |
| } | |
| final response = await http | |
| .post( | |
| Uri.parse('https://api.airqo.net/api/v2/users/reset-password-request'), | |
| headers: { | |
| "Authorization": apiToken, | |
| "Accept": "*/*", | |
| "Content-Type": "application/json" | |
| }, | |
| body: jsonEncode({'email': email}), | |
| ) | |
| .timeout(const Duration(seconds: 15)); |
🤖 Prompt for AI Agents
In src/mobile-v3/lib/src/app/auth/repository/auth_repository.dart around lines
100 to 109, the password reset request currently uses the AIRQO_MOBILE_TOKEN
without guarding for null and makes an HTTP call without a timeout; update the
code to first read the token into a local variable and throw or return an error
if the env var is null or empty (do not assume non-null with !), then pass that
token in the Authorization header, and call http.post with a timeout (e.g.,
using .timeout with an appropriate Duration) to avoid hanging requests; ensure
any TimeoutException or missing-token error is handled/logged appropriately.
| if (response.statusCode == 200) { | ||
| final data = jsonDecode(response.body); | ||
| loggy.info('AuthRepository: Password reset successful'); | ||
| return data['message'] ?? 'Password reset successful.'; | ||
| } else { | ||
| final error = | ||
| jsonDecode(response.body)['message'] ?? 'Failed to reset password.'; | ||
| throw Exception(error); | ||
| } | ||
|
|
||
| final error = | ||
| jsonDecode(response.body)['message'] ?? 'Failed to reset password.'; | ||
| loggy.error('AuthRepository: Password reset failed - $error'); | ||
| throw Exception(error); | ||
| } |
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.
🛠️ Refactor suggestion
Update password: add timeout and safer error parsing
jsonDecode may throw; add timeout and fallback error message if parsing fails.
Apply this diff:
- final response = await http.post(
+ final response = await http
+ .post(
Uri.parse('https://api.airqo.net/api/v2/users/reset-password/$token'),
headers: {
'Content-Type': "application/json",
},
body: jsonEncode({
'password': password,
'confirmPassword': confirmPassword,
}),
- );
+ )
+ .timeout(const Duration(seconds: 15));
@@
- final error =
- jsonDecode(response.body)['message'] ?? 'Failed to reset password.';
+ String error = 'Failed to reset password.';
+ try {
+ final decoded = jsonDecode(response.body);
+ if (decoded is Map && decoded['message'] != null) {
+ error = decoded['message'].toString();
+ }
+ } catch (_) {}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (response.statusCode == 200) { | |
| final data = jsonDecode(response.body); | |
| loggy.info('AuthRepository: Password reset successful'); | |
| return data['message'] ?? 'Password reset successful.'; | |
| } else { | |
| final error = | |
| jsonDecode(response.body)['message'] ?? 'Failed to reset password.'; | |
| throw Exception(error); | |
| } | |
| final error = | |
| jsonDecode(response.body)['message'] ?? 'Failed to reset password.'; | |
| loggy.error('AuthRepository: Password reset failed - $error'); | |
| throw Exception(error); | |
| } | |
| if (response.statusCode == 200) { | |
| final data = jsonDecode(response.body); | |
| loggy.info('AuthRepository: Password reset successful'); | |
| return data['message'] ?? 'Password reset successful.'; | |
| } | |
| String error = 'Failed to reset password.'; | |
| try { | |
| final decoded = jsonDecode(response.body); | |
| if (decoded is Map && decoded['message'] != null) { | |
| error = decoded['message'].toString(); | |
| } | |
| } catch (_) {} | |
| loggy.error('AuthRepository: Password reset failed - $error'); | |
| throw Exception(error); | |
| } |
🤖 Prompt for AI Agents
In src/mobile-v3/lib/src/app/auth/repository/auth_repository.dart around lines
229-239, the code uses jsonDecode without handling decode errors and also lacks
a request timeout; update the HTTP call that produces this response to apply a
timeout (e.g., .timeout with a reasonable Duration) and in this block wrap
jsonDecode in try/catch for both success and error branches — on success try to
extract data['message'] but if parsing fails fall back to a generic success
message or the raw response.body, and on non-200 parse the error message inside
try/catch and fall back to a default 'Failed to reset password.' (or the raw
body) before logging with loggy.error and throwing the Exception.
| on<ConnectivityCheckRequested>((event, emit) async { | ||
| var connectivityResult = await _connectivity.checkConnectivity(); | ||
| logDebug('Connectivity check result: $connectivityResult'); | ||
| bool isConnected = connectivityResult != ConnectivityResult.none && | ||
| await _hasInternetConnection(); | ||
| if (isConnected) { | ||
| logInfo('Emitting ConnectivityOnline'); | ||
| emit(ConnectivityOnline()); | ||
| } else { | ||
| emit(ConnectivityOffline()); | ||
| final reappeared = state is ConnectivityOffline && | ||
| (state as ConnectivityOffline).isDismissed; | ||
| logInfo( | ||
| 'Emitting ConnectivityOffline(isDismissed: false, reappeared: $reappeared)'); | ||
| emit(ConnectivityOffline(isDismissed: false, reappeared: reappeared)); | ||
| } | ||
| }); |
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.
🛠️ Refactor suggestion
Debounce/droppable transformer for ConnectivityCheckRequested to avoid concurrent checks
If a user taps Retry rapidly, multiple checks can run concurrently. Use a droppable or restartable transformer to limit concurrency.
Apply this diff:
- on<ConnectivityCheckRequested>((event, emit) async {
+ on<ConnectivityCheckRequested>((event, emit) async {
var connectivityResult = await _connectivity.checkConnectivity();
logDebug('Connectivity check result: $connectivityResult');
bool isConnected = connectivityResult != ConnectivityResult.none &&
await _hasInternetConnection();
if (isConnected) {
logInfo('Emitting ConnectivityOnline');
emit(ConnectivityOnline());
} else {
final reappeared = state is ConnectivityOffline &&
(state as ConnectivityOffline).isDismissed;
logInfo(
'Emitting ConnectivityOffline(isDismissed: false, reappeared: $reappeared)');
emit(ConnectivityOffline(isDismissed: false, reappeared: reappeared));
}
- });
+ }, transformer: droppable());And add the import (outside selected lines):
import 'package:bloc_concurrency/bloc_concurrency.dart';Alternatively, use restartable() if you want to always use the latest tap and cancel in-flight checks.
🤖 Prompt for AI Agents
In src/mobile-v3/lib/src/app/shared/bloc/connectivity_bloc.dart around lines 41
to 56, the ConnectivityCheckRequested handler can run multiple concurrent checks
if the user taps Retry rapidly; change the on<ConnectivityCheckRequested>
registration to use a bloc_concurrency transformer (e.g., transformer:
droppable() to drop new events while one is running, or transformer:
restartable() to cancel in-flight checks and use the latest) and add the import
line import 'package:bloc_concurrency/bloc_concurrency.dart'; at the top of the
file (outside the selected lines); keep the handler body the same but pass the
chosen transformer as the on(...) parameter to limit concurrency.
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.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/mobile-v3/lib/src/app/auth/pages/login_page.dart (7)
42-57: Small UX polish in _login: dismiss keyboard before dispatchConsider unfocusing the text fields before triggering login to avoid the keyboard lingering over the loading UI.
Apply this diff:
final currentForm = formKey.currentState; if (currentForm != null && currentForm.validate()) { + // Dismiss keyboard for a cleaner transition + FocusScope.of(context).unfocus(); setState(() { _isLoading = true; error = null; }); authBloc.add(LoginUser( emailController.text.trim(), passwordController.text.trim(), )); }
68-75: Schedule navigation after frame to avoid context edge casesNavigating directly inside a bloc listener is usually fine, but scheduling it post-frame avoids occasional “called during build/deactivated widget” edge cases.
Apply this diff:
- if (state is AuthLoaded) { - Navigator.of(context).pushAndRemoveUntil( - MaterialPageRoute( - builder: (context) => NavPage(), - ), - (_) => false, - ); - } else if (state is EmailUnverifiedError) { + if (state is AuthLoaded) { + WidgetsBinding.instance.addPostFrameCallback((_) { + if (!mounted) return; + Navigator.of(context).pushAndRemoveUntil( + MaterialPageRoute(builder: (context) => NavPage()), + (_) => false, + ); + }); + } else if (state is EmailUnverifiedError) {
75-147: Guard against duplicate “Email Verification Required” dialogsIf the bloc emits
EmailUnverifiedErrormore than once, multiple dialogs can stack. Add a simple guard and ensure the flag resets when the dialog closes.Apply this diff to the listener:
- } else if (state is EmailUnverifiedError) { - setState(() { - error = state.message; - _isLoading = false; - }); - - showDialog( + } else if (state is EmailUnverifiedError) { + if (_verificationDialogOpen) return; + setState(() { + error = state.message; + _isLoading = false; + _verificationDialogOpen = true; + }); + + showDialog( context: context, barrierDismissible: false, builder: (context) => AlertDialog( @@ - TextButton( - onPressed: () => Navigator.pop(context), + TextButton( + onPressed: () { + _verificationDialogOpen = false; + Navigator.pop(context); + }, child: Text( 'Cancel', style: TextStyle( color: Colors.grey[600], ), ), ), ElevatedButton( @@ - onPressed: () { - Navigator.pop(context); - Navigator.push( + onPressed: () { + _verificationDialogOpen = false; + Navigator.pop(context); + Navigator.push( context, MaterialPageRoute( builder: (context) => EmailVerificationScreen( email: state.email, ), ), ); }, child: Text('Verify Now'), ), ], shape: RoundedRectangleBorder( borderRadius: BorderRadius.circular(12), ), ), - ); + ).then((_) { + if (mounted) { + setState(() { + _verificationDialogOpen = false; + }); + } + });Add this field to the state class (outside the shown range):
// Inside _LoginPageState bool _verificationDialogOpen = false;
155-167: SnackBar UX: hide existing before showing offline; consider floating styleTo avoid SnackBar stacking and improve visibility, hide the current one before showing the offline message and use floating style.
Apply this diff:
- if (state is ConnectivityOffline) { - ScaffoldMessenger.of(context).showSnackBar( - SnackBar( - content: Text( - 'No Internet Connection. Please check your network.'), - duration: Duration(seconds: 3), - ), - ); - } + if (state is ConnectivityOffline) { + final messenger = ScaffoldMessenger.of(context); + messenger.hideCurrentSnackBar(); + messenger.showSnackBar( + SnackBar( + content: const Text( + 'No Internet Connection. Please check your network.', + ), + duration: const Duration(seconds: 3), + behavior: SnackBarBehavior.floating, + ), + ); + }If offline login cannot proceed (no cached creds), consider resetting
_isLoadinghere to avoid a stuck spinner. If offline login is supported, ignore this suggestion.
221-223: Clear stale error state on input changeCurrently
onChangedis a no-op, so the red error persists after the user edits inputs. Clear it when the user types.Apply this diff:
- controller: emailController, - onChanged: (value) {}, + controller: emailController, + onChanged: (_) { + if (error != null) { + setState(() => error = null); + } + }, @@ - controller: passwordController, - onChanged: (value) {}, + controller: passwordController, + onChanged: (_) { + if (error != null) { + setState(() => error = null); + } + },Also applies to: 255-257
246-254: Consistency: mark password as required or remove asterisk from emailEmail uses “Email*” while password label omits the asterisk, yet it’s required per validator. Align the labels for consistency.
Apply this diff:
- label: "Password", + label: "Password*",
148-152: User-facing errors: consider mapping to friendly copy
state.messagemay contain backend/transport phrasing. Consider mapping common auth errors (invalid credentials, network issues, server errors) to user-friendly messages before showing them.Would you like a small helper (e.g.,
String humanizeAuthError(String raw)) added to the page or bloc to normalize messages?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/mobile-v3/lib/src/app/auth/pages/login_page.dart(11 hunks)
🔇 Additional comments (1)
src/mobile-v3/lib/src/app/auth/pages/login_page.dart (1)
11-11: Connectivity import: verify barrel exports and provider availabilityLooks good. Please confirm:
ConnectivityBlocre-exports its state types (e.g.,ConnectivityOffline). If not, import the specific state file.ConnectivityBlocis provided above this page in the widget tree; otherwise the listener won’t find an instance.
Summary of Changes (What does this PR do?)
Status of maturity (all need to be checked before merging):
How should this be manually tested?
What are the relevant tickets?
Screenshots (optional)
Summary by CodeRabbit
New Features
Improvements