Skip to content

Enhance install error message on app backgrounding #3432

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.

Sign up for GitHub

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

Open
avazirna wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Enhance install error message on app backgrounding #3432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
310bc41
Add message for when connection is interrupted by the system
avazirna Nov 24, 2025
50f28bc
Add flag to monitor foreground status
avazirna Nov 24, 2025
cdb47f8
Throw UnresolvedResourceException when connection is interrupted
avazirna Nov 24, 2025
0cd1764
Update foreground flag when onPause is called
avazirna Nov 24, 2025
a580a83
Remove unnecessary printStackTrace()
avazirna Nov 24, 2025
2dc4076
Lint
avazirna Nov 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions app/assets/locales/android_translatable_strings.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,10 @@ notification.install.rate.limited.title=Servers are busy
notification.install.rate.limited.detail=Install failed because of servers being busy at the moment.
notification.install.rate.limited.action=Our servers are unavailable at this time. Please try again later

notification.install.connection.interrupted.title=Internet connection was interrupted
notification.install.connection.interrupted.detail=Installation failed because the internet connection was interrupted, possibly due to moving away from CommCare
notification.install.connection.interrupted.action=Please try again and keep CommCare open until the installation is complete. If the issue persists, contact your supervisor.

notification.logger.submitted.title=Device Logs Submitted
notification.logger.submitted.detail=Your devices logs have been successfully submitted

Expand Down
11 changes: 11 additions & 0 deletions app/src/org/commcare/CommCareApplication.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ public class CommCareApplication extends Application implements LifecycleEventOb
private CommCareNoficationManager noficationManager;

private boolean backgroundSyncSafe;
private boolean isForeground = true;
Copy link

@coderabbitai coderabbitai bot Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Consider thread safety for the foreground state flag.

The isForeground boolean is accessed from multiple threads (lifecycle callbacks and the install process) without synchronization. While boolean reads/writes are atomic in Java, there's no guarantee of visibility across threads without proper synchronization.

Consider one of these approaches:

  • Make the field volatile to ensure visibility across threads
  • Use AtomicBoolean for thread-safe operations
  • Synchronize access to the field

Apply this diff to make the field volatile:

-    private boolean isForeground = true;
+    private volatile boolean isForeground = true;
📝 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.

Suggested change
private boolean isForeground = true;
private volatile boolean isForeground = true;
🤖 Prompt for AI Agents 
In app/src/org/commcare/CommCareApplication.java around line 215, the
isForeground boolean is accessed from multiple threads without synchronization
which can cause visibility issues; make the field volatile (or alternatively
replace with AtomicBoolean or synchronize access) so updates in one thread are
visible to others—update the field declaration to use the volatile modifier and
adjust any direct reads/writes if you choose AtomicBoolean or synchronized
access instead.


private int connectJobIdForAnalytics = -1;

