From c333e1711953456a25cf8f8499ffac4e31d32dbb Mon Sep 17 00:00:00 2001 From: "Shane B." Date: Sun, 13 Sep 2026 17:37:17 +0200 Subject: [PATCH 1/2] Offer the log from the login screen, where failures were undebuggable The error report page - the one screen that can produce a log - was reachable from the map and the device list and from nowhere else. So a sign-in failure, the failure most worth diagnosing and the one a new user is most likely to meet, could hand them nothing. Their options were adb on a second machine or nothing, which in practice means the report arrives as "it does not work". An `Export logs` button now sits in the error box under the failure message and opens the report page on the failure that is showing. It is a separate activity, so closing it comes straight back with the error still up and the typed Apple ID still there - looking at the log costs nothing. The body text is its own string rather than the export one. A sign-in can fail because Apple declined, which is worth waiting out rather than reporting, and the page says so before it asks for a report - the distinction #177 drew on the screen, carried through to the page behind it. `rootOf` moves to `ErrorReportActivity` beside `describe`, because every caller of that needs it first and for a reason easy to miss: Rx wraps what a `map` throws, so describing the error as it arrives puts "RuntimeException" on the page and buries the sentence the reporter needs. Two screens keeping their own copy is two reports of one bug looking like two. Two tests: that a failed sign-in opens the page with the cause carried through - asserted on the extra, not merely on the component, since an intent opening a blank report would pass that - and that the button is not offered before anything has failed. --- .../opentagviewer/AppleLoginFlowTest.java | 47 +++++++++++++++++++ .../opentagviewer/AppleLoginActivity.java | 28 +++++++++++ .../opentagviewer/MyDevicesListActivity.java | 13 +---- .../ui/error/ErrorReportActivity.java | 16 +++++++ .../main/res/layout/activity_apple_login.xml | 42 +++++++++++++---- app/src/main/res/values-de/strings.xml | 4 ++ app/src/main/res/values-en/strings.xml | 4 ++ app/src/main/res/values-fr/strings.xml | 4 ++ app/src/main/res/values-ja/strings.xml | 4 ++ app/src/main/res/values-ko/strings.xml | 4 ++ app/src/main/res/values-nl/strings.xml | 4 ++ app/src/main/res/values-ru/strings.xml | 4 ++ app/src/main/res/values-zh-rCN/strings.xml | 4 ++ app/src/main/res/values-zh-rTW/strings.xml | 4 ++ app/src/main/res/values/strings.xml | 4 ++ 15 files changed, 167 insertions(+), 19 deletions(-) diff --git a/app/src/androidTest/java/dev/wander/android/opentagviewer/AppleLoginFlowTest.java b/app/src/androidTest/java/dev/wander/android/opentagviewer/AppleLoginFlowTest.java index f56a3231..7f4dd0df 100644 --- a/app/src/androidTest/java/dev/wander/android/opentagviewer/AppleLoginFlowTest.java +++ b/app/src/androidTest/java/dev/wander/android/opentagviewer/AppleLoginFlowTest.java @@ -25,6 +25,8 @@ import android.os.SystemClock; import androidx.test.core.app.ActivityScenario; +import static androidx.test.espresso.intent.matcher.IntentMatchers.hasExtra; +import static org.hamcrest.Matchers.allOf; import androidx.test.espresso.intent.Intents; import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.filters.LargeTest; @@ -42,6 +44,7 @@ import dev.wander.android.opentagviewer.db.repo.model.UserSettings; import dev.wander.android.opentagviewer.python.FakeAppleAuthService; import dev.wander.android.opentagviewer.python.AppDependencies; +import dev.wander.android.opentagviewer.ui.error.ErrorReportActivity; import dev.wander.android.opentagviewer.python.PythonAuthService.AuthMethodPhone; import dev.wander.android.opentagviewer.util.android.AppCryptographyUtil; @@ -342,6 +345,50 @@ public void appleDecliningIsNotBlamedOnThePasswordOrTheNetwork() { not(withText(containsString("Grand Slam"))))); } + /** + * A failed sign-in can produce a log without a second machine. + * + *

