Skip to content

Improved Connect DB error handling #3348

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

Merged
OrangeAndGreen merged 5 commits into from
Sep 19, 2025
Merged

Improved Connect DB error handling #3348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
656fb87
No longer logging when Connect DB passphrase (local or remote) not fo…
OrangeAndGreen Sep 17, 2025
6682ffe
Logging non-fatal exception before attempting trySqlCipherDbUpdate wh…
OrangeAndGreen Sep 17, 2025
ba12699
Improved exception logging for DB crashes to better indicate source.
OrangeAndGreen Sep 18, 2025
80f8734
Preserving existing enum value order
OrangeAndGreen Sep 18, 2025
0af2793
Including source exception that caused DB crash when available.
OrangeAndGreen Sep 18, 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
3 changes: 2 additions & 1 deletion app/src/org/commcare/connect/PersonalIdManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.commcare.utils.BiometricsHelper;
import org.commcare.utils.CrashUtil;
import org.commcare.utils.EncryptionKeyProvider;
import org.commcare.utils.GlobalErrors;
import org.commcare.views.dialogs.StandardAlertDialog;
import org.javarosa.core.io.StreamsUtil;
import org.javarosa.core.services.Logger;
Expand Down Expand Up @@ -124,7 +125,7 @@ public void init(Context parent) {
}
} else if (ConnectDatabaseHelper.isDbBroken()) {
//Corrupt DB, inform user to recover
ConnectDatabaseHelper.crashDb();
ConnectDatabaseHelper.crashDb(GlobalErrors.PERSONALID_DB_STARTUP_ERROR);
}
}
}
Expand Down
20 changes: 10 additions & 10 deletions app/src/org/commcare/connect/database/ConnectDatabaseHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ public static void handleReceivedDbPassphrase(Context context, String remotePass
//Store the received passphrase as what's in use locally
ConnectDatabaseUtils.storeConnectDbPassphrase(context, remotePassphrase, true);
} catch (Exception e) {
Logger.exception("Handling received DB passphrase", e);
Copy link
Contributor

@shubham1g5 shubham1g5 Sep 18, 2025

Choose a reason for hiding this comment

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

curious why are not we logging the underlying exceptions now for crashDb calls.

Copy link
Contributor Author

@OrangeAndGreen OrangeAndGreen Sep 18, 2025

Choose a reason for hiding this comment

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

Ah, interesting. I removed them because they were essentially just duplicates of the fatal exception that gets logged immediately after when the app crashes, but didn't think about the lost crash-causing exception. I'll pass it through so the crashing exception can wrap it and we can preserve the full stack while avoiding duplicate events.

Copy link
Contributor Author

@OrangeAndGreen OrangeAndGreen Sep 18, 2025

Choose a reason for hiding this comment

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

0af2793

Copy link
Contributor Author

@OrangeAndGreen OrangeAndGreen Sep 18, 2025

Choose a reason for hiding this comment

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

Also something worth noting: I checked back 90 days and neither of the two related non-fatal exceptions had been logged a single time

Copy link
Contributor

@shubham1g5 shubham1g5 Sep 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

neither of the two related non-fatal exceptions

Confused exactly which two non-fatals you are mentioning here.

crashDb();
crashDb(GlobalErrors.PERSONALID_DB_UPGRADE_ERROR, e);
}
}

