CCCT-1990 BaseConnectFragment NPE Crash #3470

conroy-ricketts · 2025-12-22T19:31:16Z

CCCT-1990

Technical Summary

This crash happens when we try to update the UI after an asynchronous API call if the user navigates away from the screen we are trying to update.

In other words, there are cases where we are attempting to retrieve the binding (to update the UI) after it's set to null in BaseConnectFragment.onDestroyView() (here).

So, since we are incorrectly updating the UI, I searched through all of our Connect fragments and added checks to any fragment where we are updating the UI with that binding after an API call (if we weren't doing so already) so that this crash does not happen again. I actually fixed two crashes at the same time by doing this.

Repro Path 1 (Example of the original crash)

After completing a learning assessment, go to the opportunity details page.
Tap the "DOWNLOAD" button and then immediately hit the back button.
Observe the app crash.

Screen_Recording_20251222_125928_CommCare.Debug.mp4

The app crashes here with java.lang.NullPointerException at org.commcare.fragments.base.BaseConnectFragment.getBinding()) - this is the original crash I reported in the ticket.

With my changes, this is what the app looks like when it does not crash:

Screen_Recording_20251222_130417_CommCare.Debug.mp4

Repro Path 2 (A different but similar crash)

Go to the job intro screen for a brand new opportunity.
Tap the "GO TO LEARN APP" button and then immediately hit the back button.
Observe the app crash.

Screen_Recording_20251222_123008_CommCare.Debug.mp4

The app crashes here with java.lang.IllegalStateException: Fragment ConnectJobIntroFragment... not attached to an activity.

With my changes, this is what the app looks like when it does not crash:

Screen_Recording_20251222_124254_CommCare.Debug.mp4

I was not able to reproduce the app crashing when refreshing data (here and here) or confirming payments (here) in both the ConnectJobsListsFragment and the ConnectDeliveryProgressFragment, which is actually a bit weird to me because the app should be crashing similarly to the two repro paths I outlined above unless I'm missing something. Regardless, I went ahead and added checks to those two fragments as well anyways because we are still using the binding to update the UI after an API call and it's good practice. Let me know if there are any objections to that.

Safety Assurance

Safety story

I verified that I no longer see the app crash for the two repro paths above.

I verified that the app behaves as expected when not forcing a crash.

QA Plan

We should ensure that the app never crashes in the two repro paths I outlined above.

Added checks to API callbacks for the case that the respective fragment is added to the activity.

…nto bug/CCCT-1990-NPE-for-base-connect-fragment

coderabbitai · 2025-12-22T19:34:26Z

📝 Walkthrough

Walkthrough

This PR adds defensive lifecycle-aware guards across four Connect-related fragments to prevent UI operations when fragments are not attached to their parent activities. Specifically, isAdded() checks are inserted before UI updates and state changes in callback methods (onSuccess/onFailure paths) across ConnectDeliveryDetailsFragment, ConnectDeliveryProgressFragment, ConnectJobIntroFragment, and ConnectJobsListsFragment. Analytics calls remain unguarded. No public APIs or method signatures are modified.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Rationale: The changes follow a consistent, homogeneous pattern (adding isAdded() guards) across four files with straightforward defensive logic. No new functionality or complex control flow is introduced. The edits are repetitive and low-risk.

Possibly related PRs

- fixed view jib status click issue #3242 — Applies the same isAdded() guard pattern to Connect fragment callbacks to prevent operations when fragments are detached.
Require active job in some places #3198 — Modifies the same Connect fragments with changes to fragment lifecycle and job-access patterns.
Learn-to-deliver transition improvement #2924 — Updates the same Connect fragments including ConnectDeliveryDetailsFragment and ConnectJobIntroFragment.

Suggested reviewers

OrangeAndGreen
shubham1g5
Jignesh-dimagi
jaypanchal-13

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main issue being fixed: an NPE crash in BaseConnectFragment, which directly corresponds to the changeset's primary purpose.
Description check	✅ Passed	The description includes technical summary with ticket link, detailed repro paths with video evidence, safety story with verification steps, and QA plan, covering most required template sections comprehensively.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch bug/CCCT-1990-NPE-for-base-connect-fragment

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0dcdc43 and 3b4dd97.

📒 Files selected for processing (4)

app/src/org/commcare/fragments/connect/ConnectDeliveryDetailsFragment.java
app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java
app/src/org/commcare/fragments/connect/ConnectJobIntroFragment.java
app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java

