Do not reject settings calls when the settings file cannot be saved

CHANGELOG.md

@@ -133,6 +133,38 @@ and the errno name when the cause chain has one. A failed rename or
 directory create on Android now also says why, for example `Permission
 denied`. The code stays `E_STORAGE`.
 
+`setWifiOnly()`, `updateHeaders()`, `pause()` and `resume()` no longer
+reject `E_STORAGE` when the settings file cannot be saved, on both
+platforms. The new value applies in memory at once, so this session uses
+it. The failed save is logged with its cause. On Android,
+`takeDiagnostics()` returns it. On iOS, it goes to the device log only for
+now. The library saves again at the next settings call, when the app
+becomes active, and when it leaves the foreground. These retries happen
+only while the process lives. A new process uses the last value that
+reached the disk, so an app should send its settings again at each start.
+A call with an unchanged value writes nothing. Before, one failed save at
+startup kept the old value for the whole session, and an app that stopped
+on the rejection sent nothing. `configure()` also applies its retry
+defaults when the save fails; before, they were dropped. See "Settings on
+a full disk" in the README. `updateHeaders()` with an invalid header still
+rejects `E_INVALID` and changes nothing.
+
+On Android, `updateHeaders()`, `pause()` and `resume()` still reject
+`E_STORAGE` when an entry file cannot be saved, because Android reads the
+entries from disk. They now change and schedule every other entry first.
+Before, the first failed entry stopped the loop, and entries already saved
+as `queued` got no work until the next launch. An entry that failed keeps
+its old state.
+
+On Android, `resume()` and the launch sweep now compare a parked entry with
+its own header generation, not the one in the settings file. Before, after
+a settings save that never landed, a resumed entry that already had new
+headers could go back to `awaiting-auth`.
+
+The header generation no longer goes back after a relaunch that read an
+older settings file. iOS raises it at launch above every entry and task
+record. Android raises it above every entry at each `updateHeaders()`.
+
 ## 11.1.0
 
 Added `takeDiagnostics()`. On Android it returns the module's recent

README.md

@@ -436,6 +436,10 @@ event (`paused`, then `queued`). No outcome is produced and the bytes are
 kept. A paused entry past `expiresAt` settles `error` with
 `errorKind: 'expired'` when it is resumed.
 
