Merge branch 'chore/review-sentry-logs' into 'develop'

Reviews the logs and introduces exception filtering

See merge request android/mail/proton-mail-android!1053
This commit is contained in:
Maciej Surmacz 2022-04-14 10:54:37 +00:00
commit 1e573c0c1a
25 changed files with 71 additions and 54 deletions

View File

@ -747,7 +747,7 @@ public class ComposeMessageActivity
ArrayList<String> emails = (ArrayList<String>) intent.getSerializableExtra(Intent.EXTRA_EMAIL);
addStringRecipientsToView(emails, toRecipientView);
} catch (Exception e) {
Timber.e(e, "Extract mail to getting extra email");
Timber.d(e, "Failed extracting recipients from the intent");
}
}
}
@ -783,7 +783,7 @@ public class ComposeMessageActivity
ArrayList<String> emails = (ArrayList<String>) intent.getSerializableExtra(Intent.EXTRA_EMAIL);
addStringRecipientsToView(emails, toRecipientView);
} catch (Exception e) {
Timber.w(e, "Extract mail to getting extra email");
Timber.d(e, "Failed extracting recipients from the intent");
}
}
}
@ -815,7 +815,7 @@ public class ComposeMessageActivity
try {
extractMailTo(intent);
} catch (Exception e) {
Timber.w(e, "Handle set text: extracting email");
Timber.d(e, "Handle set text: extracting email");
}
handleSendFileUri(uri);
}
@ -1282,7 +1282,7 @@ public class ComposeMessageActivity
composeMessageViewModel.setAttachmentList(listToSet);
afterAttachmentsAdded();
} else {
Timber.w("ComposeMessageAct.onActivityResult Received result not handled. \n" +
Timber.d("ComposeMessageAct.onActivityResult Received result not handled. \n" +
"Request code = %s\n" +
"Result code = %s", requestCode, resultCode);
super.onActivityResult(requestCode, resultCode, data);
@ -1931,7 +1931,7 @@ public class ComposeMessageActivity
sendingToast = R.string.sending_message_offline;
}
if(dbId == null){
Timber.w("Error while saving message. DbId is null.");
Timber.d("Error while saving message. DbId is null.");
TextExtensions.showToast(ComposeMessageActivity.this, R.string.error_saving_try_again);
} else {
TextExtensions.showToast(ComposeMessageActivity.this, sendingToast);

View File

@ -96,7 +96,7 @@ class NetworkConfigurator @Inject constructor(
val result: List<String>? = try {
provider.getAlternativeBaseUrls()
} catch (ioException: IOException) {
Timber.w(ioException, "DoH getAlternativeBaseUrls error!")
Timber.d(ioException, "DoH getAlternativeBaseUrls error!")
null
}
if (result != null) {

View File

@ -122,10 +122,10 @@ class AttachmentsRepository @Inject constructor(
)
}
} catch (exception: IOException) {
Timber.e("Upload attachment failed: $exception")
Timber.d("Upload attachment failed: $exception")
return@withContext Result.Failure("Upload attachment request failed")
} catch (cancellationException: CancellationException) {
Timber.w("Upload attachment was cancelled. $cancellationException. Rethrowing")
Timber.d("Upload attachment was cancelled. $cancellationException. Rethrowing")
throw cancellationException
} catch (exception: Exception) {
Timber.w("Upload attachment failed throwing generic exception: $exception")

View File

@ -101,7 +101,6 @@ class HandleSingleAttachment @Inject constructor(
)
)
} else {
Timber.w("handleSingleAttachment failure")
AppUtil.postEventOnUi(
DownloadedAttachmentEvent(
Status.FAILED, filenameInCache, null, attachment.attachmentId, messageId, false
@ -136,7 +135,7 @@ class HandleSingleAttachment @Inject constructor(
}
} catch (exception: IOException) {
if (runAttemptCount >= MAX_RETRY_ATTEMPTS) {
Timber.w(exception, "Unable to download attachment file $filename retry has failed")
Timber.d(exception, "Unable to download attachment file $filename retry has failed")
runAttemptCount = 0
return null
} else {
@ -224,7 +223,7 @@ class HandleSingleAttachment @Inject constructor(
arrayOf(mimeType),
callback
)
continuation.invokeOnCancellation { Timber.w("Attachment Uri resolution cancelled") }
continuation.invokeOnCancellation { Timber.d("Attachment Uri resolution cancelled") }
}
}

View File

@ -219,10 +219,7 @@ class UploadAttachmentsWorker @AssistedInject constructor(
exception: Throwable? = null,
messageId: String?
): ListenableWorker.Result {
Timber.e(
"UploadAttachments Worker failed permanently for " +
"$messageId. error = $error, exception = $exception. FAILING"
)
Timber.e(exception, "UploadAttachments Worker failed permanently for $messageId. error = $error. FAILING")
val errorData = workDataOf(
KEY_OUTPUT_RESULT_UPLOAD_ATTACHMENTS_ERROR to error,
)

View File

@ -291,7 +291,7 @@ class FetchContactsMapper @Inject constructor(
try {
crypto.verify(decryptedVCardType2, vCardType2Signature).isSignatureValid
} catch (exception: GeneralSecurityException) {
Timber.w(exception, "VCard type2 verification error")
Timber.d(exception, "VCard type2 verification error")
false
}
} else {
@ -303,7 +303,7 @@ class FetchContactsMapper @Inject constructor(
try {
crypto.verify(decryptedVCardType3, vCardType3Signature).isSignatureValid
} catch (exception: GeneralSecurityException) {
Timber.w(exception, "VCard type3 verification error")
Timber.d(exception, "VCard type3 verification error")
false
}
} else {

View File

@ -69,7 +69,7 @@ class ContactGroupEditCreateRepository @Inject constructor(
enqueueCreateContactGroupWorker(contactLabel, true)
}
else -> {
Timber.w("updateLabel failure $updateLabelResult")
Timber.d("updateLabel failure $updateLabelResult")
}
}
return updateLabelResult
@ -176,7 +176,7 @@ class ContactGroupEditCreateRepository @Inject constructor(
enqueueCreateContactGroupWorker(contactLabel, false)
}
else -> {
Timber.w("createContactGroup failure $createLabelResult")
Timber.d("createContactGroup failure $createLabelResult")
}
}

View File

@ -195,7 +195,7 @@ class ContactGroupEditCreateViewModel @Inject constructor(
)
}
else -> {
Timber.w("editContactGroup failure $editContactResult")
Timber.d("editContactGroup failure $editContactResult")
}
}
}
@ -239,7 +239,7 @@ class ContactGroupEditCreateViewModel @Inject constructor(
)
}
else -> {
Timber.w("createGroup failure $createGroupResponse")
Timber.d("createGroup failure $createGroupResponse")
}
}
}