🧰 Additional context used

🧠 Learnings (16)

📓 Common learnings

Learnt from: shubham1g5
Repo: dimagi/commcare-android PR: 0
File: :0-0
Timestamp: 2025-05-08T11:08:18.530Z
Learning: PR #3048 "Phase 4 Connect PR" introduces a substantial feature called "Connect" to the CommCare Android app, which includes messaging, job management, delivery tracking, payment processing, authentication flows, and learning modules. It follows a modern architecture using Navigation Components with three navigation graphs, segregated business logic in Manager classes, and proper database persistence.

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3108
File: app/src/org/commcare/fragments/connect/ConnectDownloadingFragment.java:74-78
Timestamp: 2025-06-06T19:54:26.428Z
Learning: In ConnectDownloadingFragment.java and similar Connect-related code, the team prefers to let "should never happen" scenarios like null app records crash rather than add defensive null checks, following a fail-fast philosophy to catch programming errors during development.

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3121
File: app/src/org/commcare/activities/CommCareSetupActivity.java:360-364
Timestamp: 2025-05-22T14:28:35.959Z
Learning: In CommCareSetupActivity.java, the call to installFragment.showConnectErrorMessage() after fragment transactions is intentionally unguarded with null checks. This follows the app's design pattern where critical error paths prefer immediate crashes over silent failures, making potential issues immediately visible during development rather than hiding them with defensive programming.

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/connect/MessageManager.java:0-0
Timestamp: 2025-07-29T14:54:47.940Z
Learning: User OrangeAndGreen organizes messaging-related changes into separate PRs rather than mixing them with other Connect work, which aligns with their preference for handling code issues in separate PRs when they are not directly related to the main changes.

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java:0-0
Timestamp: 2025-07-29T14:11:36.386Z
Learning: In ConnectJobsListsFragment.java error handling for JSON parsing, if the JSONObject obj is null when passed to handleCorruptJob(), the team prefers to let the code crash rather than adding defensive null checks. This follows the fail-fast philosophy used throughout the Connect feature to catch programming errors immediately rather than masking them.

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3121
File: app/src/org/commcare/fragments/SelectInstallModeFragment.java:201-205
Timestamp: 2025-05-22T14:26:41.341Z
Learning: In SelectInstallModeFragment.java, the showConnectErrorMessage method intentionally omits null checks because it's called at a specific point in the startup flow where UI is guaranteed to be loaded. It's designed to crash if activity or view is null to make potential issues immediately visible rather than hiding them with defensive programming.

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/fragments/connect/ConnectUnlockFragment.java:0-0
Timestamp: 2025-07-29T14:10:55.131Z
Learning: In ConnectUnlockFragment.java, opportunityId values are expected to always contain valid integer strings. Integer.parseInt() should be allowed to throw NumberFormatException if invalid data is encountered, following the fail-fast philosophy used throughout the Connect feature for data contract violations.

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3108
File: app/src/org/commcare/fragments/connect/ConnectUnlockFragment.java:62-64
Timestamp: 2025-06-04T19:17:21.213Z
Learning: In ConnectUnlockFragment.java, the user prefers to let getArguments() potentially throw NullPointerException rather than adding null checks, as the arguments are required for proper navigation flow and their absence indicates a programming error that should fail fast.

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3040
File: app/src/org/commcare/android/database/connect/models/ConnectJobDeliveryFlagRecord.java:39-55
Timestamp: 2025-04-18T20:13:29.655Z
Learning: In the CommCare Android Connect feature, the JSON object passed to `ConnectJobDeliveryFlagRecord.fromJson()` method should never be null based on the implementation design.

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java:0-0
Timestamp: 2025-07-29T14:10:58.243Z
Learning: In ConnectJobsListsFragment.java, the team intentionally uses different error handling strategies: JSONException should throw RuntimeException to crash the app (fail-fast for data contract violations), while IOException should be logged and allow graceful continuation (for network/stream issues). This follows the established Connect module pattern of treating different exception types differently based on their nature.

📚 Learning: 2025-07-29T14:11:36.386Z

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java:0-0
Timestamp: 2025-07-29T14:11:36.386Z
Learning: In ConnectJobsListsFragment.java error handling for JSON parsing, if the JSONObject obj is null when passed to handleCorruptJob(), the team prefers to let the code crash rather than adding defensive null checks. This follows the fail-fast philosophy used throughout the Connect feature to catch programming errors immediately rather than masking them.

