Fix: Prevent crash by moving Toast to main thread in UserRepo#253
Fix: Prevent crash by moving Toast to main thread in UserRepo#253Tomeshwari-02 wants to merge 3 commits intoPSMRI:mainfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 54 minutes and 32 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRefactored the missing- Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 (1)
app/src/main/java/org/piramalswasthya/cho/repositories/UserRepo.kt (1)
229-237:⚠️ Potential issue | 🟠 MajorGuard is ineffective —
getInt("facilityID")on line 235 will still throw when the key is missing.The
!vanSp.has("facilityID")branch only shows a toast and sleeps; control then falls through to line 235 (vanSp.getInt("facilityID")), which will throwJSONExceptionbecause the key does not exist. The net result is that after this PR the user sees the toast and the login flow still crashes/fails for the same input the toast was meant to warn about. Add acontinue(to skip this entry) orreturn@withContext falseinside theifblock so the missing-facility case is actually handled.🐛 Proposed fix
if (!vanSp.has("facilityID")) { withContext(Dispatchers.Main) { Toast.makeText(context, "Facility ID not found", Toast.LENGTH_LONG).show() } delay(3000) + return@withContext false } val facilityId = vanSp.getInt("facilityID")Use
continueinstead ofreturn@withContext falseif the intent is to skip this entry but still process subsequentvanSpDetailsArrayelements.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/org/piramalswasthya/cho/repositories/UserRepo.kt` around lines 229 - 237, The guard for missing "facilityID" is ineffective because after showing the toast and delaying you still call vanSp.getInt("facilityID") which will throw; inside the if (!vanSp.has("facilityID")) block (the block that shows the Toast and delay) add a continue to skip this vanSp entry so you don't run vanSp.getInt("facilityID") and assign user?.facilityID / user?.parkingPlaceId for a missing key; this preserves processing of subsequent vanSpDetailsArray elements.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@app/src/main/java/org/piramalswasthya/cho/repositories/UserRepo.kt`:
- Around line 229-237: The guard for missing "facilityID" is ineffective because
after showing the toast and delaying you still call vanSp.getInt("facilityID")
which will throw; inside the if (!vanSp.has("facilityID")) block (the block that
shows the Toast and delay) add a continue to skip this vanSp entry so you don't
run vanSp.getInt("facilityID") and assign user?.facilityID /
user?.parkingPlaceId for a missing key; this preserves processing of subsequent
vanSpDetailsArray elements.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8b84949e-76c4-4f93-99a2-757b41d57e4e
📒 Files selected for processing (1)
app/src/main/java/org/piramalswasthya/cho/repositories/UserRepo.kt
|
@Tomeshwari-02 there are SonarCloud comments. Please check. |
|
@sushant-bizbrolly please review |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/org/piramalswasthya/cho/repositories/UserRepo.kt`:
- Around line 223-229: The loop that iterates vanSpDetailsArray currently
continues past entries missing "facilityID", which can leave user (user.vanId,
user.servicePointId, user.servicePointName, user.facilityID,
user.parkingPlaceId) uninitialized while the function still returns true and
downstream callers like getLocDetailsBasedOnSpIDAndPsmID() dereference
user!!.vanId; fix by tracking whether at least one valid entry was processed
(e.g., processedFound flag) and return false if none were processed, move the
Toast + delay out of the per-item loop so you show a single message and do a
single delay (or break after the first toast) to avoid cumulative 3s×N blocking,
and keep using withContext(Dispatchers.Main) for the Toast instead of spawning
GlobalScope coroutines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3cf7824d-a8cd-4781-b014-214733a09e58
📒 Files selected for processing (1)
app/src/main/java/org/piramalswasthya/cho/repositories/UserRepo.kt
|
Done 👍 |
2 participants
📋 Description
This PR fixes a potential crash caused by displaying a Toast from a background thread (Dispatchers.IO) in UserRepo.kt.
Previously, a Toast message ("Facility ID not found") was triggered inside an IO coroutine, which can lead to CalledFromWrongThreadException since UI operations must run on the main thread.
The fix ensures that the Toast is executed on Dispatchers.Main, making it safe and compliant with Android threading rules.
✅ Type of Change
ℹ️ Additional Information
Testing
Verified the login flow where the Toast is triggered
Confirmed that the Toast displays correctly without any crash
Ensured no impact on existing functionality
Notes
This is a minimal and safe fix
Improves app stability by ensuring UI operations run on the main thread
Aligns with Android best practices for coroutine usage
Summary by CodeRabbit