fix: prevent LateInitializationError crashes in AuthServiceImpl#1048
Open
vibhutomer wants to merge 1 commit into
Open
fix: prevent LateInitializationError crashes in AuthServiceImpl#1048vibhutomer wants to merge 1 commit into
vibhutomer wants to merge 1 commit into
Conversation
…ects Signed-off-by: vibhutomer <vibhutomer25@gmail.com>
WalkthroughService method return types are relaxed from non-nullable to nullable across authentication and machine key operations. Interface contracts in the platform SPI layer are updated, followed by matching implementations that now safely return ChangesNullable Service Return Types
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
lib/platform_android/auth_service_impl.dart (6)
64-76:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical:
latekeyword still present—LateInitializationError risk remains.The
logout()method still useslate String logoutResponse;on line 66. If either catch block is executed,logoutResponseremains uninitialized, and returning it will throw a LateInitializationError—the exact bug this PR aims to fix. According to the PR objectives, this method should initialize with a safe default (empty string).🐛 Proposed fix: Initialize with empty string
Future<String> logout() async { - late String logoutResponse; + String logoutResponse = ""; try { logoutResponse = await AuthResponseApi().logout(); - }on PlatformException { + } on PlatformException { debugPrint('Logout Api call failed!'); - }catch (e) { + } catch (e) { debugPrint(e.toString()); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/platform_android/auth_service_impl.dart` around lines 64 - 76, The logout() method declares a late String logoutResponse and returns it even when an exception occurs; initialize logoutResponse to a safe default (e.g., empty string) at declaration so it is always defined, keep the try/catch using AuthResponseApi().logout() as-is, and ensure any error paths still return the initialized value from logout().
117-128:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical:
latekeyword still present—LateInitializationError risk remains.The
getAutoLogoutPopupTimeout()method still useslate String refreshLoginTime;on line 119. If either catch block is executed, the variable remains uninitialized, causing a LateInitializationError when returned.🐛 Proposed fix: Initialize with empty string
Future<String> getAutoLogoutPopupTimeout() async { - late String refreshLoginTime; + String refreshLoginTime = ""; try { refreshLoginTime = await AuthResponseApi().getAutoLogoutPopupTimeout();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/platform_android/auth_service_impl.dart` around lines 117 - 128, The getAutoLogoutPopupTimeout method uses a late String refreshLoginTime which may remain uninitialized if an exception is thrown; initialize refreshLoginTime with a safe default (e.g., empty string or a sensible fallback) when declaring it, or return the fallback directly from the catch blocks so refreshLoginTime is never returned uninitialized; locate getAutoLogoutPopupTimeout and the refreshLoginTime variable and update the declaration/exception handling accordingly.
104-115:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical:
latekeyword still present—LateInitializationError risk remains.The
getIdleTime()method still useslate String idleTime;on line 106. If either catch block is executed, the variable remains uninitialized, causing a LateInitializationError when returned.🐛 Proposed fix: Initialize with empty string
Future<String> getIdleTime() async { - late String idleTime; + String idleTime = ""; try { idleTime = await AuthResponseApi().getIdleTime();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/platform_android/auth_service_impl.dart` around lines 104 - 115, The getIdleTime method uses a late String (idleTime) which can remain uninitialized if an exception occurs; replace the late variable with a safely initialized default (e.g., empty string) or return a default directly in the catch blocks so getIdleTime() never returns an uninitialized value—update the AuthResponseApi().getIdleTime() call handling in getIdleTime to assign a default on exceptions and ensure the method always returns a valid String.
78-89:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical:
latekeyword still present—LateInitializationError risk remains.The
stopAlarmService()method still useslate String stopAlarmServiceResponse;on line 80. If either catch block is executed, the variable remains uninitialized, causing a LateInitializationError when returned—the exact issue this PR is meant to resolve.🐛 Proposed fix: Initialize with empty string
Future<String> stopAlarmService() async{ - late String stopAlarmServiceResponse; + String stopAlarmServiceResponse = ""; try { stopAlarmServiceResponse = await AuthResponseApi().stopAlarmService(); - }on PlatformException { + } on PlatformException { debugPrint('stopAlarmService Api call failed!'); - }catch (e) { + } catch (e) { debugPrint(e.toString()); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/platform_android/auth_service_impl.dart` around lines 78 - 89, The method stopAlarmService currently declares late String stopAlarmServiceResponse which can remain uninitialized if an exception occurs; replace the late declaration by initializing the variable (e.g., String stopAlarmServiceResponse = '' ) or make it nullable (String? stopAlarmServiceResponse) and return a safe default so the return never throws; update references in stopAlarmService and keep the await call to AuthResponseApi().stopAlarmService() and the existing catch blocks intact.
91-102:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical:
latekeyword still present—LateInitializationError risk remains.The
forgotPasswordUrl()method still useslate String forgotPasswordResponse;on line 93. If either catch block is executed, the variable remains uninitialized, causing a LateInitializationError when returned.🐛 Proposed fix: Initialize with empty string
Future<String> forgotPasswordUrl() async{ - late String forgotPasswordResponse; + String forgotPasswordResponse = ""; try { forgotPasswordResponse = await AuthResponseApi().forgotPasswordUrl();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/platform_android/auth_service_impl.dart` around lines 91 - 102, forgotPasswordUrl() declares late String forgotPasswordResponse which can remain uninitialized if a catch runs; initialize the variable (e.g., String forgotPasswordResponse = ''; ) or return a safe default from the catch blocks instead so you never return an uninitialized value. Update the forgotPasswordUrl method to set a default response before the try or to return a default in the PlatformException / catch blocks when calling AuthResponseApi().forgotPasswordUrl().
143-154:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical:
latekeyword still present—LateInitializationError risk remains.The
getPasswordLength()method still useslate String passwordLength;on line 145. If either catch block is executed, the variable remains uninitialized, causing a LateInitializationError when returned.🐛 Proposed fix: Initialize with empty string
Future<String> getPasswordLength() async { - late String passwordLength; + String passwordLength = ""; try { passwordLength = await AuthResponseApi().getPasswordLength();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/platform_android/auth_service_impl.dart` around lines 143 - 154, getPasswordLength uses a `late String passwordLength` which can remain uninitialized if an exception is caught, causing a LateInitializationError; change this by initializing passwordLength with a safe default (e.g., `String passwordLength = ""`) or by returning a default value directly from the catch blocks, and ensure the call to AuthResponseApi().getPasswordLength() is awaited into that initialized variable so getPasswordLength always returns a defined String.
🧹 Nitpick comments (4)
lib/platform_android/auth_service_impl.dart (3)
50-51: ⚡ Quick winRemove commented-out code.
The commented-out line showing the old
late PacketAuth packetAuth;should be deleted.🧹 Proposed cleanup
String username, String password) async { - // late PacketAuth packetAuth; PacketAuth? packetAuth;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/platform_android/auth_service_impl.dart` around lines 50 - 51, Remove the leftover commented-out declaration "// late PacketAuth packetAuth;" and keep only the active nullable declaration "PacketAuth? packetAuth;" so the file no longer contains dead/commented code; update any nearby comments if needed to reflect that packetAuth is now nullable.
34-35: ⚡ Quick winRemove commented-out code.
The commented-out line showing the old
late AuthResponse authResponse;should be deleted.🧹 Proposed cleanup
String username, String password, bool isConnected) async { - // late AuthResponse authResponse; AuthResponse? authResponse;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/platform_android/auth_service_impl.dart` around lines 34 - 35, Remove the dead commented-out declaration "// late AuthResponse authResponse;" and keep only the active nullable declaration "AuthResponse? authResponse;" so there is no commented legacy code; ensure no other references rely on the removed comment and run a quick formatting pass to clean whitespace.
19-20: ⚡ Quick winRemove commented-out code.
The commented-out line showing the old
late User user;should be deleted to keep the codebase clean.🧹 Proposed cleanup
Future<User?> validateUser(String username, String langCode) async { - // late User user; User? user;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/platform_android/auth_service_impl.dart` around lines 19 - 20, Remove the commented-out dead code by deleting the line containing the old declaration "late User user;" so only the active nullable declaration "User? user;" remains in auth_service_impl.dart; this cleans up the file and avoids leaving stale commented code next to the active User? user field.lib/platform_android/machine_key_service_impl.dart (1)
17-17: ⚡ Quick winRemove commented-out code.
The commented-out line should be deleted rather than left in the codebase.
🧹 Proposed cleanup
Future<Machine?> getMachineKeys() async { - // late Machine machine; Machine? machine;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/platform_android/machine_key_service_impl.dart` at line 17, Delete the leftover commented-out declaration "// late Machine machine;" to clean up dead code; remove the commented line referencing Machine/machine from machine_key_service_impl.dart so the file contains only active code and no unused commented declarations.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@lib/platform_android/auth_service_impl.dart`:
- Around line 64-76: The logout() method declares a late String logoutResponse
and returns it even when an exception occurs; initialize logoutResponse to a
safe default (e.g., empty string) at declaration so it is always defined, keep
the try/catch using AuthResponseApi().logout() as-is, and ensure any error paths
still return the initialized value from logout().
- Around line 117-128: The getAutoLogoutPopupTimeout method uses a late String
refreshLoginTime which may remain uninitialized if an exception is thrown;
initialize refreshLoginTime with a safe default (e.g., empty string or a
sensible fallback) when declaring it, or return the fallback directly from the
catch blocks so refreshLoginTime is never returned uninitialized; locate
getAutoLogoutPopupTimeout and the refreshLoginTime variable and update the
declaration/exception handling accordingly.
- Around line 104-115: The getIdleTime method uses a late String (idleTime)
which can remain uninitialized if an exception occurs; replace the late variable
with a safely initialized default (e.g., empty string) or return a default
directly in the catch blocks so getIdleTime() never returns an uninitialized
value—update the AuthResponseApi().getIdleTime() call handling in getIdleTime to
assign a default on exceptions and ensure the method always returns a valid
String.
- Around line 78-89: The method stopAlarmService currently declares late String
stopAlarmServiceResponse which can remain uninitialized if an exception occurs;
replace the late declaration by initializing the variable (e.g., String
stopAlarmServiceResponse = '' ) or make it nullable (String?
stopAlarmServiceResponse) and return a safe default so the return never throws;
update references in stopAlarmService and keep the await call to
AuthResponseApi().stopAlarmService() and the existing catch blocks intact.
- Around line 91-102: forgotPasswordUrl() declares late String
forgotPasswordResponse which can remain uninitialized if a catch runs;
initialize the variable (e.g., String forgotPasswordResponse = ''; ) or return a
safe default from the catch blocks instead so you never return an uninitialized
value. Update the forgotPasswordUrl method to set a default response before the
try or to return a default in the PlatformException / catch blocks when calling
AuthResponseApi().forgotPasswordUrl().
- Around line 143-154: getPasswordLength uses a `late String passwordLength`
which can remain uninitialized if an exception is caught, causing a
LateInitializationError; change this by initializing passwordLength with a safe
default (e.g., `String passwordLength = ""`) or by returning a default value
directly from the catch blocks, and ensure the call to
AuthResponseApi().getPasswordLength() is awaited into that initialized variable
so getPasswordLength always returns a defined String.
---
Nitpick comments:
In `@lib/platform_android/auth_service_impl.dart`:
- Around line 50-51: Remove the leftover commented-out declaration "// late
PacketAuth packetAuth;" and keep only the active nullable declaration
"PacketAuth? packetAuth;" so the file no longer contains dead/commented code;
update any nearby comments if needed to reflect that packetAuth is now nullable.
- Around line 34-35: Remove the dead commented-out declaration "// late
AuthResponse authResponse;" and keep only the active nullable declaration
"AuthResponse? authResponse;" so there is no commented legacy code; ensure no
other references rely on the removed comment and run a quick formatting pass to
clean whitespace.
- Around line 19-20: Remove the commented-out dead code by deleting the line
containing the old declaration "late User user;" so only the active nullable
declaration "User? user;" remains in auth_service_impl.dart; this cleans up the
file and avoids leaving stale commented code next to the active User? user
field.
In `@lib/platform_android/machine_key_service_impl.dart`:
- Line 17: Delete the leftover commented-out declaration "// late Machine
machine;" to clean up dead code; remove the commented line referencing
Machine/machine from machine_key_service_impl.dart so the file contains only
active code and no unused commented declarations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 248b5019-d175-4df1-ae06-61c3d2f2f018
📒 Files selected for processing (4)
lib/platform_android/auth_service_impl.dartlib/platform_android/machine_key_service_impl.dartlib/platform_spi/auth_service.dartlib/platform_spi/machine_key_service.dart
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
1 participant
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.
Description
This PR resolves a widespread architectural bug that caused "silent crashes" across multiple core services, including
AuthServiceImplandMachineKeyImpl.Previously, strict non-nullable interface constraints forced developers to use the
latekeyword for variables assigned insidetry-catchblocks. In the event of aPlatformExceptionor network failure, thecatchblock would execute, but the methods would still attempt to return the uninitializedlatevariable, resulting in an immediateLateInitializationErrorand a fatal application crash.Changes Made
auth_service.dartandmachine_key_service.dartto allow nullable return types (e.g.,Future<User?>,Future<Machine?>) where appropriate.latekeyword fromvalidateUser,login,packetAuthentication, andgetMachineKeys. These methods now safely returnnullif the underlying API call fails, allowing the UI to handle the error gracefully.logout(),stopAlarmService(),forgotPasswordUrl(),getIdleTime(),getAutoLogoutPopupTimeout(), andgetPasswordLength()inauth_service_impl.dart. Removed thelatekeyword and initialized them with safe default values ("").Related Issue
Closes #1047
Type of Change
Checklist
Summary by CodeRabbit