Applied to files:

app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java
app/src/org/commcare/fragments/connect/ConnectJobIntroFragment.java
app/src/org/commcare/fragments/connect/ConnectDeliveryDetailsFragment.java

📚 Learning: 2025-07-29T14:10:58.243Z

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java:0-0
Timestamp: 2025-07-29T14:10:58.243Z
Learning: In ConnectJobsListsFragment.java, the team intentionally uses different error handling strategies: JSONException should throw RuntimeException to crash the app (fail-fast for data contract violations), while IOException should be logged and allow graceful continuation (for network/stream issues). This follows the established Connect module pattern of treating different exception types differently based on their nature.

Applied to files:

app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java
app/src/org/commcare/fragments/connect/ConnectJobIntroFragment.java
app/src/org/commcare/fragments/connect/ConnectDeliveryDetailsFragment.java

📚 Learning: 2025-06-06T19:54:26.428Z

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3108
File: app/src/org/commcare/fragments/connect/ConnectDownloadingFragment.java:74-78
Timestamp: 2025-06-06T19:54:26.428Z
Learning: In ConnectDownloadingFragment.java and similar Connect-related code, the team prefers to let "should never happen" scenarios like null app records crash rather than add defensive null checks, following a fail-fast philosophy to catch programming errors during development.

Applied to files:

app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java
app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java
app/src/org/commcare/fragments/connect/ConnectJobIntroFragment.java
app/src/org/commcare/fragments/connect/ConnectDeliveryDetailsFragment.java

📚 Learning: 2025-05-08T11:08:18.530Z

Learnt from: shubham1g5
Repo: dimagi/commcare-android PR: 0
File: :0-0
Timestamp: 2025-05-08T11:08:18.530Z
Learning: PR #3048 "Phase 4 Connect PR" introduces a substantial feature called "Connect" to the CommCare Android app, which includes messaging, job management, delivery tracking, payment processing, authentication flows, and learning modules. It follows a modern architecture using Navigation Components with three navigation graphs, segregated business logic in Manager classes, and proper database persistence.

Applied to files:

app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java
app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java
app/src/org/commcare/fragments/connect/ConnectJobIntroFragment.java
app/src/org/commcare/fragments/connect/ConnectDeliveryDetailsFragment.java

📚 Learning: 2025-05-22T14:28:35.959Z

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3121
File: app/src/org/commcare/activities/CommCareSetupActivity.java:360-364
Timestamp: 2025-05-22T14:28:35.959Z
Learning: In CommCareSetupActivity.java, the call to installFragment.showConnectErrorMessage() after fragment transactions is intentionally unguarded with null checks. This follows the app's design pattern where critical error paths prefer immediate crashes over silent failures, making potential issues immediately visible during development rather than hiding them with defensive programming.

Applied to files:

app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java
app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java
app/src/org/commcare/fragments/connect/ConnectJobIntroFragment.java
app/src/org/commcare/fragments/connect/ConnectDeliveryDetailsFragment.java

📚 Learning: 2025-04-18T20:13:29.655Z

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3040
File: app/src/org/commcare/android/database/connect/models/ConnectJobDeliveryFlagRecord.java:39-55
Timestamp: 2025-04-18T20:13:29.655Z
Learning: In the CommCare Android Connect feature, the JSON object passed to `ConnectJobDeliveryFlagRecord.fromJson()` method should never be null based on the implementation design.

Applied to files:

app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java
app/src/org/commcare/fragments/connect/ConnectJobIntroFragment.java
app/src/org/commcare/fragments/connect/ConnectDeliveryDetailsFragment.java

📚 Learning: 2025-06-20T15:51:14.157Z

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3108
File: app/src/org/commcare/models/connect/ConnectLoginJobListModel.java:79-92
Timestamp: 2025-06-20T15:51:14.157Z
Learning: The ConnectLoginJobListModel class in app/src/org/commcare/models/connect/ConnectLoginJobListModel.java does not need to implement Parcelable interface as it is not passed between Android activities or fragments.

Applied to files:

app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java
app/src/org/commcare/fragments/connect/ConnectJobIntroFragment.java

📚 Learning: 2025-06-06T19:52:53.173Z

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3108
File: app/src/org/commcare/fragments/connect/ConnectDeliveryDetailsFragment.java:163-180
Timestamp: 2025-06-06T19:52:53.173Z
Learning: In the CommCare Android Connect feature, database operations like ConnectJobUtils.upsertJob should be allowed to crash rather than being wrapped in try-catch blocks. The team prefers fail-fast behavior for database errors instead of graceful error handling.