Expand Down Expand Up @@ -1231,12 +1232,22 @@ private void setRxJavaGlobalHandler() {
@Override
public void onStateChanged(@NonNull LifecycleOwner source, @NonNull Lifecycle.Event event) {
switch (event) {
case ON_START, ON_RESUME:
isForeground = true;
break;
case ON_STOP, ON_PAUSE:
isForeground = false;
break;
case ON_DESTROY:
Logger.log(LogTypes.TYPE_MAINTENANCE, "CommCare has been closed");
break;
}
}

public boolean isAppInForeground() {
return isForeground;
}

public IDatabase getGlobalDbOpenHelper() {
return new EncryptedDatabaseAdapter(new DatabaseGlobalOpenHelper(this));
}
Expand Down
16 changes: 12 additions & 4 deletions app/src/org/commcare/android/resource/installers/FileSystemInstaller.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.commcare.util.CommCarePlatform;
import org.commcare.util.LogTypes;
import org.commcare.utils.AndroidCommCarePlatform;
import org.commcare.utils.ConnectivityStatus;
import org.commcare.utils.FileUtil;
import org.commcare.utils.StringUtils;
import org.javarosa.core.io.StreamsUtil;
Expand All @@ -41,6 +42,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.UnknownHostException;
import java.util.HashMap;
import java.util.Map;
import java.util.Vector;
Expand Down Expand Up @@ -125,10 +127,16 @@ public boolean install(Resource r, ResourceLocation location,

throw mURE;
} catch (IOException e) {
e.printStackTrace();
UnreliableSourceException exception = new UnreliableSourceException(r, e.getMessage());
exception.initCause(e);
throw exception;
if (e instanceof UnknownHostException && !CommCareApplication.instance().isAppInForeground()) {
Copy link
Contributor

@Jignesh-dimagi Jignesh-dimagi Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@avazirna Can it also throw IOException in some cases?

Copy link
Contributor Author

@avazirna avazirna Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, UnknownHostException is a subclass of IOException as well as SocketException, SocketTimeoutException and a few others

UnresolvedResourceException mURE =
Copy link
Contributor

@Jignesh-dimagi Jignesh-dimagi Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also add initCause method?

Copy link
Contributor Author

@avazirna avazirna Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This constructor I'm using already accepts a cause object.

new UnresolvedResourceException(r, e,
"The connection was interrupted because CommCare is in the background", true);
throw mURE;
} else {
UnreliableSourceException exception = new UnreliableSourceException(r, e.getMessage());
exception.initCause(e);
throw exception;
}
} catch (RateLimitedException e) {
UnresolvedResourceException mURE = new UnresolvedResourceException(r, "Our servers are unavailable at this time. Please try again later", true);
mURE.initCause(e);
Expand Down
10 changes: 7 additions & 3 deletions app/src/org/commcare/engine/resource/AppInstallStatus.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public enum AppInstallStatus implements MessageTag {
UnknownFailure("notification.install.unknown"),
NoLocalStorage("notification.install.nolocal"),
NoConnection("notification.install.no.connection"),
ConnectionInterrupted("notification.install.connection.interrupted"),
NetworkFailure("notification.install.network.failure"),
RateLimited("notification.install.rate.limited"),
BadSslCertificate("notification.install.badcert"),
Expand Down Expand Up @@ -76,7 +77,8 @@ public boolean isUpdateInCompletedState() {

public boolean shouldRetryUpdate() {
return (this == NetworkFailure ||
this == NoConnection);
this == NoConnection ||
this == ConnectionInterrupted);
}

@Override
Expand All @@ -91,7 +93,8 @@ public boolean causeUpdateReset() {
this == NoConnection ||
this == CaptivePortal ||
this == RateLimited ||
this == NetworkFailure);
this == NetworkFailure ||
this == ConnectionInterrupted);
}

public boolean isNonPersistentFailure() {
Expand All @@ -101,7 +104,8 @@ public boolean isNonPersistentFailure() {
this == CaptivePortal ||
this == RateLimited ||
this == NetworkFailure ||
this == NoLocalStorage
this == NoLocalStorage ||
this == ConnectionInterrupted
);
}

Expand Down
8 changes: 8 additions & 0 deletions app/src/org/commcare/engine/resource/ResourceInstallUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

import java.net.MalformedURLException;
import java.net.URL;
import java.net.UnknownHostException;
import java.util.Date;

import javax.net.ssl.SSLException;
Expand Down Expand Up @@ -144,6 +145,9 @@ public static void handleAppInstallResult(ResourceEngineTask resourceEngineTask,
case CaptivePortal:
receiver.failWithNotification(AppInstallStatus.CaptivePortal);
break;
case ConnectionInterrupted:
receiver.failWithNotification(AppInstallStatus.ConnectionInterrupted);
break;
default:
receiver.failUnknown(AppInstallStatus.UnknownFailure);
break;
Expand Down Expand Up @@ -267,6 +271,10 @@ public static AppInstallStatus processUnresolvedResource(UnresolvedResourceExcep
Logger.exception("Encountered SSLException while trying to install a resource", exception);
return AppInstallStatus.BadSslCertificate;
}
if (exception.getCause() instanceof UnknownHostException) {
Copy link
Contributor

@Jignesh-dimagi Jignesh-dimagi Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@avazirna Should it check for UnresolvedResourceException ? UnknownHostException might also come whenever there is no internet in app

Copy link
Contributor Author

@avazirna avazirna Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The determining factor is when CommCare is in the background, and in that case we wrap the UnknownHostException inside a UnresolvedResourceException. From Android 15+, checking if the device is connected when in the background seems to always return false.

Copy link
Contributor

@Jignesh-dimagi Jignesh-dimagi Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So just trying to understand here. UnresolvedResourceException is raised whenever there is UnknownHostException and app is in background so shouldn't we check for UnresolvedResourceException instead of UnknownHostException?

Copy link
Contributor Author

@avazirna avazirna Nov 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, so this check is inside a method that only handles UnresolvedResourceException wrapped exceptions, processUnresolvedResource() and this is the catch block where that method is called.

Logger.exception("Encountered UnknownHostException while trying to install a resource", exception);
return AppInstallStatus.ConnectionInterrupted;
}

if(exception.getCause() instanceof RateLimitedException){
return AppInstallStatus.RateLimited;
Expand Down