Expand All @@ -73,6 +72,9 @@ public IDatabase getHandle() {
if (connectDatabase == null || !connectDatabase.isOpen()) {
try {
byte[] passphrase = ConnectDatabaseUtils.getConnectDbPassphrase(context, true);
if(passphrase == null) {
throw new IllegalStateException("Attempting to access Connect DB without a passphrase");
}

String remotePassphrase = ConnectDatabaseUtils.getConnectDbEncodedPassphrase(context, false);
String localPassphrase = ConnectDatabaseUtils.getConnectDbEncodedPassphrase(context, true);
Expand All @@ -83,17 +85,15 @@ public IDatabase getHandle() {
UserSandboxUtils.getSqlCipherEncodedKey(passphrase));
} else {
//LEGACY: Used to open the DB using the byte[], not String overload
String encrypted = passphrase != null ? "(encrypted)" : "(unencrypted)";
Logger.exception("Legacy DB Usage", new Exception("Accessing Connect DB via legacy code " + encrypted));
Logger.exception("Legacy DB Usage", new Exception("Accessing Connect DB via legacy code"));
dbConnectOpenHelper = new DatabaseConnectOpenHelper(this.c,
Base64.encodeToString(passphrase, Base64.NO_WRAP));
}
connectDatabase = new EncryptedDatabaseAdapter(dbConnectOpenHelper);
} catch (Exception e) {
//Flag the DB as broken if we hit an error opening it (usually means corrupted or bad encryption)
dbBroken = true;
Logger.exception("Corrupt Connect DB", e);
crashDb();
crashDb(GlobalErrors.PERSONALID_GENERIC_ERROR, e);
}
}
return connectDatabase;
Expand All @@ -111,13 +111,13 @@ public static void teardown() {
}
}