View File

@ -20,6 +20,7 @@ package ch.protonmail.android.core
import android.os.Build
import android.util.Log
import ch.protonmail.android.data.remote.OfflineException
import ch.protonmail.android.domain.entity.EmailAddress
import ch.protonmail.android.utils.AppUtil
import ch.protonmail.android.utils.extensions.obfuscateEmail
@ -27,6 +28,12 @@ import io.sentry.Sentry
import io.sentry.SentryEvent
import io.sentry.protocol.Message
import timber.log.Timber
import java.net.ConnectException
import java.net.SocketException
import java.net.SocketTimeoutException
import java.net.UnknownHostException
import javax.net.ssl.SSLException
import kotlin.coroutines.cancellation.CancellationException
/**
* Production variant of [Timber.Tree]
@ -39,11 +46,13 @@ internal class SentryTree : Timber.Tree() {
override fun isLoggable(tag: String?, priority: Int): Boolean = priority >= Log.WARN
override fun log(priority: Int, tag: String?, message: String, t: Throwable?) {
override fun log(priority: Int, tag: String?, message: String, throwable: Throwable?) {
if (throwable.shouldBeIgnored()) return
val event = SentryEvent().apply {
tag?.let { setTag(TAG_LOG, it) }
if (t is DetailedException) {
setExtras(t.extras)
if (throwable is DetailedException) {
setExtras(throwable.extras)
}
setMessage(obfuscatedMessage(message))
setTag(TAG_APP_VERSION, AppUtil.getAppVersion())
@ -62,7 +71,20 @@ internal class SentryTree : Timber.Tree() {
it.value.obfuscateEmail()
}
private fun Throwable?.shouldBeIgnored() = when (this) {
is CancellationException,
is UnknownHostException,
is SocketTimeoutException,
is SSLException,
is ConnectException,
is SocketException,
is OfflineException -> true
else -> false
}
private companion object {
private const val TAG_LOG = "LOG_TAG"
private const val TAG_APP_VERSION = "APP_VERSION"
private const val TAG_SDK_VERSION = "SDK_VERSION"

View File

@ -208,7 +208,7 @@ class UserManager @Inject constructor(
get() = currentUserId?.let(::preferencesFor)
private inline fun <T> withCurrentUserPreferences(block: (SharedPreferences) -> T): T? {
currentUserPreferences ?: Timber.e("No current user set")
currentUserPreferences ?: Timber.d("No current user set")
return currentUserPreferences?.let(block)
}

View File

@ -40,7 +40,7 @@ class FetchByLocationJob(
val userId = userId
?: run {
Timber.w("Can't fetch messages without any logged in user")
Timber.d("Can't fetch messages without any logged in user")
return
}

View File

@ -46,7 +46,7 @@ class PostStarJob(private val messageIds: List<String>) : ProtonMailEndlessJob(
runBlocking {
val message = getMessageDetailsRepository().findMessageById(messageId).firstOrNull()
if (message == null) {
Timber.w("Trying to star message which was not found in the DB. messageId = $messageId")
Timber.d("Trying to star message which was not found in the DB. messageId = $messageId")
return@runBlocking
}

View File

@ -39,7 +39,7 @@ class PostUnstarJob(private val messageIds: List<String>) : ProtonMailEndlessJob
messageIds.forEach { messageId ->
val message = runBlocking { getMessageDetailsRepository().findMessageById(messageId).firstOrNull() }
if (message == null) {
Timber.w("Trying to unstar a message which was not found in the DB. messageId = $messageId")
Timber.d("Trying to unstar a message which was not found in the DB. messageId = $messageId")
return
}

View File

@ -99,7 +99,7 @@ class ParentFolderPickerViewModel @Inject constructor(
is ParentFolderPickerState.Loading -> prevState.copy(selectedItemId = folderId)
is ParentFolderPickerState.Editing -> prevState.updateSelectedItem(folderId)
is ParentFolderPickerState.SavingAndClose -> {
Timber.w("Previous state is 'SavingAndClose', ignoring the current change")
Timber.d("Previous state is 'SavingAndClose', ignoring the current change")
prevState
}
}

View File

@ -248,7 +248,7 @@ internal class ConversationsRepositoryImpl @Inject constructor(
conversationIds.forEach forEachConversation@{ conversationId ->
val conversation = conversationDao.findConversation(userId.id, conversationId)
if (conversation == null) {
Timber.e("Conversation with id $conversationId could not be found in DB")
Timber.d("Conversation with id $conversationId could not be found in DB")
return ConversationsActionResult.Error
}
conversationDao.updateNumUnreadMessages(conversationId, conversation.numUnread + 1)
@ -402,7 +402,7 @@ internal class ConversationsRepositoryImpl @Inject constructor(
val conversation = conversationDao.findConversation(userId.id, conversationId)
if (conversation == null) {
Timber.e("Conversation with id $conversationId could not be found in DB")
Timber.d("Conversation with id $conversationId could not be found in DB")
return ConversationsActionResult.Error
}
val labelsToRemoveFromConversation = getLabelIdsForRemovingWhenMovingToFolder(
@ -716,7 +716,7 @@ internal class ConversationsRepositoryImpl @Inject constructor(
): ConversationsActionResult {
val conversation = conversationDao.findConversation(userId.id, conversationId)
if (conversation == null) {
Timber.e("Conversation with id $conversationId could not be found in DB")
Timber.d("Conversation with id $conversationId could not be found in DB")
return ConversationsActionResult.Error
}
val newLabels = mutableListOf<LabelContextDatabaseModel>()
@ -747,7 +747,7 @@ internal class ConversationsRepositoryImpl @Inject constructor(
): ConversationsActionResult {
val conversation = conversationDao.findConversation(userId.id, conversationId)
if (conversation == null) {
Timber.e("Conversation with id $conversationId could not be found in DB")
Timber.d("Conversation with id $conversationId could not be found in DB")
return ConversationsActionResult.Error
}
val labels = conversation.labels.toMutableList()

View File

@ -79,7 +79,7 @@ class DeleteConversationsRemoteWorker @AssistedInject constructor(
throw throwable
}
if (runAttemptCount > MAX_RUN_ATTEMPTS) {
Timber.e(throwable)
Timber.e(throwable, "Permanently failing DeleteConversationsRemoteWorker")
Result.failure(
workDataOf(KEY_LABEL_WORKER_ERROR_DESCRIPTION to "Run attempts exceeded the limit")
)

View File

@ -397,7 +397,7 @@ internal class MailboxActivity :
showToast(it.message.toString(), Toast.LENGTH_LONG)
}
is GetMailSettings.MailSettingsState.Error.NoPrimaryAccount -> {
Timber.w("No Primary Account")
Timber.d("No Primary Account")
}
is GetMailSettings.MailSettingsState.Success -> {
it.mailSettings?.let { mailSettings ->
@ -697,12 +697,12 @@ internal class MailboxActivity :
multiUserFcmTokenManager.saveTokenBlocking(FirebaseToken(task.result!!.token))
registerDeviceWorkerEnqueuer()
} else {
Timber.e(task.exception, "Could not retrieve FirebaseInstanceId")
Timber.d(task.exception, "Could not retrieve FirebaseInstanceId")
}
}
}.onFailure {
showToast(R.string.invalid_firebase_api_key_message)
Timber.e(it, getString(R.string.invalid_firebase_api_key_message))
Timber.d(it, "Invalid Firebase API key. Push notifications will not work.")
}
}
}

View File

@ -152,7 +152,7 @@ class MessageRepository @Inject constructor(
return if (currentUser != null) {
findMessage(currentUser, messageId)
} else {
Timber.w("Cannot find message for null user id")
Timber.d("Cannot find message for null user id")
null
}
}

View File

@ -93,7 +93,7 @@ public class AttachmentClearingService extends ProtonJobIntentService {
else
userId = userManager.getCurrentUserId();
if (userId == null) {
Timber.w("No user id provided and no user currently logged in");
Timber.d("No user id provided and no user currently logged in");
return;
}

View File

@ -216,7 +216,7 @@ internal class MessageActionSheetViewModel @Inject constructor(
)
}
} else {
Timber.e("Primary user id is null. Cannot star message/conversation")
Timber.d("Primary user id is null. Cannot star message/conversation")
}
}.invokeOnCompletion { cancellationException ->
if (cancellationException != null) {
@ -256,7 +256,7 @@ internal class MessageActionSheetViewModel @Inject constructor(
)
}
} else {
Timber.e("Primary user id is null. Cannot unstar message/conversation")
Timber.d("Primary user id is null. Cannot unstar message/conversation")
}
}.invokeOnCompletion { cancellationException ->
if (cancellationException != null) {
@ -300,7 +300,7 @@ internal class MessageActionSheetViewModel @Inject constructor(
)
}
} else {
Timber.e("Primary user id is null. Cannot mark message/conversation unread")
Timber.d("Primary user id is null. Cannot mark message/conversation unread")
}
}.invokeOnCompletion { cancellationException ->
if (cancellationException != null) {
@ -342,7 +342,7 @@ internal class MessageActionSheetViewModel @Inject constructor(
)
}
} else {
Timber.e("Primary user id is null. Cannot mark message/conversation read")
Timber.d("Primary user id is null. Cannot mark message/conversation read")
}
}.invokeOnCompletion { cancellationException ->
if (cancellationException != null) {
@ -409,7 +409,7 @@ internal class MessageActionSheetViewModel @Inject constructor(
cancel("Could not complete the action")
}
} else {
Timber.e("Primary user id is null. Cannot move message/conversation to folder")
Timber.d("Primary user id is null. Cannot move message/conversation to folder")
}
} else {
Timber.v("Move message to folder: $newFolderLocationId")

View File

@ -72,7 +72,7 @@ class SaveDraft @Inject constructor(
val addressCrypto = addressCryptoFactory.create(userId, AddressId(addressId))
val encryptedBody = addressCrypto.encrypt(message.decryptedBody ?: "", true).armored
if (message.decryptedBody == null) {
Timber.w("Save Draft for messageId $messageId - Decrypted Body was null, proceeding...")
Timber.d("Save Draft for messageId $messageId - Decrypted Body was null, proceeding...")
}
message.messageBody = encryptedBody
@ -113,7 +113,7 @@ class SaveDraft @Inject constructor(
return@map uploadAttachments(params, createdDraftId, localDraft)
} else {
Timber.e("Save Draft to API for messageId $localDraftId FAILED.")
Timber.d("Save Draft to API for messageId $localDraftId FAILED.")
val saveDraftError = workInfo?.outputData?.getString(KEY_OUTPUT_RESULT_SAVE_DRAFT_ERROR_ENUM)
saveDraftError?.let { errorName ->

View File

@ -57,7 +57,7 @@ public class HTMLToMDConverter {
try {
return remark.convert(html).replaceAll(" *\n", "\n");
} catch (StackOverflowError exception) {
Timber.w(exception);
Timber.d(exception);
}
try {
// the workaround below is for slow performance devices which Remark throws stack overflow exception
@ -65,7 +65,7 @@ public class HTMLToMDConverter {
TextContentRenderer renderer = TextContentRenderer.builder().build();
return renderer.render(parser.parse(html));
} catch (Exception exception) {
Timber.w(exception);
Timber.d(exception);
}
return "";
}

View File

@ -47,11 +47,10 @@ abstract class ExpandableRecyclerAdapter<T : ExpandableRecyclerAdapter.ListItem>
override fun getItemCount() = visibleItems.size
fun getItem(i: Int): T? {
return try {
visibleItems[i]
} catch (e: Exception) {
Timber.w(e, e.localizedMessage)
return if (i < 0 || i >= visibleItems.size) {
null
} else {
visibleItems[i]
}
}

View File

@ -134,7 +134,7 @@ class ParentFolderPickerViewModelTest : CoroutinesTest {
viewModel.state.test {
assertEquals(expectedState, awaitItem())
verify { mockTimberTree.w("Previous state is 'SavingAndClose', ignoring the current change") }
verify { mockTimberTree.d("Previous state is 'SavingAndClose', ignoring the current change") }
}
}

View File

@ -244,7 +244,7 @@ class SaveDraftTest : CoroutinesTest {
// Then
val messageCaptor = slot<Message>()
verify {
Timber.w("Save Draft for messageId 456 - Decrypted Body was null, proceeding...")
Timber.d("Save Draft for messageId 456 - Decrypted Body was null, proceeding...")
}
coVerify { messageDetailsRepository.saveMessage(capture(messageCaptor)) }
assertEquals("encrypted empty message body", messageCaptor.captured.messageBody)