Stop asking for a bug report when Apple is just declining

app/build.gradle.kts

@@ -407,7 +407,7 @@ chaquopy {
             // wheel for desktop platforms and a pure-Python `py3-none-any` one as well.
             // There is no Android wheel, so pip falls back to the pure-Python build - which
             // is correct but markedly slower. The messages here are small enough not to care.
-            install("git+https://github.com/parawanderer/FindMy.py@ddc7f2342fc9f32ebe315b85c22a4554ce419f6d")
+            install("git+https://github.com/parawanderer/FindMy.py@3c2b4926252193e9cd265b39fa52252adcbaad4e")
 
             install("NSKeyedUnArchiver==1.5")
 

app/src/androidTest/java/dev/wander/android/opentagviewer/AppleLoginFlowTest.java

@@ -13,6 +13,7 @@
 import static androidx.test.espresso.matcher.ViewMatchers.withText;
 import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -309,6 +310,38 @@ public void anunrecognisedFailureStillShowsItsDetail() {
                 .check(matches(withText(containsString("Bad password")))));
     }
 
+    /**
+     * Apple declining is told apart from a wrong password and from a dead network.
+     *
+     * <p><b>Issue #176.</b> The screen echoed the failure verbatim, so somebody entering correct
+     * credentials during a Grand Slam outage was shown a sentence about HTTP 503 and an Apple
+     * internal service name. It reads as a bug in this app, and was filed as one.
+     *
+     * <p>Asserted on all three halves, because each is a different wrong turn: that it does not
+     * blame the password, that it does not send them to check a working connection, and that the
+     * raw protocol text is gone from the screen.
+     */
+    @Test
+    public void appleDecliningIsNotBlamedOnThePasswordOrTheNetwork() {
+        this.apple = FakeAppleAuthService.appleIsDeclining();
+        AppDependencies.replaceAuthService(this.apple);
+
+        launch();
+        signIn();
+
+        final android.content.Context context = getInstrumentation().getTargetContext();
+
+        Eventually.check(() -> onView(withId(R.id.login_error_message_text)).check(matches(
+                withText(context.getString(R.string.login_failed_apple_declined)))));
+
+        onView(withId(R.id.login_error_message_text)).check(matches(
+                not(withText(context.getString(R.string.login_failed_network)))));
+        onView(withId(R.id.login_error_message_text)).check(matches(
+                not(withText(containsString("503")))));
+        onView(withId(R.id.login_error_message_text)).check(matches(
+                not(withText(containsString("Grand Slam")))));
+    }
+
     /** A rejected code says so, and gives the boxes back rather than stranding them. */
     @Test
     public void aWrongCodeIsReportedAndTheBoxesComeBack() {

app/src/androidTest/java/dev/wander/android/opentagviewer/FetchFromICloudFlowTest.java

@@ -353,6 +353,35 @@ public void aserviceHavingABadDayOffersARetryInstead() {
         TestPace.afterAStep();
     }
 
+    /**
+     * Apple declining reaches the retry screen, saying which of the two it is.
+     *
+     * <p><b>Issue #176.</b> Before this it fell through to the default branch, which puts the
+     * failure's own detail on screen - an HTTP status and Apple's internal service name. Correct,
+     * unreadable, and indistinguishable from a bug in this app.
+     *
+     * <p>Asserted on the raw text being absent as well as the sentence being present, because a
+     * branch that showed both would pass a check for only the second.
+     */
+    @Test
+    public void appleDecliningSaysSoRatherThanShowingTheStatusCode() {
+        this.open(FakeICloudService.whereAppleIsDeclining());
+
+        final android.content.Context context =
+                androidx.test.platform.app.InstrumentationRegistry
+                        .getInstrumentation().getTargetContext();
+
+        Eventually.check(() -> onView(withId(R.id.icloud_retry_container))
+                .check(matches(isDisplayed())));
+        Eventually.check(() -> onView(withId(R.id.icloud_retry_body)).check(matches(
+                withText(context.getString(R.string.icloud_apple_declined_body)))));
+
+        onView(withId(R.id.icloud_retry_body)).check(matches(
+                not(withText(org.hamcrest.Matchers.containsString("503")))));
+        onView(withId(R.id.icloud_no_tags_container)).check(matches(not(isDisplayed())));
+        TestPace.afterAStep();
+    }
+
     /** Retrying starts a fresh session rather than reusing the one that failed. */
     @Test
     public void retryingAsksAgain() {

app/src/androidTest/java/dev/wander/android/opentagviewer/python/FakeAppleAuthService.java

@@ -118,6 +118,24 @@ public static FakeAppleAuthService cannotReachApple() {
         return fake;
     }
 
+    /**
+     * Apple answered and refused to serve, which is neither a wrong password nor a dead network.
+     *
+     * <p>Its own state because the message is the giveaway: the real one is a sentence about the
+     * Grand Slam request and an HTTP status, which the screen used to echo verbatim. That reads
+     * as a bug in this app, and issue #176 is somebody reporting it as one.
+     */
+    public static FakeAppleAuthService appleIsDeclining() {
+        final FakeAppleAuthService fake =
+                new FakeAppleAuthService(LOGIN_STATE.LOGGED_OUT, null);
+        fake.loginFailsWith = new PythonAccountLoginException(
+                "The Grand Slam request was refused with HTTP 503. This is Apple declining to"
+                        + " serve the request rather than a response this library cannot read,"
+                        + " and it usually clears on its own -- wait and try again.",
+                PythonAccountLoginException.REASON_APPLE_DECLINED);
+        return fake;
+    }
+
     /** Signing in works, but the code that gets typed is refused. */
     public FakeAppleAuthService thatRejectsTheCode(final String message) {
         this.codeFailsWith = new PythonAccountLoginException(message);

app/src/androidTest/java/dev/wander/android/opentagviewer/python/icloud/FakeICloudService.java

@@ -139,6 +139,22 @@ public static FakeICloudService whereTheServiceIsUnsure() {
         return fake;
     }
 
+    /**
+     * Apple answered and refused to serve, rather than reporting nothing usable.
+     *
+     * <p>Kept apart from {@link #whereTheServiceIsUnsure()} because both reach the retry screen
+     * and they are not the same fact. That one is the keychain service having a bad day; this is
+     * an endpoint returning a 5xx, and it was reaching the screen as UNKNOWN with the HTTP
+     * status on it - issue #176.
+     */
+    public static FakeICloudService whereAppleIsDeclining() {
+        final FakeICloudService fake = new FakeICloudService();
+        fake.optionsFailsWith = new ICloudException(
+                ICloudFailure.APPLE_DECLINED,
+                "The Grand Slam request was refused with HTTP 503.");
+        return fake;
+    }
+
     /** An account with a Mac on it and no tags - the empty fetch, one step later. */
     public static FakeICloudService withNoTagsOnTheAccount() {
         final FakeICloudService fake = new FakeICloudService();

app/src/main/java/dev/wander/android/opentagviewer/AppleLoginActivity.java

@@ -734,6 +734,13 @@ private String describeLoginFailure(final Throwable error) {
             return this.getString(R.string.login_failed_network);
         }
 
+        // **Before the fallback, which would show "The Grand Slam request was refused with HTTP
+        // 503" verbatim.** That is accurate and reads as a bug in this app, which is how issue
+        // #176 came to be filed. Nothing about the Apple ID or the password is wrong here.
+        if (PythonAccountLoginException.REASON_APPLE_DECLINED.equals(reason)) {
+            return this.getString(R.string.login_failed_apple_declined);
+        }
+
         // Reached when the terms path was tried and produced nothing to accept, so the sentence
         // says what Apple said and then that accepting terms will not fix something else -
         // rather than asserting a cause that has not been established.

app/src/main/java/dev/wander/android/opentagviewer/FetchFromICloudActivity.java

@@ -698,6 +698,16 @@ private void showFailure(final Throwable error) {
                 SignInAgain.from(this);
                 break;
 
+            case APPLE_DECLINED:
+                // The retry screen, like SERVICE_UNSURE, but saying which of the two it is.
+                // Falling through to `default` would put the raw detail on screen - an HTTP
+                // status and a sentence about Grand Slam - which reads as a bug here.
+                this.showOnly(R.id.icloud_retry_container,
+                        R.string.icloud_apple_declined_title, Direction.FORWARD);
+                ((TextView) this.findViewById(R.id.icloud_retry_body))
+                        .setText(R.string.icloud_apple_declined_body);
+                break;
+
             case SERVICE_UNSURE:
             default:
                 // Everything unrecognised lands here on purpose: "try again later" is the safe

app/src/main/java/dev/wander/android/opentagviewer/python/PythonAccountLoginException.java

@@ -34,6 +34,14 @@ public class PythonAccountLoginException extends RuntimeException {
     public static final String REASON_TERMS = "terms";
 
     /** Anything not recognised. The detail is shown as-is rather than guessed at. */
+    /**
+     * Apple answered and refused to serve. Matches {@code REASON_APPLE_DECLINED}.
+     *
+     * <p>Kept apart from {@link #REASON_NETWORK} because the advice differs: a network failure is
+     * usually the phone's and worth checking, and this one is Apple's and is not.
+     */
+    public static final String REASON_APPLE_DECLINED = "apple_declined";
+
     public static final String REASON_UNKNOWN = "unknown";
 
     private final String reason;

app/src/main/java/dev/wander/android/opentagviewer/python/icloud/ICloudFailure.java

@@ -100,6 +100,21 @@ public enum ICloudFailure {
      */
     NOT_AN_ACCESSORY,
 
+    /**
+     * Apple answered and refused to serve. Worth waiting out.
+     *
+     * <p><b>Not {@link #CREDENTIALS_REJECTED} and not {@link #UNKNOWN}, and the difference is
+     * expensive in both directions.</b> Treated as rejected credentials it signs somebody out
+     * over a 503, destroying a working session for nothing. Left unknown it reaches a screen
+     * showing an HTTP status and a sentence about Grand Slam, which reads as a bug in this app -
+     * see issue #176, where that is what somebody reported.
+     *
+     * <p>Closest to {@link #SERVICE_UNSURE}, which also means "try later", and kept apart
+     * because that one is about the keychain service reporting nothing usable rather than about
+     * an endpoint declining.
+     */
+    APPLE_DECLINED,
+
     /** Anything else. The detail carries what there is to say. */
     UNKNOWN;
 
@@ -123,6 +138,7 @@ public static ICloudFailure fromWire(final String reason) {
             case "no_such_record": return NO_SUCH_RECORD;
             case "not_unlocked": return NOT_UNLOCKED;
             case "credentials_rejected": return CREDENTIALS_REJECTED;
+            case "apple_declined": return APPLE_DECLINED;
             case "membership_unusable": return MEMBERSHIP_UNUSABLE;
             case "no_such_accessory": return NO_SUCH_ACCESSORY;
             case "not_an_accessory": return NOT_AN_ACCESSORY;

app/src/main/java/dev/wander/android/opentagviewer/util/rx/ACodeAppleAlreadyTook.java

@@ -61,15 +61,25 @@ private ACodeAppleAlreadyTook() {
      * code, which is inconvenient and correct.
      *
      * <p>Matched on the name because that is what survives the bridge: the failure arrives from
-     * Chaquopy as a {@code PyException} whose message carries the Python class. FindMy.py folds
-     * every non-OK status into {@code UnhandledProtocolError} with only the number in it, so
-     * there is nothing better to match on until the fork grows a transient-failure type - see the
-     * handover note, which explains why that was left out of the first fix.
+     * Chaquopy as a {@code PyException} whose message carries the Python class.
+     *
+     * <p><b>Two names, and the second one is why this had to change.</b> FindMy.py used to fold
+     * every non-OK status into {@code UnhandledProtocolError}; the fork now raises
+     * {@code AppleServiceUnavailableError} for a 429 or a 5xx, which is a subclass and therefore
+     * invisible to a match on the parent's name. Matching only the old name would have made this
+     * quietly stop firing the moment the pin moved - the recovery would still be here, still
+     * tested, and never reached, because every test in this file writes the old name into its
+     * own fixture.
+     *
+     * <p>The old name is kept alongside it. It still covers a protocol failure after the submit
+     * that is not a 5xx, which is the wide net the paragraph above is about.
      */
     public static boolean spentIt(final Throwable error) {
         for (Throwable cause = error; cause != null; cause = cause.getCause()) {
             final String message = cause.getMessage();
-            if (message != null && message.contains("UnhandledProtocolError")) {
+            if (message != null
+                    && (message.contains("UnhandledProtocolError")
+                        || message.contains("AppleServiceUnavailableError"))) {
                 return true;
             }
             if (cause.getCause() == cause) {

app/src/main/python/icloud_bridge.py

@@ -43,7 +43,7 @@
 
 import identity as app_identity
 from exporter import icloud
-from findmy.errors import InvalidCredentialsError
+from findmy.errors import AppleServiceUnavailableError, InvalidCredentialsError
 from findmy.keychain.enrolment import DeviceDescription
 from findmy.keychain.join import JoinedPeer
 from findmy.keychain.recovery import RecoveryError
@@ -125,6 +125,20 @@
 naming record as its single source of truth.
 """
 
+REASON_APPLE_DECLINED = "apple_declined"
+"""
+Apple answered and refused to serve. Worth waiting out, and not the user's fault.
+
+**Separate from :data:`REASON_UNKNOWN` because the screen written for unknown says the wrong
+thing.** Unknown shows whatever detail there is, which here is a sentence about Grand Slam and an
+HTTP status - accurate, unreadable, and indistinguishable from a bug in this app. See
+OpenTagViewer#176, where somebody filed exactly that.
+
+Separate from :data:`REASON_CREDENTIALS_REJECTED` for the opposite reason: that one is permanent
+and ends in a forced sign-out. Signing somebody out over a 503 destroys a working session for
+nothing, which is rule 15's failure mode in the expensive direction.
+"""
+
 REASON_UNKNOWN = "unknown"
 """Anything else, with the exception text carried through so a report can be answered."""
 
@@ -176,6 +190,11 @@ def _unexpected(what: str) -> str:
     if _needsAFreshSignIn(sys.exc_info()[1]):
         return _failure(REASON_CREDENTIALS_REJECTED, _lastLineOf(detail) or what + " was refused")
 
+    # After the credentials check and before the fallback. Apple declining is neither a dead
+    # session nor a mystery, and it is the one failure here that is worth simply waiting out.
+    if isinstance(sys.exc_info()[1], AppleServiceUnavailableError):
+        return _failure(REASON_APPLE_DECLINED, _lastLineOf(detail) or what + " was declined")
+
     # `str(e)` is empty for several of these - TimeoutError most of all - so the last line of
     # the traceback stands in. It names the exception type, which is not a good message but is
     # infinitely better than a colon with nothing after it.

app/src/main/python/main.py

@@ -10,6 +10,7 @@
 import NSKeyedUnArchiver
 
 from findmy import FindMyAccessory, MobileMeDelegateError
+from findmy.errors import AppleServiceUnavailableError
 from findmy.accessory import FixedRollingKeyPairAccessory
 from findmy.keys import KeyPairType
 from findmy.reports import (
@@ -458,6 +459,21 @@ def assertAnisetteIsSupported(serializedAccountData: str) -> str | None:
 screen that keeps refusing them for a reason nothing tells them.
 """
 
+REASON_APPLE_DECLINED = "apple_declined"
+"""
+Apple answered, and refused to serve the request. Not the password, and not the network.
+
+**A 503 from Grand Slam reads as a rejected sign-in to everybody who meets it**, because the only
+thing on screen is a failure at the moment a password was entered. It is neither: nothing was
+wrong with the credentials, nothing was reached and refused on their merits, and it clears on its
+own within minutes. Reported as OpenTagViewer#176, where one account met the same 503 three times
+in three minutes at three different call sites.
+
+Distinguished from :data:`REASON_NETWORK` because the advice differs. A network failure is
+usually the phone's - check the connection. This one is Apple's, and checking anything at this
+end is wasted effort.
+"""
+
 REASON_UNKNOWN = "unknown"
 """Anything else. The detail is shown as-is, because a wrong guess is worse than raw text."""
 
@@ -484,6 +500,13 @@ def classifyLoginFailure(error: BaseException) -> str:
     if isinstance(error, MobileMeDelegateError):
         return REASON_TERMS
 
+    # Before the network checks, and not because of ordering hazards - it is a RuntimeError and
+    # collides with none of them. It is here because it reads as the same thing to a user and is
+    # not: Apple answered. Telling somebody to check their connection when their connection is
+    # fine sends them to reset a router over a 503.
+    if isinstance(error, AppleServiceUnavailableError):
+        return REASON_APPLE_DECLINED
+
     if isinstance(error, (asyncio.TimeoutError, asyncio.CancelledError)):
         return REASON_NETWORK
     if isinstance(error, _NETWORK_ERRORS):

app/src/main/res/values-de/strings.xml

@@ -366,6 +366,9 @@ Deine Tags und ihr Standortverlauf sind davon nicht betroffen, und aus deinem Ap
     <string name="twofactor_apple_took_the_code">Apple hat deinen Code angenommen und danach die Anmeldung nicht abschließen können. Der Code ist damit verbraucht, es wird also ein neuer gebraucht – wir warten kurz, bevor wir ihn anfordern, denn sofortiges Anfordern wird abgelehnt.</string>
     <string name="twofactor_waiting_seconds">Neuer Code wird in %1$d s bei Apple angefordert …</string>
     <string name="twofactor_apple_did_not_recover">Apple schließt die Anmeldung weiterhin nicht ab. Das liegt an Apple, nicht an dir und nicht an deinem Code.\n\nVersuche es in ein paar Minuten erneut. Dein Passwort wird dabei möglicherweise einmal abgelehnt – das gehört zum selben Fehler, gib es also einfach noch einmal ein, statt es für falsch zu halten.</string>
+    <string name="login_failed_apple_declined">Apple hat die Anmeldung abgelehnt. Das ist eine Ablehnung durch Apple und kein Problem mit deiner Apple-ID oder deinem Passwort. Ein erneuter Versuch nach kurzer Zeit lohnt sich, aber wenn es weiterhin abgelehnt wird, melde es bitte mit einem Protokoll, statt zu warten.</string>
+    <string name="icloud_apple_declined_title">Apple antwortet gerade nicht</string>
+    <string name="icloud_apple_declined_body">Apple hat die Anfrage abgelehnt, statt gar nicht zu antworten. Mit deinem Account ist alles in Ordnung, und es wurde nichts geändert. Versuche es bald erneut; wenn es weiterhin abgelehnt wird, melde es mit einem Protokoll.</string>
     <string name="import_history">Verlauf importieren</string>
     <string name="history_import_complete_title">Verlaufsimport abgeschlossen</string>
     <string name="history_import_result_counts">Gelesene Zeilen: %1$d\nHinzugefügte Zeilen: %2$d\nBereits vorhanden: %3$d\nFehlerhaft: %4$d\nÜbersprungen (Tag nicht importiert): %5$d</string>