Added the button to copy stack trace and debug info

[Aditya13s]

Purpose / Description

The purpose of this pr is to add a button in Error Dialog to copy stack track and debug info.

Fixes

• Fixes #17232

Approach

The approach is to first pass the exception from InitialActivity to DeckPicker by converting the StartupFailure enum into a sealed class and adding an exception parameter to each class inside the StartupFailure sealed class. Then, in the DeckPicker class, a companion object is created to store the exception. Finally, in the DatabaseErrorDialog, the companion object is accessed and used in the copyStackTraceAndDebugInfo function.

How Has This Been Tested?

This change has been tested for database errors.

Screenshots

Checklist

• [x] You have a descriptive commit message with a short title (first line, max 50 chars).
• [x] You have commented your code, particularly in hard-to-understand areas
• [x] You have performed a self-review of your own code
• [x] UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
• [ ] UI Changes: You have tested your change using the Google Accessibility Scanner

[david-allison]

Tests are falling

[Aditya13s]

@david-allison all tests are passed

[Aditya13s]

@david-allison I've been analyzing the workflow of the DatabaseErrorDialog and encountered an issue. When I click on any option, a new instance of the dialog is created. In the first instance, I can access the exception, but when subsequent instances are created, the exception is no longer passed to the function.

I see two possible solutions to address this:

1. Companion Object: I could create an exception object in the companion object. If the exception is null, I would set it, which would allow access to the exception across all instances of the dialog.
2. Passing via Bundle: I could pass the exception to the first instance of the dialog using the showDatabaseErrorDialog() function in the DeckPicker class and store it in a bundle. Then, I could retrieve the exception in onCreateDialog() and pass it to all subsequent showDatabaseErrorDialog() calls present in the class, ensuring that each new instance contains the exception.

Which approach would be more appropriate?

[david-allison]

Not 1. Don't use mutable globals

[Aditya13s]

@david-allison please review the changes.

[Aditya13s]

Many way to improve the code. And, given that we are dealing with exception handling, with the time when AnkiDroid had an issue, I really want to be certain of the code we merge here, so sorry if I got to ask many changes or explanations.

This change has been tested for database errors.

I'm sorry but I don't understand what it means. In particular, I can't reproduce it.

Can you put a video instead of a picture, so that we can see what was actually copied (please paste the result somewhere so we can see it) or at least explain how to reproduce your testing.

https://github.com/user-attachments/assets/25a02817-eea0-433a-897f-f8152c0d2a86

[Aditya13s]

Exception Stack Trace

InitialActivity com.ichi2.anki.debug net.ankiweb.rsdroid.BackendException$BackendFatalError:
at com.ichi2.anki.CollectionManager$CollectionOpenFailure.triggerFailure(CollectionManager.kt:420)
at com.ichi2.anki.CollectionManager.ensureOpenInner(CollectionManager.kt:241)
at com.ichi2.anki.CollectionManager.getColUnsafe$lambda$11$lambda$10(CollectionManager.kt:299)
at com.ichi2.anki.CollectionManager.$r8$lambda$1xvLXT2AdUAF2dT9vOLsddKVcWo(Unknown Source:0)
at com.ichi2.anki.CollectionManager$$ExternalSyntheticLambda2.invoke(D8$$SyntheticClass:0)
at com.ichi2.anki.CollectionManager$withQueue$3.invokeSuspend(CollectionManager.kt:103)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
at kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(LimitedDispatcher.kt:111)
at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:99)
at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:584)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:811)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:715)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:702)

[Arthur-Milchior]

I'm very sorry, but your video starts with "Collection not opened". This does not tell me how to reproduce it. Unless you want me to manually put a wrong file instead of the database file.

[Aditya13s]

I'm very sorry, but your video starts with "Collection not opened". This does not tell me how to reproduce it. Unless you want me to manually put a wrong file instead of the database file.

• fix: handle BackendException$BackendFatalError on startup #17231

[Aditya13s]

@Arthur-Milchior please review the changes

[Arthur-Milchior]

I'm very sorry, but your video starts with "Collection not opened". This does not tell me how to reproduce it. Unless you want me to manually put a wrong file instead of the database file.

• fix: handle BackendException$BackendFatalError on startup #17231

I did look at the link, and see no reproduction step either. Please write sentences, explain what you mean here. This is not useful for me to reproduce

[Aditya13s]

I did look at the link, and see no reproduction step either. Please write sentences, explain what you mean here. This is not useful for me to reproduce

In CollectionManager.kt line 67

Replace this var emulatedOpenFailure: CollectionOpenFailure? = null

with this var emulatedOpenFailure: CollectionOpenFailure? = CollectionOpenFailure.FATAL_ERROR

[Arthur-Milchior]

Thank you for the reproduction step. On a next occasion, I'd really appreciate if you could give them directly when you create the PR.

Thanks also for applying the change I requested.

[Aditya13s]

@david-allison need second approval

[david-allison]

Please remove the merge commits from this, and rebase so we can rebase merge it via the merge queue

[Aditya13s]