+Neither call rejects because the settings cannot be saved. On Android,
+either call can still reject `E_STORAGE` when an entry file cannot be saved.
+See [Settings on a full disk](#settings-on-a-full-disk).
+
 ### `cancel(id): Promise<void>`
 A live entry settles `cancelled` with reason `user` and is forgotten after
 its ack. A settled entry is forgotten now: row, bytes, and its
@@ -458,14 +462,61 @@ its own value. Both are checked before each attempt, so a toggle moves the
 queued entries that follow the setting. A call with the current value
 resolves without a write.
 
+It does not reject. The new value applies at once, also when the disk is
+full. See [Settings on a full disk](#settings-on-a-full-disk).
+
 ### `updateHeaders(patch): Promise<void>`
 Merges the patch into the headers of every entry not yet forgotten and
 resumes the entries parked on `awaiting-auth`. The patch also replaces
 same-named headers a part carries. This is how a fresh token
 reaches requests that stalled on 401. Each call bumps a header generation: a
 401 or 403 from an attempt issued under an older generation re-issues at once
 instead of parking. Parking emits one `state` event per entry. A header name
-or value the platform HTTP client cannot send rejects with `E_INVALID`.
+or value the platform HTTP client cannot send rejects with `E_INVALID` and
+changes nothing. A settings file that cannot be saved does not reject. On
+Android, an entry file that cannot be saved does reject `E_STORAGE`. See
+[Settings on a full disk](#settings-on-a-full-disk).
+
+### Settings on a full disk
+`setWifiOnly()`, `updateHeaders()`, `pause()`, `resume()` and `configure()`
+change queue settings that native saves to a file. They apply the new value
+in memory first, so this session uses it at once. Then they save the file.
+If the save fails, for example on a full disk, the call still resolves.
+
+The library records a warning. On Android, `takeDiagnostics()` returns it.
+On iOS, the warning goes to the device log only for now. It will go to
+`takeDiagnostics()` when iOS has a diagnostics list.
+
+The library tries the save again at the next one of these calls, when the
+app becomes active, and when the app leaves the foreground. These retries
+happen only while the process lives. A new process, for example a relaunch
+in the background with no JS, uses the last value that reached the disk:
+
+- `setWifiOnly()`: the earlier Wi-Fi setting applies.
+- `pause()` and `resume()`: the earlier pause state applies. At launch,
+  native moves the entries to match it. So an entry that was paused can run,
+  and an entry that was resumed can wait.
+- `updateHeaders()`: the headers are saved with each entry, not in the
+  settings file, so they are not lost. Only a counter of header updates is.
+  Native raises that counter again above the entries' values, so a 401 still
+  re-issues as described above.
+
+So send your settings again at each app start: call `setWifiOnly()`, and
+`pause()` or `resume()`, with the values you want. A call with the value
+native already has writes nothing.
+
+`updateHeaders()`, `pause()` and `resume()` also save each entry they
+change. On iOS, a failed entry save is logged, the entry changes in memory,
+and the next save of that entry writes it. On Android, the entries are read
+from disk, so they cannot change in memory only. There, the call changes and
+schedules every entry it can save, then rejects `E_STORAGE` when one or more
+entries could not be saved. Each entry that failed keeps its old state:
+
+- `updateHeaders()`: it keeps its old headers and stays in `awaiting-auth`
+  if it was there. Call `updateHeaders()` again.
+- `pause()`: it keeps its state, but its worker sees the pause before its
+  next attempt.
+- `resume()`: it stays `paused`. Call `resume()` again.
 
 ### `getRequests(filter?): RequestRow[]`
 Synchronous, from native's in-memory index, so it works offline. Returns

android/src/main/java/ai/openspace/backgroundupload/EntryTransitions.kt

@@ -58,9 +58,14 @@ object EntryTransitions {
   fun toPaused(e: QueueEntry, now: Long) =
     e.copy(state = EntryState.PAUSED, nextAttemptAt = null, updatedAt = now)
 
-  /** resume(): back to awaiting-auth only when no updateHeaders() came in between. */
-  fun toResumed(e: QueueEntry, headerGeneration: Int, now: Long): QueueEntry =
-    if (e.parkedGeneration != null && e.parkedGeneration == headerGeneration) {
+  /**
+   * resume(): back to awaiting-auth only when no updateHeaders() came in
+   * between. It compares the entry's own headerGeneration, which
+   * updateHeaders() saves with the new headers. The settings value can be
+   * older after a settings write that never landed.
+   */
+  fun toResumed(e: QueueEntry, now: Long): QueueEntry =
+    if (e.parkedGeneration != null && e.parkedGeneration == e.headerGeneration) {
       e.copy(state = EntryState.AWAITING_AUTH, updatedAt = now)
     } else {
       e.copy(state = EntryState.QUEUED, parkedGeneration = null, updatedAt = now)

android/src/main/java/ai/openspace/backgroundupload/QueueController.kt

@@ -186,37 +186,47 @@ class QueueController(
    * paused set, and an empty list changes nothing. Each live row that is now
    * paused (gate on, or its key in the set) moves to paused and its work
    * stops. No outcome.
+   *
+   * A failed settings write does not throw (see [QueueSettingsStore.update]).
+   * A failed entry save throws E_STORAGE after every other entry is moved
+   * and its work stopped. The entry that failed keeps its old row; its
+   * worker sees the paused gate before its next attempt.
    */
   fun pause(keys: List<String>? = null) {
     if (keys != null && keys.isEmpty()) return
     val s = settings.update {
       if (keys == null) it.copy(paused = true) else it.copy(pausedKeys = it.pausedKeys + keys)
     }
     val now = clock()
-    val paused = transformAll { e ->
+    val result = transformAll { e ->
       if (e.isLive && e.state != EntryState.PAUSED && s.isPaused(e.key)) EntryTransitions.toPaused(e, now) else e
     }
-    paused.forEach { scheduler.cancel(it.id) }
-    paused.forEach { events.state(it.toRow()) }
+    result.changed.forEach { scheduler.cancel(it.id) }
+    result.changed.forEach { events.state(it.toRow()) }
+    result.throwIfFailed("pause")
   }
 
   /**
    * Undoes the pause of the same scope: [keys] null turns the gate off, a
    * list removes those keys from the set. A row comes back only when no
    * scope still pauses it, so the gate on + its key resumed stays paused.
+   *
+   * A failed entry save throws E_STORAGE after every other entry is moved
+   * and every queued entry is scheduled. The entry that failed stays paused.
    */
   fun resume(keys: List<String>? = null) {
     if (keys != null && keys.isEmpty()) return
     val s = settings.update {
       if (keys == null) it.copy(paused = false) else it.copy(pausedKeys = it.pausedKeys - keys.toSet())
     }
     val now = clock()
-    val resumed = transformAll { e ->
-      if (e.state == EntryState.PAUSED && !s.isPaused(e.key)) EntryTransitions.toResumed(e, s.headerGeneration, now) else e
+    val result = transformAll { e ->
+      if (e.state == EntryState.PAUSED && !s.isPaused(e.key)) EntryTransitions.toResumed(e, now) else e
     }
-    resumed.forEach { events.state(it.toRow()) }
+    result.changed.forEach { events.state(it.toRow()) }
     // Every queued entry, not only the resumed ones: a run is idempotent.
     store.all().forEach { scheduleRun(it) }
+    result.throwIfFailed("resume")
   }
 
   /**
@@ -316,6 +326,11 @@ class QueueController(
     generation = e.generation,
   )
 
+  /**
+   * The settings calls (this one, [pause], [resume], [configureRetry],
+   * [updateHeaders]) never throw for a failed settings write. See
+   * [QueueSettingsStore.update].
+   */
   fun setWifiOnly(enabled: Boolean) {
     settings.update { it.copy(wifiOnly = enabled) }
   }
@@ -330,21 +345,30 @@ class QueueController(
    * own headerGeneration, which changes here under the store lock together
    * with its headers. So a worker that gets a 401 either sees the patched
    * entry and re-issues, or parks first and is requeued here.
+   *
+   * A failed entry save throws E_STORAGE after every other entry is patched
+   * and the unparked ones are scheduled. The entry that failed keeps its old
+   * headers on disk and stays parked if it was. A later call patches it.
    */
   fun updateHeaders(patch: Map<String, String>) {
-    val s = settings.update { it.copy(headerGeneration = it.headerGeneration + 1) }
+    // Above every entry's value too. After a failed settings write and a
+    // relaunch, the file can hold an older generation than the entries, and
+    // a 401 under the newest one must still see newer headers.
+    val entriesMax = store.all().maxOfOrNull { it.headerGeneration } ?: 0
+    val s = settings.update { it.copy(headerGeneration = maxOf(it.headerGeneration, entriesMax) + 1) }
     val now = clock()
     val unparkedIds = mutableSetOf<String>()
-    val changed = transformAll { e ->
+    val result = transformAll { e ->
       val patched = e.withHeadersPatched(patch, s.headerGeneration)
       if (patched.state != EntryState.AWAITING_AUTH) return@transformAll patched
       unparkedIds += e.id
       EntryTransitions.toUnparked(patched, s.isPaused(patched.key), now)
     }
-    changed.filter { it.id in unparkedIds }.forEach { entry ->
+    result.changed.filter { it.id in unparkedIds }.forEach { entry ->
       scheduleRun(entry)
       events.state(entry.toRow())
     }
+    result.throwIfFailed("updateHeaders")
   }
 
   // MARK: - journal
@@ -460,7 +484,7 @@ class QueueController(
           if (paused && cur.state != EntryState.PAUSED) {
             cur = EntryTransitions.toPaused(cur, now)
           } else if (!paused && cur.state == EntryState.PAUSED) {
-            cur = EntryTransitions.toResumed(cur, s.headerGeneration, now)
+            cur = EntryTransitions.toResumed(cur, now)
           }
           if (cur !== e) {
             if (trySave(cur)) changed += cur else continue
@@ -543,16 +567,42 @@ class QueueController(
     if (!isWorkerRunning(id)) scheduler.cancelWake(id)
   }
 
-  /** Applies [transform] to every non-legacy entry under the store lock. Returns the ones it changed. */
-  private fun transformAll(transform: (QueueEntry) -> QueueEntry): List<QueueEntry> {
+  /** The entries [transformAll] saved, and the saves that failed. */
+  private class Transformed(val changed: List<QueueEntry>, val failures: Int, val firstFailure: IOException?) {
+    /** Throws E_STORAGE when an entry save failed. Call it after the work for [changed] is done. */
+    fun throwIfFailed(method: String) {
+      val e = firstFailure ?: return
+      throw QueueException(
+        QueueException.E_STORAGE,
+        "$method: could not save $failures entries: ${e.message}" + StorageCause.suffix(e, withMessage = false),
+        e,
+      )
+    }
+  }
+
+  /**
+   * Applies [transform] to every non-legacy entry under the store lock. An
+   * entry whose save fails keeps its old row, and the loop goes on, so one
+   * full-disk failure does not leave the other entries unchanged.
+   */
+  private fun transformAll(transform: (QueueEntry) -> QueueEntry): Transformed {
     val changed = mutableListOf<QueueEntry>()
+    var failures = 0
+    var first: IOException? = null
     store.locked {
       store.all().filter { !it.legacy }.forEach { e ->
-        store.compute(e.id) { cur ->
-          if (cur == null) null else transform(cur).also { if (it !== cur) changed += it }
+        try {
+          var next: QueueEntry? = null
+          store.compute(e.id) { cur ->
+            if (cur == null) null else transform(cur).also { if (it !== cur) next = it }
+          }
+          next?.let { changed += it } // only once the save landed
+        } catch (error: IOException) {
+          failures++
+          if (first == null) first = error
         }
       }
     }
-    return changed
+    return Transformed(changed, failures, first)
   }
 }

android/src/main/java/ai/openspace/backgroundupload/QueueSettings.kt

@@ -81,6 +81,14 @@ class QueueSettingsStore(private val file: File) {
 
   private var cached: QueueSettings? = null
 
+  /**
+   * True while the file holds an older value than the cache, because a
+   * write failed. The next [update] and [saveIfBehind] try again.
+   */
+  @get:Synchronized
+  var isBehind = false
+    private set
+
   @Synchronized
   fun load(): QueueSettings {
     cached?.let { return it }
@@ -91,20 +99,52 @@ class QueueSettingsStore(private val file: File) {
   }
 
   /**
-   * Throws IOException when the write fails. The cache then keeps the old
-   * value. A [transform] that changes nothing writes nothing, so a call such
-   * as setWifiOnly() at every app start does not fail on a full disk.
+   * Applies [transform] to the cache at once, then writes the file. A failed
+   * write does not throw: this process uses the new value, a warning is
+   * recorded, and the file stays behind until a later write in this process
+   * lands. A new process (a background relaunch with no JS) reads the last
+   * value that reached the file.
+   *
+   * A [transform] that changes nothing writes nothing, unless the file is
+   * behind. So a call such as setWifiOnly() at every app start does not
+   * write on a full disk.
    */
   @Synchronized
   fun update(transform: (QueueSettings) -> QueueSettings): QueueSettings {
     val current = load()
     val next = transform(current)
-    if (next == current) return current
-    AtomicFiles.writeText(file, gson.toJson(next))
+    if (next == current) {
+      saveIfBehind()
+      return current
+    }
     cached = next
+    write(next)
     return next
   }
 
+  /** Writes the cache when an earlier write failed. The module calls it when the app enters and leaves the foreground. */
+  @Synchronized
+  fun saveIfBehind() {
+    if (isBehind) write(load())
+  }
+
+  private fun write(s: QueueSettings) {
+    try {
+      AtomicFiles.writeText(file, gson.toJson(s))
+      isBehind = false
+    } catch (e: Exception) {
+      // Only the first failure in a row is persisted, so retries on a full
+      // disk do not crowd the diagnostic log.
+      Diag.warn(
+        "could not save the queue settings; this session uses the new value, and the next settings " +
+          "call or app foreground tries again" + StorageCause.suffix(e),
+        e,
+        persist = !isBehind,
+      )
+      isBehind = true
+    }
+  }
+
   // Gson does not run constructors, so absent fields read as null or 0.
   @Suppress("SENSELESS_COMPARISON", "USELESS_ELVIS", "UNNECESSARY_SAFE_CALL")
   private fun validated(s: QueueSettings?): QueueSettings {