public static void crashDb() {
crashDb(GlobalErrors.PERSONALID_GENERIC_ERROR);
public static void crashDb(GlobalErrors error) {
crashDb(error, null);
}

public static void crashDb(GlobalErrors error) {
public static void crashDb(GlobalErrors error, Exception ex) {
GlobalErrorUtil.addError(new GlobalErrorRecord(new Date(), error.ordinal()));
throw new RuntimeException("Connect database crash");
throw new RuntimeException("Connect database crash: " + error.name(), ex);
}

public static void storeHqToken(Context context, String appId, String userId, SsoToken token) {
Expand Down
20 changes: 10 additions & 10 deletions app/src/org/commcare/connect/database/ConnectDatabaseUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,17 @@ public static String getConnectDbEncodedPassphrase(Context context, boolean isLo
public static byte[] getConnectDbPassphrase(Context context, boolean isLocal) {
try {
ConnectKeyRecord record = ConnectDatabaseUtils.getKeyRecord(isLocal);
if (record != null) {
byte[] encrypted = Base64.decode(record.getEncryptedPassphrase());

EncryptionKeyProvider encryptionKeyProvider = new EncryptionKeyProvider(context, false,
SECRET_NAME);
EncryptionKeyAndTransform keyAndTransform = encryptionKeyProvider.getKeyForDecryption();
return EncryptionUtils.decrypt(encrypted, keyAndTransform.getKey(), keyAndTransform.getTransformation(), true);
} else {
CrashUtil.log("We don't find paraphrase in db");
throw new RuntimeException("We don't find a record in db to get passphrase");
if (record == null) {
return null;
}

byte[] encrypted = Base64.decode(record.getEncryptedPassphrase());

EncryptionKeyProvider encryptionKeyProvider = new EncryptionKeyProvider(context, false,
SECRET_NAME);
EncryptionKeyAndTransform keyAndTransform = encryptionKeyProvider.getKeyForDecryption();
return EncryptionUtils.decrypt(encrypted, keyAndTransform.getKey(),
keyAndTransform.getTransformation(), true);
} catch (Base64DecoderException | EncryptionUtils.EncryptionException e) {
throw new RuntimeException(e);
}
Expand Down
2 changes: 2 additions & 0 deletions app/src/org/commcare/models/database/app/DatabaseAppOpenHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.commcare.logging.DataChangeLogger;
import org.commcare.models.database.EncryptedDatabaseAdapter;
import org.commcare.models.database.DbUtil;
import org.javarosa.core.services.Logger;

import static org.commcare.models.database.app.AppDatabaseSchemaManager.DB_VERSION_APP;
import static org.commcare.models.database.app.AppDatabaseSchemaManager.getDbName;
Expand Down Expand Up @@ -44,6 +45,7 @@ public SQLiteDatabase getWritableDatabase() {
try {
return super.getWritableDatabase();
} catch (SQLiteException sqle) {
Logger.exception("Opening database failed", sqle);
Copy link
Contributor

@shubham1g5 shubham1g5 Sep 18, 2025

Choose a reason for hiding this comment

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

nit: it might be useful to know what database is failing here from the messag like "Opening App database failed"

Copy link
Contributor Author

@OrangeAndGreen OrangeAndGreen Sep 18, 2025

Choose a reason for hiding this comment

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

I think we'll be able to see that pretty easily from the stack trace, no? Like whether it comes from DatabaseAppOpenHelper, DatabaseConnectOpenHelper, etc.

Copy link
Contributor

@shubham1g5 shubham1g5 Sep 19, 2025

Choose a reason for hiding this comment

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

I am concerned that Crashlytics will club them together into one single issue or not as it might be valuable to know if this issue is happening for all Dbs or just one by getting the issue count. Although not sure whats the behaviour here on Crashlytics really, so fine to ignore it atm

DbUtil.trySqlCipherDbUpdate(key, context, getDbName(mAppId));
return super.getWritableDatabase();
}
Expand Down
2 changes: 2 additions & 0 deletions app/src/org/commcare/models/database/connect/DatabaseConnectOpenHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.commcare.util.Base64;
import org.commcare.util.Base64DecoderException;
import org.commcare.utils.CrashUtil;
import org.javarosa.core.services.Logger;

import java.io.File;

Expand Down Expand Up @@ -157,6 +158,7 @@ public SQLiteDatabase getWritableDatabase() {
try {
return super.getWritableDatabase();
} catch (SQLiteException sqle) {
Logger.exception("Opening database failed", sqle);
DbUtil.trySqlCipherDbUpdate(key, mContext, CONNECT_DB_LOCATOR);
try {
return super.getWritableDatabase();
Expand Down
2 changes: 2 additions & 0 deletions app/src/org/commcare/models/database/global/DatabaseGlobalOpenHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.commcare.logging.DataChangeLogger;
import org.commcare.models.database.DbUtil;
import org.commcare.models.database.EncryptedDatabaseAdapter;
import org.javarosa.core.services.Logger;

import static org.commcare.models.database.global.GlobalDatabaseSchemaManager.GLOBAL_DB_LOCATOR;
import static org.commcare.models.database.global.GlobalDatabaseSchemaManager.GLOBAL_DB_VERSION;
Expand Down Expand Up @@ -39,6 +40,7 @@ public SQLiteDatabase getWritableDatabase() {
try {
return super.getWritableDatabase();
} catch (SQLiteException sqle) {
Logger.exception("Opening database failed", sqle);
DbUtil.trySqlCipherDbUpdate(key, mContext, GLOBAL_DB_LOCATOR);
return super.getWritableDatabase();
}
Expand Down
2 changes: 2 additions & 0 deletions app/src/org/commcare/models/database/user/DatabaseUserOpenHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.commcare.logging.DataChangeLogger;
import org.commcare.models.database.EncryptedDatabaseAdapter;
import org.commcare.models.database.DbUtil;
import org.javarosa.core.services.Logger;

import static org.commcare.models.database.user.UserDatabaseSchemaManager.USER_DB_VERSION;
import static org.commcare.models.database.user.UserDatabaseSchemaManager.getDbName;
Expand Down Expand Up @@ -48,6 +49,7 @@ public SQLiteDatabase getWritableDatabase() {
try {
return super.getWritableDatabase();
} catch (SQLiteException sqle) {
Logger.exception("Opening database failed", sqle);
DbUtil.trySqlCipherDbUpdate(key, context, getDbName(mUserId));
return super.getWritableDatabase();
}
Expand Down
4 changes: 3 additions & 1 deletion app/src/org/commcare/utils/GlobalErrors.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

public enum GlobalErrors {
PERSONALID_GENERIC_ERROR(R.string.personalid_generic_error),
PERSONALID_LOST_CONFIGURATION_ERROR(R.string.recovery_network_token_request_rejected);
PERSONALID_LOST_CONFIGURATION_ERROR(R.string.recovery_network_token_request_rejected),
PERSONALID_DB_STARTUP_ERROR(R.string.personalid_generic_error),
PERSONALID_DB_UPGRADE_ERROR(R.string.personalid_generic_error);

final int messageId;

Expand Down
Loading