The report page was reachable from the map and the device list and from nowhere else, so + * the one screen whose failures most need diagnosing was the one screen that could not hand a + * user any evidence. Anyone whose sign-in failed had adb on a laptop or had nothing. + * + *

Asserted on the cause reaching the page rather than merely on the page opening: the + * point is that the report names the failure that was on screen, and an intent that opened a + * blank report would pass a check that only looked at the component. + */ + @Test + public void afailedSignInOffersTheLogWithoutADeveloperMachine() { + this.apple = FakeAppleAuthService.rejectsTheSignIn("Bad password"); + AppDependencies.replaceAuthService(this.apple); + + launch(); + signIn(); + + Eventually.check(() -> onView(withId(R.id.login_error_export_logs)) + .check(matches(isDisplayed()))); + onView(withId(R.id.login_error_export_logs)).perform(click()); + + Eventually.check(() -> intended(allOf( + hasComponent(ErrorReportActivity.class.getName()), + hasExtra(ErrorReportActivity.EXTRA_CAUSE, containsString("Bad password"))))); + } + + /** + * And it is only offered once there is something to report. + * + *

A button sitting under an empty error box invites a log of a sign-in that has not been + * attempted, which is a report with nothing in it. + */ + @Test + public void thelogButtonIsNotOfferedBeforeAnythingHasFailed() { + this.apple = FakeAppleAuthService.rejectsTheSignIn("Bad password"); + AppDependencies.replaceAuthService(this.apple); + + launch(); + + onView(withId(R.id.login_error_export_logs)).check(matches(not(isDisplayed()))); + } + /** A rejected code says so, and gives the boxes back rather than stranding them. */ @Test public void aWrongCodeIsReportedAndTheBoxesComeBack() { diff --git a/app/src/main/java/dev/wander/android/opentagviewer/AppleLoginActivity.java b/app/src/main/java/dev/wander/android/opentagviewer/AppleLoginActivity.java index 24045d64..421699d3 100644 --- a/app/src/main/java/dev/wander/android/opentagviewer/AppleLoginActivity.java +++ b/app/src/main/java/dev/wander/android/opentagviewer/AppleLoginActivity.java @@ -33,6 +33,7 @@ import androidx.core.os.LocaleListCompat; import androidx.databinding.DataBindingUtil; +import dev.wander.android.opentagviewer.ui.error.ErrorReportActivity; import dev.wander.android.opentagviewer.ui.compat.WindowPaddingUtil; import androidx.lifecycle.ViewModelProvider; @@ -264,6 +265,9 @@ public void handleOnBackPressed() { this.loginButton = this.findViewById(R.id.login_button_main); this.twoFactorAuthChoiceBackButton = this.findViewById(R.id.twofactorauthchoice_back_button); + this.findViewById(R.id.login_error_export_logs) + .setOnClickListener(v -> this.openTheReportPageForTheLoginFailure()); + this.findViewById(R.id.login_terms_agree_button) .setOnClickListener(v -> this.onAgreeToTerms()); @@ -698,6 +702,15 @@ private void onAgreeToTerms() { }); } + /** + * The failure the error box is currently showing, for the report page to name. + * + *

Kept rather than re-derived: {@link #describeLoginFailure} produces a sentence for a + * person, and the report page wants the exception - class and message - which is a different + * string and the one worth pasting into an issue. + */ + private Throwable lastLoginFailure; + /** Hand the sign-in step back to the user with the failure on it. */ private void showLoginFailure(final Throwable error) { this.hideLoading(); @@ -707,11 +720,26 @@ private void showLoginFailure(final Throwable error) { this.findViewById(R.id.password_input_field).setEnabled(true); this.findViewById(R.id.login_button_main).setClickable(true); + this.lastLoginFailure = error; this.findViewById(R.id.login_error_container).setVisibility(VISIBLE); ((TextView) this.findViewById(R.id.login_error_message_text)) .setText(this.describeLoginFailure(error)); } + /** + * Open the report page on the failure just shown. + * + *

A separate activity on purpose. Closing it returns here with the error box still + * up and the typed Apple ID still in place, so looking at the log costs the user nothing - + * which matters because the alternative was a second machine running adb. + */ + private void openTheReportPageForTheLoginFailure() { + this.startActivity(ErrorReportActivity.intentFor( + this, + ErrorReportActivity.describe(ErrorReportActivity.rootOf(this.lastLoginFailure)), + R.string.error_report_body_login)); + } + /** * What to put on screen when a sign-in fails. * diff --git a/app/src/main/java/dev/wander/android/opentagviewer/MyDevicesListActivity.java b/app/src/main/java/dev/wander/android/opentagviewer/MyDevicesListActivity.java index e8626c91..e89a06ef 100644 --- a/app/src/main/java/dev/wander/android/opentagviewer/MyDevicesListActivity.java +++ b/app/src/main/java/dev/wander/android/opentagviewer/MyDevicesListActivity.java @@ -583,19 +583,10 @@ private void onBundleExportFailed(final Throwable error) { // **The cause, not the wrapper.** Rx wraps what a `map` throws, so `describe(error)` here // would put "RuntimeException" on the page and bury the sentence the reporter needs. this.startActivity(ErrorReportActivity.intentFor( - this, ErrorReportActivity.describe(rootOf(error)), + this, ErrorReportActivity.describe(ErrorReportActivity.rootOf(error)), R.string.error_report_body_export)); } - /** The innermost cause, which is the one that says what actually happened. */ - private static Throwable rootOf(final Throwable error) { - Throwable cause = error; - while (cause.getCause() != null && cause.getCause() != cause) { - cause = cause.getCause(); - } - return cause; - } - private void exportHistoryForSelection() { this.pendingExport = this.deviceListAdaptor.getSelectedBeacons(); @@ -744,7 +735,7 @@ private void historyImportFailed(@NonNull final Throwable error) { || failure.getReason() == HistoryImportException.Reason.UNEXPECTED) { this.startActivity(ErrorReportActivity.intentFor( this, - ErrorReportActivity.describe(rootOf(error)), + ErrorReportActivity.describe(ErrorReportActivity.rootOf(error)), R.string.error_report_body_history_import)); return; } diff --git a/app/src/main/java/dev/wander/android/opentagviewer/ui/error/ErrorReportActivity.java b/app/src/main/java/dev/wander/android/opentagviewer/ui/error/ErrorReportActivity.java index dc0d4e8c..46aa8bce 100644 --- a/app/src/main/java/dev/wander/android/opentagviewer/ui/error/ErrorReportActivity.java +++ b/app/src/main/java/dev/wander/android/opentagviewer/ui/error/ErrorReportActivity.java @@ -93,6 +93,22 @@ public static Intent intentFor( * exactly this string and two of them writing it slightly differently makes two reports of * one bug look like two bugs. */ + /** + * The innermost cause, which is the one that says what actually happened. + * + *

Here beside {@link #describe} because every caller of that needs this first, and for a + * reason that is easy to miss: Rx wraps what a {@code map} throws, so describing the error as + * it arrives puts "RuntimeException" on the page and buries the sentence the reporter needs. + * Two screens writing their own copy of this is two reports of one bug looking like two. + */ + public static Throwable rootOf(final Throwable error) { + Throwable cause = error; + while (cause != null && cause.getCause() != null && cause.getCause() != cause) { + cause = cause.getCause(); + } + return cause; + } + public static String describe(final Throwable error) { if (error == null) { return "unknown"; diff --git a/app/src/main/res/layout/activity_apple_login.xml b/app/src/main/res/layout/activity_apple_login.xml index 6ea5bd97..9964a512 100644 --- a/app/src/main/res/layout/activity_apple_login.xml +++ b/app/src/main/res/layout/activity_apple_login.xml @@ -131,16 +131,42 @@ android:visibility="invisible" tools:visibility="visible"> - + android:orientation="vertical"> + + + + +