@david-allison merge commit removed and rebased the changes.

[Aditya13s]

@david-allison All changes have been made as requested

[Aditya13s]

Could you add a quick unit test which transforms an exception into a CustomExceptionData with a few lenient assertions on the result? I'd want to make sure the exception message and class name is visible to the user

@david-allison I don't have much experience with unit testing. Could you help me with that?

[david-allison]

Subject: [PATCH] improvement(i18n): link 'Re-sync Translations' 

https://github.com/ankidroid/Anki-Android/actions/workflows/sync_translations.yml

Should save a few clicks
---
Index: AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DatabaseErrorDialog.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DatabaseErrorDialog.kt b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DatabaseErrorDialog.kt
--- a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DatabaseErrorDialog.kt	(revision 2923db6f276b1ec4cf9356d3efddf3b1012da477)
+++ b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/DatabaseErrorDialog.kt	(date 1733447690095)
@@ -405,9 +405,12 @@
      * and copies the combines information to the Android clipboard.
      */
     private suspend fun copyStackTraceAndDebugInfo() {
-        val stackTrace = exceptionData?.stackTrace
-        val debugInfo = DebugInfoService.getDebugInfo(requireContext())
-        val combinedInfo = stackTrace?.let { "$it\n$debugInfo" } ?: debugInfo
+        val combinedInfo = listOf(
+            exceptionData?.toHumanReadableString(),
+            DebugInfoService.getDebugInfo(requireContext())
+        )
+            .filterNotNull()
+            .joinToString(separator = "\n")
 
         requireContext().copyToClipboard(
             combinedInfo,
@@ -683,10 +686,12 @@
     class CustomExceptionData(
         val stackTrace: String
     ) : Parcelable {
+        fun toHumanReadableString() = stackTrace
 
         companion object {
             private const val MAX_STACKTRACE_LINES = 2000
 
+            @CheckResult
             fun fromException(exception: Exception): CustomExceptionData {
                 val stackTraceLines = exception.stackTraceToString().lines()
                 val truncatedStackTrace = if (stackTraceLines.size > MAX_STACKTRACE_LINES) {
Index: AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CustomExceptionDataTest.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CustomExceptionDataTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CustomExceptionDataTest.kt
new file mode 100644
--- /dev/null	(date 1733447566234)
+++ b/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/CustomExceptionDataTest.kt	(date 1733447566234)
@@ -0,0 +1,40 @@
+/*
+ *  Copyright (c) 2024 David Allison <[email protected]>
+ *
+ *  This program is free software; you can redistribute it and/or modify it under
+ *  the terms of the GNU General Public License as published by the Free Software
+ *  Foundation; either version 3 of the License, or (at your option) any later
+ *  version.
+ *
+ *  This program is distributed in the hope that it will be useful, but WITHOUT ANY
+ *  WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+ *  PARTICULAR PURPOSE. See the GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along with
+ *  this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package com.ichi2.anki.dialogs
+
+import com.ichi2.anki.dialogs.DatabaseErrorDialog.*
+import org.hamcrest.MatcherAssert.assertThat
+import org.hamcrest.Matchers.arrayWithSize
+import org.hamcrest.Matchers.containsString
+import org.hamcrest.Matchers.not
+import org.junit.Test
+
+/** Test of [DatabaseErrorDialog.CustomExceptionData] */
+class CustomExceptionDataTest {
+
+    @Test
+    fun `exception type and message is printed`() {
+        val exception = IllegalStateException("Java heap space")
+        assertThat("stack trace should exist", exception.stackTrace, not(arrayWithSize(0)))
+
+        val exceptionData = CustomExceptionData.fromException(exception)
+
+        val outputString = exceptionData.toHumanReadableString()
+
+        assertThat(outputString, containsString("IllegalStateException: Java heap space"))
+    }
+}
\ No newline at end of file

Other part of the patch is untested

[Aditya13s]

Thanks, @david-allison , for helping with the tests. I have a few questions: Why did you create a new function toHumanReadableString() instead of using the stackTrace variable directly? Also, what is the purpose of the @CheckResult annotation?

[david-allison]

If I were adding message to the data, the method call and tests wouldn't change, only the implementation would

@CheckResult - it's a pure function and we always want to use the result. Turn it into an IDE error if it's not used, and auto suggest creating a variable

[github-actions[bot]]

Hi there @Aditya13s! This is the OpenCollective Notice for PRs merged from 2024-12-01 through 2024-12-31

If you are interested in compensation for this work, the process with details is here:

https://github.com/ankidroid/Anki-Android/wiki/OpenCollective-Payment-Process#how-to-get-paid

[!IMPORTANT] PLEASE NOTE: The process was updated in August 2024. Re-read the Payment Process page if you have not already.

We only post one comment per person per month to avoid spamming you, regardless of the number of PRs merged, but this note applies to all PRs merged for this month

Please understand that our monthly budget is never guaranteed to cover all claims - the cap on payments-per-person may be lower, but we try to make our process as fair and transparent as possible, we just need your understanding.

Thanks!