Fix: Prevent login crash by safely handling missing facilityID and phoneNumber#23
Fix: Prevent login crash by safely handling missing facilityID and phoneNumber#23gurnoorpannu wants to merge 1 commit intoPSMRI:mainfrom
Conversation
…laceID in UserVanSpDetails - Wrapped Toast in withContext(Dispatchers.Main) to avoid UI thread issues - Added null checks for 'facilityID' and 'parkingPlaceID' - Assigned default values (-1) when fields are missing to prevent crashes - Removed redundant lines that accessed these fields unconditionally - Fix verified using demo credentials provided by maintainers
WalkthroughThe code handling user van and service point details in the UserRepo class was updated to improve error handling for missing JSON fields. The presence of "facilityID" and "parkingPlaceID" fields is now checked before assignment. If either field is missing, a default value of -1 is assigned to the respective property. Additionally, when "facilityID" is missing, a Toast message is displayed on the main thread, followed by a 3-second delay. Unconditional assignments that could cause crashes have been removed to enhance robustness. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant UserRepo
participant MainThread (UI)
participant JSON
App->>UserRepo: Retrieve user van/service point details
UserRepo->>JSON: Check for "facilityID"
alt "facilityID" present
UserRepo->>UserRepo: Assign facilityID from JSON
else "facilityID" missing
UserRepo->>MainThread (UI): Show Toast message
MainThread (UI)-->>UserRepo: Wait 3 seconds
UserRepo->>UserRepo: Assign facilityID = -1
end
UserRepo->>JSON: Check for "parkingPlaceID"
alt "parkingPlaceID" present
UserRepo->>UserRepo: Assign parkingPlaceID from JSON
else "parkingPlaceID" missing
UserRepo->>UserRepo: Assign parkingPlaceID = -1
end
Poem
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
app/src/main/java/org/piramalswasthya/cho/repositories/UserRepo.kt (4)
262-264: Proper error handling with fallback value.Setting a default value (-1) when the facility ID is missing is a good approach to prevent null pointer exceptions.
Consider adding a log statement using Timber to help with debugging:
delay(3000) // Set a default value instead of trying to access a non-existent field + Timber.w("facilityID not found in response, using default value (-1)") user?.facilityID = -1 // Use a default value like -1 to indicate missing data
269-273: Well-implemented null check for parkingPlaceID.Your handling of the missing parkingPlaceID field is consistent with the approach for facilityID, providing a default value when the field is absent.
Consider adding a similar log message for consistency:
if (!vanSp.has("parkingPlaceID")) { + Timber.w("parkingPlaceID not found in response, using default value (-1)") user?.parkingPlaceId = -1 // Default value } else { user?.parkingPlaceId = vanSp.getInt("parkingPlaceID") }
262-262: Reconsider the delay duration.The 3-second delay after showing the Toast message might create a noticeable pause in the login flow. Consider if this delay is necessary or if it could be reduced.
- delay(3000) + delay(1500) // Shorter delay while still ensuring the Toast is visible
259-273: Improve code maintainability with constants.The default value (-1) is used in multiple places. Consider defining it as a constant to improve maintainability.
Add this at the class level or in a companion object:
private const val DEFAULT_MISSING_ID = -1Then use it instead of hardcoded -1 values:
- user?.facilityID = -1 // Use a default value like -1 to indicate missing data + user?.facilityID = DEFAULT_MISSING_ID- user?.parkingPlaceId = -1 // Default value + user?.parkingPlaceId = DEFAULT_MISSING_ID
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/java/org/piramalswasthya/cho/repositories/UserRepo.kt(1 hunks)
🔇 Additional comments (3)
app/src/main/java/org/piramalswasthya/cho/repositories/UserRepo.kt (3)
259-261: Good use of withContext for UI operations.Using
withContext(Dispatchers.Main)ensures that Toast messages are properly displayed on the main thread, avoiding potential crashes from updating UI elements from background threads.
274-277: Effective code cleanup.Removing the redundant lines that were causing crashes is a good approach. The comment explains why these lines were removed, which helps with code maintenance.
259-277:Details
❌ Incorrect review comment
PR objective partially addressed.
The changes properly handle missing facilityID and parkingPlaceID, but the PR description also mentioned handling missing phoneNumber which isn't addressed in these changes.
🏁 Script executed:
#!/bin/bash # Search for phoneNumber-related code in the repository to verify if it's handled elsewhere echo "Searching for phoneNumber usage in the codebase..." rg -A 3 "phoneNumber" echo "Checking for any remaining unhandled JSON field access that might cause crashes..." rg -A 2 "vanSp\.getInt|vanSp\.getString" --type kotlinLength of output: 9690
Ignore phoneNumber handling suggestion in UserRepo.kt
The facilityID and parkingPlaceID branches are now correctly guarded. This snippet does not read or set any
phoneNumberfield, so the recommendation to handle a missing phoneNumber here isn’t applicable—no further changes are needed in this file.Likely an incorrect or invalid review comment.
1 participant
This pull request addresses a critical issue during the login flow, where the app would crash if expected fields, such as facilityID or phoneNumber, were missing or null in the user data response. The crash was traced to unsafe assumptions in UserRepo.kt, where these fields were accessed without proper null handling.
Changes Introduced
Implemented null safety checks for facilityID and phoneNumber in UserRepo.kt.
Assigned default fallback values (-1) for missing fields to prevent crashes.
Refactored redundant code that previously accessed these fields unconditionally.
Wrapped Toast calls in withContext(Dispatchers.Main) to ensure UI thread compatibility.
Testing
Tested with both fresh and returning user logins.
Simulated missing or null values in the backend response.
Verified that the login process completes without crashes when essential fields are missing.
Added log warnings for missing fields to align backend expectations.
Impact
Enhances the stability of the login flow.
Prevents app crashes due to incomplete or malformed backend responses.
Improves overall user experience by making the app more resilient to data inconsistencies.
Summary by CodeRabbit