Applied to files:

app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java
app/src/org/commcare/fragments/connect/ConnectJobIntroFragment.java

📚 Learning: 2025-07-29T14:10:55.131Z

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3248
File: app/src/org/commcare/fragments/connect/ConnectUnlockFragment.java:0-0
Timestamp: 2025-07-29T14:10:55.131Z
Learning: In ConnectUnlockFragment.java, opportunityId values are expected to always contain valid integer strings. Integer.parseInt() should be allowed to throw NumberFormatException if invalid data is encountered, following the fail-fast philosophy used throughout the Connect feature for data contract violations.

Applied to files:

app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java
app/src/org/commcare/fragments/connect/ConnectDeliveryDetailsFragment.java

📚 Learning: 2025-02-19T15:15:01.935Z

Learnt from: pm-dimagi
Repo: dimagi/commcare-android PR: 2956
File: app/src/org/commcare/fragments/connectMessaging/ConnectMessageChannelListFragment.java:58-60
Timestamp: 2025-02-19T15:15:01.935Z
Learning: Error handling for message retrieval in ConnectMessageChannelListFragment's retrieveMessages callback is not required as per user preference.

Applied to files:

app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java

📚 Learning: 2025-01-21T18:19:05.799Z

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 2912
File: app/src/org/commcare/fragments/connectId/ConnectIdPasswordVerificationFragment.java:214-215
Timestamp: 2025-01-21T18:19:05.799Z
Learning: In ConnectIdPasswordVerificationFragment, when creating a ConnectUserRecord, it's acceptable for payment information (paymentName and paymentPhone) to be empty strings if the server response doesn't include payment info in the CONNECT_PAYMENT_INFO field.

Applied to files:

app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java

📚 Learning: 2025-05-22T14:26:41.341Z

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3121
File: app/src/org/commcare/fragments/SelectInstallModeFragment.java:201-205
Timestamp: 2025-05-22T14:26:41.341Z
Learning: In SelectInstallModeFragment.java, the showConnectErrorMessage method intentionally omits null checks because it's called at a specific point in the startup flow where UI is guaranteed to be loaded. It's designed to crash if activity or view is null to make potential issues immediately visible rather than hiding them with defensive programming.

Applied to files:

app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java
app/src/org/commcare/fragments/connect/ConnectJobIntroFragment.java
app/src/org/commcare/fragments/connect/ConnectDeliveryDetailsFragment.java

📚 Learning: 2025-06-06T20:15:21.134Z

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3108
File: app/src/org/commcare/fragments/connect/ConnectJobIntroFragment.java:65-71
Timestamp: 2025-06-06T20:15:21.134Z
Learning: In the CommCare Android Connect module, job.getLearnAppInfo() and getLearnModules() should never return null according to the system design. The team prefers to let the code crash if these values are unexpectedly null rather than adding defensive null checks, following a fail-fast philosophy to catch bugs early rather than masking them.

Applied to files:

app/src/org/commcare/fragments/connect/ConnectJobIntroFragment.java

📚 Learning: 2025-05-09T10:57:41.073Z

Learnt from: Jignesh-dimagi
Repo: dimagi/commcare-android PR: 3093
File: app/res/navigation/nav_graph_connect_messaging.xml:41-45
Timestamp: 2025-05-09T10:57:41.073Z
Learning: In the CommCare Android codebase, the navigation graph for Connect messaging (`nav_graph_connect_messaging.xml`) intentionally uses `channel_id` as the argument name in the connectMessageFragment, despite using `channelId` in other parts of the same navigation graph. This naming difference is by design in the refactored code.

Applied to files:

app/src/org/commcare/fragments/connect/ConnectDeliveryDetailsFragment.java

📚 Learning: 2025-06-04T19:17:21.213Z

Learnt from: OrangeAndGreen
Repo: dimagi/commcare-android PR: 3108
File: app/src/org/commcare/fragments/connect/ConnectUnlockFragment.java:62-64
Timestamp: 2025-06-04T19:17:21.213Z
Learning: In ConnectUnlockFragment.java, the user prefers to let getArguments() potentially throw NullPointerException rather than adding null checks, as the arguments are required for proper navigation flow and their absence indicates a programming error that should fail fast.

Applied to files:

app/src/org/commcare/fragments/connect/ConnectDeliveryDetailsFragment.java

🧬 Code graph analysis (2)

app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java (1)

app/src/org/commcare/connect/network/PersonalIdOrConnectApiErrorHandler.java (1)

PersonalIdOrConnectApiErrorHandler (25-74)

app/src/org/commcare/fragments/connect/ConnectDeliveryDetailsFragment.java (3)

app/src/org/commcare/google/services/analytics/FirebaseAnalyticsUtil.java (1)

FirebaseAnalyticsUtil (43-612)

app/src/org/commcare/connect/network/base/BaseApiHandler.kt (1)

onFailure (19-22)

app/src/org/commcare/connect/network/base/BaseApi.kt (1)

onFailure (60-65)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Lint Code Base

🔇 Additional comments (7)

app/src/org/commcare/fragments/connect/ConnectJobsListsFragment.java (2)

83-85: LGTM: Lifecycle-aware guard prevents UI operations on detached fragment.

The early return correctly prevents both the Toast display and the subsequent navigateFailure() call (which updates the RecyclerView) when the fragment is no longer attached.

94-97: LGTM: Conditional UI update prevents crashes when fragment is detached.

The guard ensures setJobListData() only executes when the fragment is attached. The corruptJobs assignment on Line 93 occurs unconditionally, which is safe since it's just updating internal state without UI operations.

app/src/org/commcare/fragments/connect/ConnectJobIntroFragment.java (1)

123-131: LGTM: State persists unconditionally; UI actions guarded appropriately.

The job status update and database upsert (Lines 126-127) execute regardless of fragment state, ensuring the successful API call is reflected in persistent storage. The isAdded() guard then prevents UI operations (app launch or navigation) when the fragment is detached. This ordering is correct: persist state changes but skip user-facing actions if the fragment is no longer active.

app/src/org/commcare/fragments/connect/ConnectDeliveryProgressFragment.java (2)

123-128: LGTM: Combined guard efficiently protects all UI updates in refresh callback.

The condition success && isAdded() ensures that the four UI operations (updating last updated text, card message, payment confirmation tile, and refreshing the view pager) only execute when both the API call succeeds and the fragment remains attached.

150-154: LGTM: Payment confirmation UI update guarded appropriately.

The isAdded() check ensures updatePaymentConfirmationTile() only executes when the fragment is attached, preventing UI updates on a detached fragment after the async payment confirmation call completes.

app/src/org/commcare/fragments/connect/ConnectDeliveryDetailsFragment.java (2)

97-103: LGTM: Success path guards UI operations while preserving analytics.

The isAdded() check correctly prevents proceedAfterJobClaimed() (which updates job status, closes the session, and navigates) from executing when the fragment is detached. The analytics call on Line 102 intentionally remains outside the guard to ensure API success is tracked regardless of fragment lifecycle state.

106-118: LGTM: Failure path guards error handling while preserving analytics.

The isAdded() guard appropriately wraps error message construction and snackbar display, preventing UI operations when the fragment is detached. The analytics call on Line 117 correctly remains outside the guard to ensure API failures are tracked regardless of fragment state.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

OrangeAndGreen

Will definitely be nice to stop getting those NPEs

Jignesh-dimagi · 2025-12-23T05:28:04Z

@conroy-ricketts This looks good overall. This is just a though and need your views also @conroy-ricketts @OrangeAndGreen @shubham1g5

Your changes will definitely reduce the number of crashes but it cannot guarantee that it will stop crashes depending upon when onDestroyView is called. If application execution has gone beyond isAdded() and than onDestroyView is called, application will still crash. I feel that making binding = null in onDestroyView doesn't make sense at all. It should be removed as short term solution. Long term solution, all calls should be moved to ViewModel with viewModelScope so that whenever it get destroyed, calls are not hitting UI at all.

conroy-ricketts · 2025-12-23T13:10:24Z

@Jignesh-dimagi I think we are setting the binding to null to prevent memory leaks if I'm not mistaken

conroy-ricketts · 2025-12-23T13:13:08Z

But I do agree that these changes will not 100% guarantee the crashes stop completely, especially if we implement or change something in the future and we forget about these scenarios

conroy-ricketts · 2025-12-23T13:15:24Z

And also +1 on the comment about viewModelScope

Jignesh-dimagi · 2025-12-23T13:29:27Z

@Jignesh-dimagi I think we are setting the binding to null to prevent memory leaks if I'm not mistaken

