diff --git a/app/src/androidTest/kotlin/org/libremail/notifications/NotificationIntentsTest.kt b/app/src/androidTest/kotlin/org/libremail/notifications/NotificationIntentsTest.kt index 8a546ff..5cf8aef 100644 --- a/app/src/androidTest/kotlin/org/libremail/notifications/NotificationIntentsTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/notifications/NotificationIntentsTest.kt @@ -13,9 +13,11 @@ import org.junit.Test import org.junit.runner.RunWith /** - * Locks in the notification deep-link contract: a message id round-trips build → parse, and intents - * for different messages are distinct under [Intent.filterEquals] — the identity PendingIntent keys - * on — so per-message notifications never collapse onto one shared PendingIntent. + * Locks in the notification deep-link contract: a message id round-trips build → parse, intents for + * different messages are distinct under [Intent.filterEquals] — the identity PendingIntent keys on — + * so per-message notifications never collapse onto one shared PendingIntent, and an `ACTION_OPEN_MESSAGE` + * intent that does not carry this app's own sender token is ignored so a foreign caller targeting the + * exported activity cannot drive the reader (#307). */ @RunWith(AndroidJUnit4::class) class NotificationIntentsTest { @@ -25,20 +27,31 @@ class NotificationIntentsTest { @Test fun message_id_round_trips_through_the_intent() { val id = "imap:user@example.com:INBOX:42" - assertEquals(id, NotificationIntents.messageId(NotificationIntents.openMessage(context, id))) + assertEquals(id, NotificationIntents.messageId(context, NotificationIntents.openMessage(context, id))) } @Test fun uri_hostile_ids_round_trip() { val id = "imap:user@example.com:[Gmail]/All Mail:7?&%#" - assertEquals(id, NotificationIntents.messageId(NotificationIntents.openMessage(context, id))) + assertEquals(id, NotificationIntents.messageId(context, NotificationIntents.openMessage(context, id))) } @Test fun other_intents_carry_no_message_id() { - assertNull(NotificationIntents.messageId(null)) - assertNull(NotificationIntents.messageId(Intent(Intent.ACTION_MAIN))) - assertNull(NotificationIntents.messageId(Intent(Intent.ACTION_VIEW, Uri.parse("mailto:a@b.c")))) + assertNull(NotificationIntents.messageId(context, null)) + assertNull(NotificationIntents.messageId(context, Intent(Intent.ACTION_MAIN))) + assertNull(NotificationIntents.messageId(context, Intent(Intent.ACTION_VIEW, Uri.parse("mailto:a@b.c")))) + } + + @Test + fun open_message_intent_without_our_sender_token_is_ignored() { + // A hostile app can target the exported activity with our action (and a message id), but it + // cannot mint a PendingIntent attributed to us — so an ACTION_OPEN_MESSAGE intent that lacks our + // sender token must be ignored, while the genuine one (built by openMessage) still resolves. + val genuine = NotificationIntents.openMessage(context, "imap:a@b:INBOX:1") + val forged = Intent().setAction(genuine.action) + assertNull(NotificationIntents.messageId(context, forged)) + assertEquals("imap:a@b:INBOX:1", NotificationIntents.messageId(context, genuine)) } @Test diff --git a/app/src/main/kotlin/org/libremail/MainActivity.kt b/app/src/main/kotlin/org/libremail/MainActivity.kt index 4a09b81..c71630c 100644 --- a/app/src/main/kotlin/org/libremail/MainActivity.kt +++ b/app/src/main/kotlin/org/libremail/MainActivity.kt @@ -122,7 +122,7 @@ class MainActivity : FragmentActivity() { private fun handleIntent(intent: Intent) { if (!IntentHandledMarker.markIfUnhandled(intent)) return IntentComposeParser.parse(intent)?.let { pendingCompose.value = it } - NotificationIntents.messageId(intent)?.let { pendingOpenMessageId.value = it } + NotificationIntents.messageId(this, intent)?.let { pendingOpenMessageId.value = it } } } diff --git a/app/src/main/kotlin/org/libremail/data/sync/AccountThrottleGate.kt b/app/src/main/kotlin/org/libremail/data/sync/AccountThrottleGate.kt new file mode 100644 index 0000000..003eedd --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/sync/AccountThrottleGate.kt @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import org.libremail.reporting.AppLog +import org.libremail.reporting.accountLogRef +import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.ThreadLocalRandom +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Per-account reactive throttle state for issue #360: the single, shared place that turns a classified + * [ThrottleSignal] (from [ThrottleClassifier]) into an exponentially-growing, jittered backoff window + * (via [ThrottleBackoff]) and remembers, per account, when that window elapses. + * + * **Graceful degradation, not failing hard.** Background activity (notably the full-history backfill, + * [MailBackfiller]) consults [remainingBackoffMillis] / [isThrottled] before touching an account and + * *skips* one that is still cooling down, resuming automatically once the window passes — instead of + * re-hitting a provider that just rate-limited or locked us (which the on-device perf drilldown proved + * makes throttling worse, `docs/perf/issue-125-*`). + * + * **Per-account isolation.** State is keyed by account id, so one throttled account never stalls the + * others; each escalates and recovers on its own. + * + * **Interactive priority.** Interactive/foreground sync feeds this gate ([onThrottle]) so background + * work backs off, but is itself never blocked by it — opening a message is never queued behind a + * backfill backoff. + * + * State lives only in-process (a `@Singleton`); a process restart resets it, which is fine — WorkManager + * job backoff covers the cross-process case and a fresh process simply re-probes. Every log line is + * PII-free: [accountLogRef] for the account, and durations/counts only. + */ +@Singleton +class AccountThrottleGate internal constructor(private val nowMillis: () -> Long, private val random: () -> Double) { + /** Production wiring: the real wall clock and a per-thread RNG for the jitter draw. */ + @Inject + constructor() : this(nowMillis = System::currentTimeMillis, random = { ThreadLocalRandom.current().nextDouble() }) + + /** One account's live backoff: how many consecutive throttles, until when, and the last computed wait. */ + private data class State(val attempt: Int, val throttledUntilMillis: Long, val lastBackoffMillis: Long) + + private val states = ConcurrentHashMap() + + /** + * Records a throttle for [accountId] and returns the resulting backoff in ms. Escalates the + * consecutive-attempt count (so repeats back off exponentially) and extends the account's cooldown + * window to `now + backoff`. Atomic per account. Logs a PII-free breadcrumb (kind, attempt, backoff). + */ + fun onThrottle(accountId: String, signal: ThrottleSignal): Long { + val now = nowMillis() + val updated = states.compute(accountId) { _, previous -> + val attempt = (previous?.attempt ?: 0) + 1 + val backoff = ThrottleBackoff.delayMillis(attempt, signal, random()) + State(attempt = attempt, throttledUntilMillis = now + backoff, lastBackoffMillis = backoff) + }!! + AppLog.w( + TAG, + "throttled ${accountLogRef(accountId)} kind=${signal.kind} attempt=${updated.attempt} " + + "backoff=${updated.lastBackoffMillis}ms", + ) + return updated.lastBackoffMillis + } + + /** + * Clears any backoff for [accountId] after a successful operation, so a recovered account resumes at + * full speed with the attempt count reset. A no-op (and silent) when the account was not throttled, + * so callers can invoke it on every success without log spam. + */ + fun onSuccess(accountId: String) { + val previous = states.remove(accountId) ?: return + AppLog.i(TAG, "throttle cleared ${accountLogRef(accountId)} after ${previous.attempt} attempt(s)") + } + + /** + * Milliseconds until [accountId]'s backoff window elapses, or 0 when it is not throttled (or the + * window already passed). A passed window keeps its attempt count until the next [onSuccess], so a + * re-throttle before recovery escalates rather than restarting from the base delay. + */ + fun remainingBackoffMillis(accountId: String): Long { + val state = states[accountId] ?: return 0L + return (state.throttledUntilMillis - nowMillis()).coerceAtLeast(0L) + } + + /** True while [accountId] is inside its backoff window and background work should skip it. */ + fun isThrottled(accountId: String): Boolean = remainingBackoffMillis(accountId) > 0L + + private companion object { + const val TAG = "ThrottleGate" + } +} diff --git a/app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt b/app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt index 48b2a90..e067cb7 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/MailBackfiller.kt @@ -3,6 +3,7 @@ package org.libremail.data.sync import android.content.Context import dagger.hilt.android.qualifiers.ApplicationContext +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.currentCoroutineContext import kotlinx.coroutines.delay @@ -56,6 +57,7 @@ class MailBackfiller @Inject constructor( private val batteryStatusProvider: BatteryStatusProvider, private val mailRepository: MailRepository, private val maintenanceGate: MailMaintenanceGate, + private val throttleGate: AccountThrottleGate, ) { /** One folder's slice outcome: pages fetched, and whether an immediate follow-up slice has work to do. */ private data class FolderResult(val batches: Int, val moreWork: Boolean) @@ -72,6 +74,18 @@ class MailBackfiller @Inject constructor( var remaining = maxBatches var moreWork = false accounts@ for (account in accountDao.getAll().map { it.toDomain() }) { + // Graceful degradation + per-account isolation (#360): an account still inside its throttle + // backoff window is skipped this slice — we don't page a provider that just rate-limited or + // locked us (hammering it makes throttling worse, the on-device perf finding). The window + // elapses on its own, so a later scheduled slice resumes this account automatically. A skip + // deliberately does NOT set moreWork: a slice whose only outstanding work is a throttled + // account reports "done" so the worker's slice-chaining loop stops instead of tight-looping + // over the skip. Other accounts are untouched. + val backoffRemaining = throttleGate.remainingBackoffMillis(account.id) + if (backoffRemaining > 0L) { + AppLog.i(TAG, "backfill skip ${accountLogRef(account.id)}: throttled, remaining=${backoffRemaining}ms") + continue@accounts + } val params = runCatching { connectionFactory.imapParamsFor(account) }.getOrNull() ?: continue val policy = accountSettingsRepository.effectiveRetention(settingsRepository, account.id) for (folder in messageDao.syncedFolders(account.id)) { @@ -81,10 +95,14 @@ class MailBackfiller @Inject constructor( moreWork = true break@accounts } - // Per-folder failures (e.g. a transient server error) must not abort the whole slice. - val result = runCatching { backfillFolder(account, params, folder, policy, remaining) } - .getOrElse { FolderResult(batches = 0, moreWork = true) } + // null == this folder throttled/locked the account (already recorded): stop paging the + // account for the rest of the slice — graceful degradation, not a hard failure, and not a + // moreWork spin against a server that just told us to slow down. + val result = pageFolder(account, params, folder, policy, remaining) ?: continue@accounts remaining -= result.batches + // A page landed, so the account is healthy again — clear any lingering backoff (no-op and + // silent when it was never throttled). + if (result.batches > 0) throttleGate.onSuccess(account.id) if (result.moreWork) moreWork = true } } @@ -92,6 +110,36 @@ class MailBackfiller @Inject constructor( moreWork } + /** + * Pages one folder, translating a failure into the slice's control flow (issue #360). Returns the + * [FolderResult] on success — or, for an ordinary transient error, a zero-page result whose + * [FolderResult.moreWork] asks for a follow-up slice (unchanged behaviour). Returns **null** when the + * failure classifies as provider throttling/lockout ([ThrottleClassifier]): the backoff is recorded + * against the account (exponential + jitter, via [AccountThrottleGate]) and the caller stops paging + * this account for the rest of the slice, so we degrade gracefully instead of hammering a server that + * just told us to slow down (the on-device perf finding, `docs/perf/issue-125-*`). Cancellation + * propagates so a WorkManager stop / IDLE renewal ends the run promptly. + */ + private suspend fun pageFolder( + account: Account, + params: ImapConnectionParams, + folder: String, + policy: RetentionPolicy, + maxBatches: Int, + ): FolderResult? = try { + backfillFolder(account, params, folder, policy, maxBatches) + } catch (e: CancellationException) { + throw e + } catch (e: Throwable) { + val signal = ThrottleClassifier.classify(e) + if (signal == null) { + FolderResult(batches = 0, moreWork = true) + } else { + throttleGate.onThrottle(account.id, signal) + null + } + } + private suspend fun backfillFolder( account: Account, params: ImapConnectionParams, diff --git a/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt b/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt index 1d8df94..e2f0814 100644 --- a/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt +++ b/app/src/main/kotlin/org/libremail/data/sync/MailSyncer.kt @@ -40,6 +40,7 @@ class MailSyncer @Inject constructor( private val batteryStatusProvider: BatteryStatusProvider, private val notifier: MailNotifier, private val mailRepository: MailRepository, + private val throttleGate: AccountThrottleGate, ) : Syncer { // Serializes all syncing: syncAll/syncAccount/syncFolder are invoked concurrently by the periodic // worker, pull-to-refresh, one-shot syncs, folder opens, and one IDLE watcher per account. Without @@ -156,6 +157,11 @@ class MailSyncer @Inject constructor( val folderLabel = logSafeFolderLabel(folder) AppLog.d(TAG, "sync ${accountLogRef(account.id)} folder=$folderLabel fetched=${fetched.size}") fetched.size + }.onFailure { error -> + // Interactive priority (#360): a foreground sync that hits provider throttling records it so + // the background backfill backs this account off — but the interactive sync itself is never + // blocked by the gate, so opening/refreshing mail is never queued behind a backfill backoff. + ThrottleClassifier.classify(error)?.let { throttleGate.onThrottle(account.id, it) } } /** diff --git a/app/src/main/kotlin/org/libremail/data/sync/ThrottleBackoff.kt b/app/src/main/kotlin/org/libremail/data/sync/ThrottleBackoff.kt new file mode 100644 index 0000000..8b309ec --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/sync/ThrottleBackoff.kt @@ -0,0 +1,66 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import kotlin.math.min + +/** + * The pure backoff schedule for issue #360: given how many times an account has been throttled in a + * row ([attempt], 1-based) and the [ThrottleSignal], returns how long to wait before touching that + * account again. Exponential in the attempt, capped at a bounded maximum, with **equal jitter** so a + * fleet of clients throttled at once don't retry in lockstep and re-trip the limit. + * + * Kept side-effect-free and clock-free (the caller supplies the jitter draw) so the whole schedule is + * deterministically unit-testable; [AccountThrottleGate] owns the per-account state, clock, and logging. + */ +object ThrottleBackoff { + + /** First-attempt wait for a rate limit (30s); doubles per repeat up to [RATE_LIMIT_MAX_MS]. */ + const val RATE_LIMIT_BASE_MS = 30_000L + + /** Ceiling for a rate-limit backoff (15 min) — long enough to clear a clamp, short enough to recover. */ + const val RATE_LIMIT_MAX_MS = 15 * 60_000L + + /** + * First-attempt wait for a lockout (1h) — sized to Yahoo's documented ~1-hour auth lock, the case + * that motivated the circuit-breaker lever in issue #360. + */ + const val LOCKOUT_BASE_MS = 60 * 60_000L + + /** Ceiling for a lockout backoff (4h) — a repeatedly re-locked account waits out ever longer windows. */ + const val LOCKOUT_MAX_MS = 4 * 60 * 60_000L + + /** Caps the exponential shift so `base shl (attempt-1)` can never overflow before the min-cap applies. */ + private const val MAX_SHIFT = 16 + + /** + * Backoff in milliseconds for the given 1-based [attempt] and [signal]. The uncapped target is + * `base * 2^(attempt-1)`, clamped to the kind's maximum; **equal jitter** then keeps half of that + * as a floor and spreads the other half by [random] (expected in `[0.0, 1.0)`), so the result lies + * in `[capped/2, capped]`. Finally the provider's own [ThrottleSignal.retryAfterMillis], when + * present, is honored as a lower bound — we never wait less than a server explicitly asked for. + */ + fun delayMillis(attempt: Int, signal: ThrottleSignal, random: Double): Long { + require(attempt >= 1) { "attempt must be >= 1" } + val base: Long + val cap: Long + when (signal.kind) { + ThrottleKind.RATE_LIMIT -> { + base = RATE_LIMIT_BASE_MS + cap = RATE_LIMIT_MAX_MS + } + ThrottleKind.LOCKOUT -> { + base = LOCKOUT_BASE_MS + cap = LOCKOUT_MAX_MS + } + } + val shift = min(attempt - 1, MAX_SHIFT) + val exponential = base shl shift + // shl can overflow to <= 0 for a pathological attempt; treat that as "past the cap". + val capped = if (exponential in 1..cap) exponential else cap + val half = capped / 2 + val jitter = (random.coerceIn(0.0, 1.0) * half).toLong() + val backoff = half + jitter + val floor = signal.retryAfterMillis ?: 0L + return maxOf(backoff, floor) + } +} diff --git a/app/src/main/kotlin/org/libremail/data/sync/ThrottleClassifier.kt b/app/src/main/kotlin/org/libremail/data/sync/ThrottleClassifier.kt new file mode 100644 index 0000000..c10535a --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/sync/ThrottleClassifier.kt @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +/** + * Classifies a mail-layer failure as a **provider throttling / lockout** signal, distinct from an + * ordinary transient error (a dropped socket, a timeout, a "message not found"). This is the shared, + * provider-aware detection half of issue #360's reactive backoff layer: [AccountThrottleGate] turns a + * non-null [ThrottleSignal] into an exponential, jittered per-account backoff so the app degrades + * gracefully instead of hammering a server that just asked it to slow down (which — per the on-device + * perf finding, `docs/perf/issue-125-*` — makes the throttle worse and can trip a lasting lockout). + * + * Matching is by **message text** across the whole cause chain (case-insensitive), so it works for any + * transport (IMAP/SMTP over Jakarta Mail, or a Graph HTTP error surfaced as an exception) without + * coupling to a specific exception type. It is deliberately **conservative**: an ordinary auth failure + * (wrong password) or the separately-handled "IMAP is disabled" state (issue #390) must NOT be read as + * throttling — misclassifying them would make the app back off for up to hours on a permanent error. + * A false negative merely falls back to the existing transient-error handling; a false positive would + * silently stall an account, so the patterns anchor on explicit throttle/lock wording. + */ +object ThrottleClassifier { + + /** HTTP 429 Too Many Requests — the canonical rate-limit status (e.g. Microsoft Graph). */ + const val HTTP_TOO_MANY_REQUESTS = 429 + + /** HTTP 503 Service Unavailable — a load-shed / backoff signal, usually with a `Retry-After`. */ + const val HTTP_SERVICE_UNAVAILABLE = 503 + + /** + * Classifies [error] (and its transitive causes) into a [ThrottleSignal], or null when it is not a + * throttling/lockout response. [ThrottleKind.LOCKOUT] is tested first so a message naming both a + * lock and a rate limit takes the longer, safer backoff. + */ + fun classify(error: Throwable): ThrottleSignal? { + val text = causeChain(error) + .mapNotNull { it.message } + .joinToString(separator = " | ") + .lowercase() + if (text.isBlank()) return null + if (LOCKOUT_PATTERNS.any { it.containsMatchIn(text) }) return ThrottleSignal(ThrottleKind.LOCKOUT) + if (RATE_LIMIT_PATTERNS.any { it.containsMatchIn(text) }) return ThrottleSignal(ThrottleKind.RATE_LIMIT) + return null + } + + /** + * Classifies an HTTP response by status code, honoring a parsed `Retry-After` when the caller has + * one (Microsoft Graph returns it on a 429). The structured entry point for a REST transport that + * already has the status + header in hand, complementing the text-based [classify] used by the + * IMAP/SMTP paths. Only the throttling statuses map to a signal; everything else is null. + */ + fun classifyHttpStatus(status: Int, retryAfterMillis: Long? = null): ThrottleSignal? = when (status) { + HTTP_TOO_MANY_REQUESTS, HTTP_SERVICE_UNAVAILABLE -> + ThrottleSignal(ThrottleKind.RATE_LIMIT, retryAfterMillis) + else -> null + } + + /** + * The exception and its transitive causes, in order, guarding against a self-referential or cyclic + * cause chain (identity-based visited check — [Throwable] does not override `equals`). Mirrors the + * same-shaped walk in [org.libremail.mail.ImapAuthError]. + */ + private fun causeChain(error: Throwable): List { + val seen = mutableListOf() + var current: Throwable? = error + while (current != null && seen.none { it === current }) { + seen.add(current) + current = current.cause + } + return seen + } + + /** + * Lockout wording (matched against a lowercased message). Anchored on lock/suspend/too-many-logins + * so a plain "AUTHENTICATE failed" wrong-password message never matches, and "IMAP is disabled" + * (issue #390's actionable state) is deliberately excluded — that is not a throttle. + */ + private val LOCKOUT_PATTERNS = listOf( + // "account temporarily locked", "your account has been locked", "account locked", "lockout". + Regex("""lock(ed|out)"""), + // "temporarily suspended", "account suspended". + Regex("""suspend(ed)?"""), + // Yahoo-style repeated-login lock precursor: "too many login attempts", "too many failed logins". + Regex("""too many (failed )?log(in|ins)"""), + ) + + /** + * Rate-limit wording (matched against a lowercased message): explicit IMAP/SMTP throttle NOs, the + * RFC 5530 `[LIMIT]` / `[UNAVAILABLE]` response codes, connection/request caps, and an HTTP 429 that + * surfaced only as text. Each anchors on a rate/throttle phrase, never a bare number. + */ + private val RATE_LIMIT_PATTERNS = listOf( + Regex("""throttl"""), // throttled / throttling / [THROTTLED] + Regex("""too many requests"""), + Regex("""too many (simultaneous|concurrent) connections"""), + Regex("""too many connections"""), + Regex("""too many messages"""), + Regex("""rate[ -]?limit"""), + Regex("""\[limit\]"""), + Regex("""\[unavailable\]"""), + Regex("""temporarily unavailable"""), + Regex("""service (not|un)available"""), + Regex("""http 429"""), + Regex("""429 too many"""), + ) +} diff --git a/app/src/main/kotlin/org/libremail/data/sync/ThrottleSignal.kt b/app/src/main/kotlin/org/libremail/data/sync/ThrottleSignal.kt new file mode 100644 index 0000000..f586b08 --- /dev/null +++ b/app/src/main/kotlin/org/libremail/data/sync/ThrottleSignal.kt @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +/** + * How severe a provider's throttling response is, which sets how long the reactive backoff waits + * before the offending activity may touch that account again (issue #360). + */ +enum class ThrottleKind { + /** + * A transient rate / connection / bandwidth limit — an IMAP `[THROTTLED]` / "Too many requests" + * NO, an HTTP 429, or "too many simultaneous connections". Recoverable after a short, exponentially + * growing backoff: the provider is asking us to slow down, not shutting us out. + */ + RATE_LIMIT, + + /** + * A provider lockout — e.g. Yahoo's ~1-hour auth lock after repeated logins, or an explicit + * "account temporarily locked / suspended". A hard signal to stop retrying for a long window; a + * tight retry loop here only prolongs (or re-arms) the lock. + */ + LOCKOUT, +} + +/** + * A classified provider throttling response — the output of [ThrottleClassifier] and the input to + * [AccountThrottleGate]. Deliberately carries no PII (no host, address, or server body): only the + * [kind] and, when the provider gave a machine-readable minimum wait (e.g. an HTTP `Retry-After` + * header — [retryAfterMillis]), a number of milliseconds. IMAP/SMTP throttling rarely carries a + * `Retry-After`, so [retryAfterMillis] is usually null and the exponential schedule alone applies. + */ +data class ThrottleSignal( + val kind: ThrottleKind, + /** Provider-suggested minimum wait in ms (e.g. Graph `Retry-After`), or null when none was given. */ + val retryAfterMillis: Long? = null, +) diff --git a/app/src/main/kotlin/org/libremail/notifications/NotificationIntents.kt b/app/src/main/kotlin/org/libremail/notifications/NotificationIntents.kt index a27a40b..3c77c1e 100644 --- a/app/src/main/kotlin/org/libremail/notifications/NotificationIntents.kt +++ b/app/src/main/kotlin/org/libremail/notifications/NotificationIntents.kt @@ -1,10 +1,13 @@ // SPDX-License-Identifier: GPL-3.0-or-later package org.libremail.notifications +import android.app.PendingIntent import android.content.Context import android.content.Intent import android.net.Uri +import androidx.core.content.IntentCompat import org.libremail.MainActivity +import org.libremail.reporting.AppLog /** * Builds and parses the intent behind a tapped per-message new-mail notification, keeping both sides @@ -14,20 +17,55 @@ import org.libremail.MainActivity * distinct URI every message's notification would collapse onto one FLAG_UPDATE_CURRENT PendingIntent * and always open the most-recently-notified message. The intent is explicit (component set), so the * private scheme needs no manifest intent-filter and adds no exported surface. + * + * [MainActivity] is nonetheless `exported="true"` (launcher / mailto: / share), so another app could + * still target it with an explicit `ACTION_OPEN_MESSAGE` intent and drive the reader to an arbitrary + * cached message id (#307). To honour only this app's own notification taps, [openMessage] attaches an + * unforgeable **sender token** — a PendingIntent whose creator package is stamped by the system — and + * [messageId] yields the id only when that token was created by us. A foreign caller carries no token + * (or one attributed to its own package), so its intent is ignored. */ object NotificationIntents { + private const val TAG = "NotificationIntents" private const val ACTION_OPEN_MESSAGE = "org.libremail.action.OPEN_MESSAGE" + private const val ACTION_SENDER_TOKEN = "org.libremail.action.SENDER_TOKEN" private const val EXTRA_MESSAGE_ID = "org.libremail.extra.MESSAGE_ID" + private const val EXTRA_SENDER_TOKEN = "org.libremail.extra.SENDER_TOKEN" fun openMessage(context: Context, messageId: String): Intent = Intent(context, MainActivity::class.java).apply { action = ACTION_OPEN_MESSAGE data = Uri.parse("libremail://message/${Uri.encode(messageId)}") putExtra(EXTRA_MESSAGE_ID, messageId) + putExtra(EXTRA_SENDER_TOKEN, senderToken(context)) flags = Intent.FLAG_ACTIVITY_SINGLE_TOP or Intent.FLAG_ACTIVITY_CLEAR_TOP } - /** The tapped message's id, or null for any other intent (launcher, mailto:, share, …). */ - fun messageId(intent: Intent?): String? = - intent?.takeIf { it.action == ACTION_OPEN_MESSAGE }?.getStringExtra(EXTRA_MESSAGE_ID) + /** + * The tapped message's id, or null for any other intent (launcher, mailto:, share, …). Also null + * for an `ACTION_OPEN_MESSAGE` intent that did not originate from this app's own notification — + * i.e. one whose [senderToken] is missing or was minted by another package — so a foreign caller + * targeting the exported [MainActivity] can never drive the reader (#307). + */ + fun messageId(context: Context, intent: Intent?): String? { + if (intent?.action != ACTION_OPEN_MESSAGE) return null + val token = IntentCompat.getParcelableExtra(intent, EXTRA_SENDER_TOKEN, PendingIntent::class.java) + if (token?.creatorPackage != context.packageName) { + AppLog.w(TAG, "Ignoring ACTION_OPEN_MESSAGE intent lacking this app's sender token") + return null + } + return intent.getStringExtra(EXTRA_MESSAGE_ID) + } + + /** + * An immutable PendingIntent used purely as unforgeable proof of origin: only this app can mint one + * whose [PendingIntent.getCreatorPackage] is our package. It is never sent — a package-scoped + * broadcast to a receiver we don't register — so firing it (which never happens) is a no-op. + */ + private fun senderToken(context: Context): PendingIntent = PendingIntent.getBroadcast( + context, + 0, + Intent(ACTION_SENDER_TOKEN).setPackage(context.packageName), + PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT, + ) } diff --git a/app/src/test/kotlin/org/libremail/data/sync/AccountThrottleGateTest.kt b/app/src/test/kotlin/org/libremail/data/sync/AccountThrottleGateTest.kt new file mode 100644 index 0000000..940f6ad --- /dev/null +++ b/app/src/test/kotlin/org/libremail/data/sync/AccountThrottleGateTest.kt @@ -0,0 +1,140 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import android.util.Log +import io.mockk.every +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runTest +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.libremail.reporting.AppLog +import org.libremail.reporting.RingLogBuffer +import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +/** + * [AccountThrottleGate] must escalate a throttled account's backoff, isolate accounts from one + * another, reset on success, expose an accurate remaining window (proven against coroutines-test + * virtual time), and log only PII-free breadcrumbs. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class AccountThrottleGateTest { + + private val logBuffer = RingLogBuffer() + + /** A manual virtual clock for the non-timing tests; [gate] reads it live, so tests advance it by hand. */ + private var now = 0L + + /** random = 0.0 makes the equal-jitter draw deterministic (always the lower bound). */ + private fun gate(random: () -> Double = { 0.0 }) = AccountThrottleGate(nowMillis = { now }, random = random) + + @Before + fun setUp() { + // AppLog forwards to android.util.Log, a throwing no-op stub under plain JVM unit tests. + mockkStatic(Log::class) + every { Log.i(any(), any()) } returns 0 + every { Log.w(any(), any()) } returns 0 + AppLog.install(logBuffer) + } + + @After + fun tearDown() = unmockkAll() + + @Test + fun `onThrottle marks the account throttled and returns the backoff`() { + val gate = gate() + + val backoff = gate.onThrottle("acct", ThrottleSignal(ThrottleKind.RATE_LIMIT)) + + assertTrue(backoff > 0L) + assertTrue(gate.isThrottled("acct")) + assertEquals(backoff, gate.remainingBackoffMillis("acct")) + } + + @Test + fun `repeated throttles escalate the backoff window`() { + val gate = gate() + + val first = gate.onThrottle("acct", ThrottleSignal(ThrottleKind.RATE_LIMIT)) + val second = gate.onThrottle("acct", ThrottleSignal(ThrottleKind.RATE_LIMIT)) + + assertTrue(second > first, "a consecutive throttle must back off longer ($second !> $first)") + } + + @Test + fun `onSuccess clears the backoff and resets the attempt count`() { + val gate = gate() + + val first = gate.onThrottle("acct", ThrottleSignal(ThrottleKind.RATE_LIMIT)) + gate.onThrottle("acct", ThrottleSignal(ThrottleKind.RATE_LIMIT)) // escalate to attempt 2 + gate.onSuccess("acct") + + assertFalse(gate.isThrottled("acct")) + // A fresh throttle after recovery starts back at the base (attempt 1) delay. + assertEquals(first, gate.onThrottle("acct", ThrottleSignal(ThrottleKind.RATE_LIMIT))) + } + + @Test + fun `a throttled account never stalls a healthy one`() { + val gate = gate() + + gate.onThrottle("throttled", ThrottleSignal(ThrottleKind.LOCKOUT)) + + assertTrue(gate.isThrottled("throttled")) + assertFalse(gate.isThrottled("healthy")) + assertEquals(0L, gate.remainingBackoffMillis("healthy")) + } + + @Test + fun `an elapsed window stops throttling but still escalates a re-throttle`() { + val gate = gate() + + val first = gate.onThrottle("acct", ThrottleSignal(ThrottleKind.RATE_LIMIT)) + now += first // the window elapses + assertFalse(gate.isThrottled("acct"), "the account is free once its window passes") + + // Re-throttling before any success keeps the attempt count — it escalates, not restarts. + val next = gate.onThrottle("acct", ThrottleSignal(ThrottleKind.RATE_LIMIT)) + assertTrue(next > first) + } + + @Test + fun `the throttle window clears exactly when the backoff elapses`() = runTest { + val gate = AccountThrottleGate(nowMillis = { testScheduler.currentTime }, random = { 0.0 }) + + val backoff = gate.onThrottle("acct", ThrottleSignal(ThrottleKind.RATE_LIMIT)) + assertTrue(gate.isThrottled("acct")) + + advanceTimeBy(backoff - 1) + assertTrue(gate.isThrottled("acct"), "still throttled just before the window elapses") + + advanceTimeBy(1) + assertFalse(gate.isThrottled("acct"), "cleared the instant the backoff elapses") + assertEquals(0L, gate.remainingBackoffMillis("acct")) + } + + @Test + fun `throttle and clear log PII-free breadcrumbs`() { + val gate = gate() + + gate.onThrottle("outlook:user@example.org", ThrottleSignal(ThrottleKind.LOCKOUT)) + gate.onSuccess("outlook:user@example.org") + + val messages = logBuffer.snapshot().map { it.message } + assertTrue(messages.any { it.startsWith("throttled outlook:") && it.contains("kind=LOCKOUT") }) + assertTrue(messages.any { it.startsWith("throttle cleared outlook:") }) + messages.forEach { assertFalse(it.contains("user@example.org"), it) } + } + + @Test + fun `onSuccess on a healthy account is silent`() { + gate().onSuccess("acct") + + assertTrue(logBuffer.snapshot().isEmpty()) + } +} diff --git a/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt b/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt index 670080d..d43d195 100644 --- a/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt +++ b/app/src/test/kotlin/org/libremail/data/sync/MailBackfillerTest.kt @@ -13,6 +13,7 @@ import io.mockk.mockkStatic import io.mockk.unmockkAll import jakarta.mail.Folder import jakarta.mail.Message +import jakarta.mail.MessagingException import jakarta.mail.Session import jakarta.mail.internet.InternetAddress import jakarta.mail.internet.MimeMessage @@ -475,6 +476,54 @@ class MailBackfillerTest { ) } + // --- issue #360: throttling backoff + graceful degradation ---------------------------------- + + /** + * When the server throttles the backfill (a `[THROTTLED]` / "too many connections" NO), the slice + * must record a backoff for that account and STOP paging it — not retry the page in a tight loop + * (which is exactly what makes provider throttling worse, `docs/perf/issue-125-*`). It also must not + * report more-work, so [BackfillWorker]'s slice-chaining loop stops rather than spinning. + */ + @Test + fun `a throttling server pauses that account's backfill instead of hammering it`() = runTest { + cached += fetchedMessage(uid = "60").toEntity("acct", "INBOX") + var calls = 0 + val imapClient = mockk() + coEvery { imapClient.fetchOlderThan(any(), any(), any(), any()) } answers { + calls++ + throw MessagingException("A3 NO [THROTTLED] Too many simultaneous connections") + } + val gate = AccountThrottleGate() + + val moreWork = backfiller(AccountSettings("acct"), imapClient = imapClient, throttleGate = gate).runBackfill() + + assertFalse(moreWork, "a throttled account must not drive an immediate re-slice (no tight loop)") + assertTrue(gate.isThrottled("acct"), "the account is now backing off") + assertEquals(1, calls, "paging stops at the first throttle, it is not retried in a tight loop") + assertTrue( + logBuffer.snapshot().any { it.message.startsWith("throttled acct:") }, + "a PII-free throttle breadcrumb is recorded", + ) + } + + /** + * An account still inside its backoff window is skipped entirely — no server call at all — so a + * provider we were just throttled by is left alone until the window elapses (graceful degradation + + * per-account isolation). + */ + @Test + fun `an account inside its backoff window is skipped, not paged`() = runTest { + cached += fetchedMessage(uid = "60").toEntity("acct", "INBOX") + val imapClient = mockk(relaxed = true) + val gate = AccountThrottleGate().apply { onThrottle("acct", ThrottleSignal(ThrottleKind.RATE_LIMIT)) } + + val moreWork = backfiller(AccountSettings("acct"), imapClient = imapClient, throttleGate = gate).runBackfill() + + assertFalse(moreWork, "a slice whose only account is throttled reports done, not more-work") + coVerify(exactly = 0) { imapClient.fetchOlderThan(any(), any(), any(), any()) } + assertTrue(logBuffer.snapshot().any { it.message.startsWith("backfill skip acct:") }) + } + // --- issue #329: AppLog breadcrumbs --------------------------------------------------------- @Test @@ -553,6 +602,7 @@ class MailBackfillerTest { fetchPolicy: FetchPolicy = FetchPolicy.ON_DEMAND, battery: BatteryStatus = BatteryStatus(percent = 100, isCharging = false), imapClient: ImapClient = client, + throttleGate: AccountThrottleGate = AccountThrottleGate(), ): MailBackfiller { val accountDao = mockk() coEvery { accountDao.getAll() } returns listOf(accountEntity) @@ -608,6 +658,7 @@ class MailBackfillerTest { batteryStatusProvider = batteryStatusProvider, mailRepository = mailRepository, maintenanceGate = MailMaintenanceGate(), + throttleGate = throttleGate, ).also { lastMessageDao = messageDao lastMailRepository = mailRepository diff --git a/app/src/test/kotlin/org/libremail/data/sync/MailMaintenanceGateTest.kt b/app/src/test/kotlin/org/libremail/data/sync/MailMaintenanceGateTest.kt index 7732a0e..dbe5e91 100644 --- a/app/src/test/kotlin/org/libremail/data/sync/MailMaintenanceGateTest.kt +++ b/app/src/test/kotlin/org/libremail/data/sync/MailMaintenanceGateTest.kt @@ -200,6 +200,7 @@ class MailMaintenanceGateTest { }, mailRepository = mockk(relaxed = true), maintenanceGate = gate, + throttleGate = AccountThrottleGate(), ) } diff --git a/app/src/test/kotlin/org/libremail/data/sync/MailSyncConcurrencyTest.kt b/app/src/test/kotlin/org/libremail/data/sync/MailSyncConcurrencyTest.kt index 27af4f0..1f2ebd7 100644 --- a/app/src/test/kotlin/org/libremail/data/sync/MailSyncConcurrencyTest.kt +++ b/app/src/test/kotlin/org/libremail/data/sync/MailSyncConcurrencyTest.kt @@ -330,6 +330,7 @@ class MailSyncConcurrencyTest { batteryStatusProvider = fullBattery(), notifier = mockk(relaxed = true), mailRepository = mockk(relaxed = true), + throttleGate = AccountThrottleGate(), ) } @@ -359,6 +360,7 @@ class MailSyncConcurrencyTest { batteryStatusProvider = fullBattery(), mailRepository = mockk(relaxed = true), maintenanceGate = MailMaintenanceGate(), + throttleGate = AccountThrottleGate(), ) } diff --git a/app/src/test/kotlin/org/libremail/data/sync/MailSyncerTest.kt b/app/src/test/kotlin/org/libremail/data/sync/MailSyncerTest.kt index 5e5ec17..1f5e611 100644 --- a/app/src/test/kotlin/org/libremail/data/sync/MailSyncerTest.kt +++ b/app/src/test/kotlin/org/libremail/data/sync/MailSyncerTest.kt @@ -12,6 +12,7 @@ import io.mockk.every import io.mockk.mockk import io.mockk.mockkStatic import io.mockk.unmockkAll +import jakarta.mail.MessagingException import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runTest import org.junit.After @@ -93,6 +94,7 @@ class MailSyncerTest { globalSettings: AppSettings = AppSettings(), battery: BatteryStatus = BatteryStatus(percent = 100, isCharging = false), fetched: List = emptyList(), + throttleGate: AccountThrottleGate = AccountThrottleGate(), ): MailSyncer { val accountDao = mockk() coEvery { accountDao.getById("acct") } returns account @@ -121,6 +123,7 @@ class MailSyncerTest { batteryStatusProvider = batteryProvider(battery), notifier = mockk(relaxed = true), mailRepository = mailRepository, + throttleGate = throttleGate, ) } @@ -336,6 +339,7 @@ class MailSyncerTest { batteryStatusProvider = batteryProvider(BatteryStatus(percent = 100, isCharging = false)), notifier = notifier, mailRepository = mockk(relaxed = true), + throttleGate = AccountThrottleGate(), ) } @@ -418,6 +422,7 @@ class MailSyncerTest { batteryStatusProvider = batteryProvider(BatteryStatus(percent = 100, isCharging = false)), notifier = mockk(relaxed = true), mailRepository = mockk(relaxed = true), + throttleGate = AccountThrottleGate(), ) } @@ -485,6 +490,38 @@ class MailSyncerTest { logBuffer.snapshot().forEach { assertNoPii(it.message) } } + // --- issue #360: foreground sync feeds the throttle gate (interactive priority) ------------- + + /** + * A foreground sync that hits provider throttling records a per-account backoff (so the background + * backfill defers that account) but is itself never blocked — the failure still surfaces to the + * caller. This is the interactive-priority half of #360: interactive work informs the gate, it is + * never gated by it. + */ + @Test + fun `a foreground sync hitting throttling records a backoff without being blocked`() = runTest { + val gate = AccountThrottleGate() + val syncer = syncer(FetchPolicy.ON_DEMAND, mockk(relaxed = true), throttleGate = gate) + coEvery { lastImapClient.fetchRecent(any(), any(), any()) } throws + MessagingException("A2 NO [THROTTLED] Too many requests") + + val result = syncer.syncFolder("acct", "INBOX") + + assertTrue(result.isFailure, "the throttle failure still surfaces to the caller") + assertTrue(gate.isThrottled("acct"), "and it records a backoff so background backfill defers") + } + + /** An ordinary (non-throttle) sync failure must NOT arm a backoff. */ + @Test + fun `a foreground sync failing on an ordinary error does not record a backoff`() = runTest { + val gate = AccountThrottleGate() + val syncer = syncer(FetchPolicy.ON_DEMAND, mockk(relaxed = true), throttleGate = gate) + coEvery { lastImapClient.fetchRecent(any(), any(), any()) } throws IOException("Connection reset") + + assertTrue(syncer.syncFolder("acct", "INBOX").isFailure) + assertFalse(gate.isThrottled("acct")) + } + /** No test fixture's email address or host may ever reach a log line — the hard PII rule. */ private fun assertNoPii(message: String) { assertFalse(message.contains("@example.org"), message) diff --git a/app/src/test/kotlin/org/libremail/data/sync/ThrottleBackoffTest.kt b/app/src/test/kotlin/org/libremail/data/sync/ThrottleBackoffTest.kt new file mode 100644 index 0000000..de120c0 --- /dev/null +++ b/app/src/test/kotlin/org/libremail/data/sync/ThrottleBackoffTest.kt @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import org.junit.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith +import kotlin.test.assertTrue + +/** + * The pure backoff schedule (issue #360): exponential in the consecutive-attempt count, capped at a + * bounded maximum, spread by equal jitter into `[capped/2, capped]`, and never shorter than a + * provider-supplied `Retry-After`. + */ +class ThrottleBackoffTest { + + private val rateLimit = ThrottleSignal(ThrottleKind.RATE_LIMIT) + private val lockout = ThrottleSignal(ThrottleKind.LOCKOUT) + + /** random = 0.0 selects the lower jitter bound (capped/2); random = 1.0 selects the upper (capped). */ + private fun low(attempt: Int, signal: ThrottleSignal) = ThrottleBackoff.delayMillis(attempt, signal, random = 0.0) + private fun high(attempt: Int, signal: ThrottleSignal) = ThrottleBackoff.delayMillis(attempt, signal, random = 1.0) + + @Test + fun `rate-limit backoff doubles per attempt at the lower jitter bound`() { + assertEquals(ThrottleBackoff.RATE_LIMIT_BASE_MS / 2, low(1, rateLimit)) + assertEquals(ThrottleBackoff.RATE_LIMIT_BASE_MS, low(2, rateLimit)) + assertEquals(ThrottleBackoff.RATE_LIMIT_BASE_MS * 2, low(3, rateLimit)) + } + + @Test + fun `the upper jitter bound of attempt 1 is the base delay`() { + assertEquals(ThrottleBackoff.RATE_LIMIT_BASE_MS, high(1, rateLimit)) + } + + @Test + fun `jitter keeps every draw within the exponential half-window`() { + // Attempt 2's capped target is 2*base; equal jitter must land in [base, 2*base] for any draw. + val lower = ThrottleBackoff.RATE_LIMIT_BASE_MS + val upper = ThrottleBackoff.RATE_LIMIT_BASE_MS * 2 + for (thousandths in 0..1000) { + val delay = ThrottleBackoff.delayMillis(attempt = 2, rateLimit, random = thousandths / 1000.0) + assertTrue(delay in lower..upper, "draw $thousandths gave $delay, outside [$lower,$upper]") + } + } + + @Test + fun `a large attempt is capped at the rate-limit maximum`() { + // 2^19 * base overflows the cap many times over; the max must hold for both jitter bounds. + assertEquals(ThrottleBackoff.RATE_LIMIT_MAX_MS / 2, low(20, rateLimit)) + assertEquals(ThrottleBackoff.RATE_LIMIT_MAX_MS, high(20, rateLimit)) + } + + @Test + fun `a lockout starts at a long window and caps higher than a rate limit`() { + assertEquals(ThrottleBackoff.LOCKOUT_BASE_MS / 2, low(1, lockout)) + assertEquals(ThrottleBackoff.LOCKOUT_BASE_MS, high(1, lockout)) + assertEquals(ThrottleBackoff.LOCKOUT_MAX_MS, high(20, lockout)) + assertTrue(ThrottleBackoff.LOCKOUT_MAX_MS > ThrottleBackoff.RATE_LIMIT_MAX_MS) + } + + @Test + fun `a retry-after longer than the computed delay is honored as a floor`() { + val retryAfter = ThrottleBackoff.RATE_LIMIT_MAX_MS * 4 + val signal = ThrottleSignal(ThrottleKind.RATE_LIMIT, retryAfterMillis = retryAfter) + assertEquals(retryAfter, ThrottleBackoff.delayMillis(attempt = 1, signal, random = 1.0)) + } + + @Test + fun `a retry-after shorter than the computed delay does not shorten the backoff`() { + val signal = ThrottleSignal(ThrottleKind.RATE_LIMIT, retryAfterMillis = 1L) + assertEquals(ThrottleBackoff.RATE_LIMIT_BASE_MS / 2, ThrottleBackoff.delayMillis(1, signal, random = 0.0)) + } + + @Test + fun `attempt below 1 is rejected`() { + assertFailsWith { ThrottleBackoff.delayMillis(0, rateLimit, random = 0.0) } + } +} diff --git a/app/src/test/kotlin/org/libremail/data/sync/ThrottleClassifierTest.kt b/app/src/test/kotlin/org/libremail/data/sync/ThrottleClassifierTest.kt new file mode 100644 index 0000000..3de2df7 --- /dev/null +++ b/app/src/test/kotlin/org/libremail/data/sync/ThrottleClassifierTest.kt @@ -0,0 +1,151 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.data.sync + +import org.junit.Test +import java.io.IOException +import kotlin.test.assertEquals +import kotlin.test.assertNull + +/** + * [ThrottleClassifier] must recognize provider throttling / lockout wording across IMAP, SMTP, and + * HTTP — and, just as importantly, must NOT flag an ordinary auth failure, the "IMAP disabled" state + * (issue #390), or a plain network error, since a false positive would silently stall an account for + * up to hours. + */ +class ThrottleClassifierTest { + + private fun classify(message: String?) = ThrottleClassifier.classify(RuntimeException(message)) + + // --- rate-limit positives ------------------------------------------------------------------- + + @Test + fun `gmail too-many-simultaneous-connections is a rate limit`() { + assertEquals( + ThrottleSignal(ThrottleKind.RATE_LIMIT), + classify("A3 NO [ALERT] Too many simultaneous connections. (Failure)"), + ) + } + + @Test + fun `an imap THROTTLED response code is a rate limit`() { + assertEquals(ThrottleKind.RATE_LIMIT, classify("* BYE [THROTTLED] slow down")?.kind) + } + + @Test + fun `too many requests is a rate limit`() { + assertEquals(ThrottleKind.RATE_LIMIT, classify("Server said: Too Many Requests")?.kind) + } + + @Test + fun `an rfc5530 UNAVAILABLE response code is a rate limit`() { + assertEquals(ThrottleKind.RATE_LIMIT, classify("NO [UNAVAILABLE] System temporarily overloaded")?.kind) + } + + @Test + fun `an rfc5530 LIMIT response code is a rate limit`() { + assertEquals(ThrottleKind.RATE_LIMIT, classify("NO [LIMIT] too much")?.kind) + } + + @Test + fun `an smtp too-many-messages rejection is a rate limit`() { + assertEquals(ThrottleKind.RATE_LIMIT, classify("421 4.7.0 Too many messages, try later")?.kind) + } + + @Test + fun `an http 429 that surfaced as text is a rate limit`() { + assertEquals(ThrottleKind.RATE_LIMIT, classify("Graph sendMail failed (HTTP 429): quota")?.kind) + } + + @Test + fun `an explicit rate-limit phrase is a rate limit`() { + assertEquals(ThrottleKind.RATE_LIMIT, classify("Request was rate-limited by the provider")?.kind) + } + + // --- lockout positives ---------------------------------------------------------------------- + + @Test + fun `a temporarily-locked account is a lockout`() { + assertEquals(ThrottleKind.LOCKOUT, classify("Your account has been temporarily locked")?.kind) + } + + @Test + fun `a suspended account is a lockout`() { + assertEquals(ThrottleKind.LOCKOUT, classify("This account is temporarily suspended")?.kind) + } + + @Test + fun `too many login attempts is a lockout`() { + assertEquals(ThrottleKind.LOCKOUT, classify("Login failed: too many login attempts")?.kind) + } + + @Test + fun `a message naming both a lock and a rate limit prefers the longer lockout`() { + assertEquals(ThrottleKind.LOCKOUT, classify("Too many requests; account locked")?.kind) + } + + // --- negatives (must NOT be throttling) ----------------------------------------------------- + + @Test + fun `a wrong-password auth failure is not throttling`() { + assertNull(classify("A2 NO [AUTHENTICATIONFAILED] Invalid credentials (Failure)")) + } + + @Test + fun `the imap-disabled state is not throttling`() { + // Issue #390's actionable "turn on IMAP" case must stay distinct from a throttle/lockout. + assertNull(classify("Your account is not enabled for IMAP use. Please enable IMAP.")) + } + + @Test + fun `an ordinary network error is not throttling`() { + assertNull(ThrottleClassifier.classify(IOException("Connection refused"))) + } + + @Test + fun `a not-found error is not throttling`() { + assertNull(classify("Message 42 not found")) + } + + @Test + fun `a null or blank message is not throttling`() { + assertNull(classify(null)) + assertNull(classify(" ")) + } + + // --- cause chain ---------------------------------------------------------------------------- + + @Test + fun `throttle wording nested in a cause is still classified`() { + val wrapped = RuntimeException("sync failed", IOException("[THROTTLED] too many requests")) + assertEquals(ThrottleKind.RATE_LIMIT, ThrottleClassifier.classify(wrapped)?.kind) + } + + @Test + fun `a cyclic cause chain terminates and classifies`() { + val a = RuntimeException("Too many requests") + val b = RuntimeException("wrapper", a) + a.initCause(b) // cycle: a -> b -> a + assertEquals(ThrottleKind.RATE_LIMIT, ThrottleClassifier.classify(b)?.kind) + } + + // --- http status entry point ---------------------------------------------------------------- + + @Test + fun `http 429 maps to a rate limit and honors a parsed retry-after`() { + assertEquals( + ThrottleSignal(ThrottleKind.RATE_LIMIT, retryAfterMillis = 120_000L), + ThrottleClassifier.classifyHttpStatus(429, retryAfterMillis = 120_000L), + ) + } + + @Test + fun `http 503 maps to a rate limit`() { + assertEquals(ThrottleKind.RATE_LIMIT, ThrottleClassifier.classifyHttpStatus(503)?.kind) + } + + @Test + fun `a 2xx status is not throttling`() { + assertNull(ThrottleClassifier.classifyHttpStatus(200)) + assertNull(ThrottleClassifier.classifyHttpStatus(401)) + } +} diff --git a/config/detekt/detekt.yml b/config/detekt/detekt.yml index e42921a..72bbd0b 100644 --- a/config/detekt/detekt.yml +++ b/config/detekt/detekt.yml @@ -70,6 +70,9 @@ style: - '**/data/sync/MailSyncConcurrencyTest.kt' - '**/data/sync/PruneWorkerTest.kt' - '**/data/sync/BackfillWorkerTest.kt' + # #360 throttle gate: onThrottle/onSuccess breadcrumb through AppLog, which forwards to + # android.util.Log (a throwing JVM stub), so this suite mockkStatic(Log) too. + - '**/data/sync/AccountThrottleGateTest.kt' # Reader-path perf logging (issue #358): the repository's openMessage and the reader ViewModel # log via AppLog, so their unit tests mockkStatic(Log) too. - '**/data/repository/MailRepositoryImplTest.kt'