I don't think it's required to set to null. View is already getting destroyed.

OrangeAndGreen · 2025-12-23T14:53:03Z

Agreed on all the above, #1 being that long-term we should get these moved to view model scopes. Additional thoughts:

@Jignesh-dimagi Do you expect removing the binding=null in onDestroyView to result in fewer crashes? If it has no effect or could result in more crashes then I'd be wary of making that change in the short-term.
If I understand correctly, the remaining concern is an API call returning after onDestroyView was called but while isAdded() is still true. Would null-checking on binding instead of (or in addition to) relying on isAdded be better?

conroy-ricketts · 2025-12-26T13:47:29Z

@OrangeAndGreen @Jignesh-dimagi

I just recently did some more testing on a build off of the master branch where all I did was remove _binding = null in onDestroyView(), and I followed the same 2 repro paths that I put in the PR description.

For repro path 1, the app still crashes but with a different exception:

java.lang.IllegalStateException: Fragment ConnectDeliveryDetailsFragment... not attached to a context.

Screen_Recording_20251226_084028_CommCare.Debug.mp4

For repro path 2, the app still crashes which makes sense because the crash here is the same one I saw the other day which is not directly related to the binding being null:

java.lang.IllegalStateException: Fragment ConnectJobIntroFragment... not attached to an activity.

Screen_Recording_20251226_083440_CommCare.Debug.mp4

So, I think that simply removing _binding = null in onDestroyView() will not result in fewer crashes.

CCCT-1990 BaseConnectFragment.getBinding NPE Crash

3b4dd97

Added checks to API callbacks for the case that the respective fragment is added to the activity.

conroy-ricketts requested review from Jignesh-dimagi, OrangeAndGreen, jaypanchal-13 and shubham1g5 December 22, 2025 19:31

conroy-ricketts self-assigned this Dec 22, 2025

conroy-ricketts added the QA Note label Dec 22, 2025

Merge branch 'master' of https://github.com/dimagi/commcare-android i…

2a6e3d3

…nto bug/CCCT-1990-NPE-for-base-connect-fragment

OrangeAndGreen approved these changes Dec 22, 2025

View reviewed changes

shubham1g5 approved these changes Dec 29, 2025

View reviewed changes

conroy-ricketts merged commit 82c52bc into master Jan 2, 2026
7 checks passed

conroy-ricketts deleted the bug/CCCT-1990-NPE-for-base-connect-fragment branch January 2, 2026 18:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

CCCT-1990 BaseConnectFragment NPE Crash #3470

CCCT-1990 BaseConnectFragment NPE Crash #3470

Uh oh!

conroy-ricketts commented Dec 22, 2025

Uh oh!

coderabbitai bot commented Dec 22, 2025

Walkthrough

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

OrangeAndGreen left a comment

Uh oh!

Jignesh-dimagi commented Dec 23, 2025

Uh oh!

conroy-ricketts commented Dec 23, 2025

Uh oh!

conroy-ricketts commented Dec 23, 2025 •

edited

Loading

Uh oh!

conroy-ricketts commented Dec 23, 2025

Uh oh!

Jignesh-dimagi commented Dec 23, 2025

Uh oh!

OrangeAndGreen commented Dec 23, 2025 •

edited

Loading

Uh oh!

conroy-ricketts commented Dec 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

CCCT-1990 BaseConnectFragment NPE Crash #3470

CCCT-1990 BaseConnectFragment NPE Crash #3470

Uh oh!

Conversation

conroy-ricketts commented Dec 22, 2025

CCCT-1990

Technical Summary

Repro Path 1 (Example of the original crash)

Repro Path 2 (A different but similar crash)

Safety Assurance

Safety story

QA Plan

Uh oh!

coderabbitai bot commented Dec 22, 2025

Walkthrough

Estimated code review effort

Possibly related PRs

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

OrangeAndGreen left a comment

Choose a reason for hiding this comment

Uh oh!

Jignesh-dimagi commented Dec 23, 2025

Uh oh!

conroy-ricketts commented Dec 23, 2025

Uh oh!

conroy-ricketts commented Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

conroy-ricketts commented Dec 23, 2025

Uh oh!

Jignesh-dimagi commented Dec 23, 2025

Uh oh!

OrangeAndGreen commented Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

conroy-ricketts commented Dec 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

conroy-ricketts commented Dec 23, 2025 •

edited

Loading

OrangeAndGreen commented Dec 23, 2025 •

edited

Loading