Don't retry operations where we received an unacceptable response.

This was actually two issues in one!

Issue 1: `isRetryable` did not consider whether we got a response or not. It assumed that the presence of a request in the error signaled that the error was definitely due to network issues, when in fact it's the presence of a request _and_ the absence of a response. That's my misreading of the Axios docs; the replacement `isRetryable` is more thorough.

Issue 2: operations in the outbox queue that fail with an exception stop the outbox drain process from making further progress, _and_ they stay in the queue. The outbox now dequeues jobs that throw an exception, and restarts itself if it terminates with a non-empty queue. The code that does this is _heinous_, but it seems to work well enough… Words I'm sure I won't come to regret.

2 files changed, 30 insertions, 3 deletions

diff --git a/ui/lib/apiServer.js b/ui/lib/apiServer.js
index cad8997..e682681 100644
--- a/ui/lib/apiServer.js
+++ b/ui/lib/apiServer.js
@@ -87,5 +87,21 @@ function responseError(err) {
 }
 
 function isRetryable(err) {
-  return !!err.request;
+  // See <https://axios-http.com/docs/handling_errors> for a breakdown of this logic.
+
+  // Any error with a response is non-retryable. The server responded; we get to act on that
+  // response. We don't do anything special for 5xx-series responses yet, but they might one day be
+  // retryable too.
+  if (err.response) {
+    return false;
+  }
+
+  // Any error with no response and a request is probably from the network side of things, and is
+  // retryable.
+  if (err.request) {
+    return true;
+  }
+
+  // Anything with neither is unexpected enough that we should not try it.
+  return false;
 }
diff --git a/ui/lib/outbox.svelte.js b/ui/lib/outbox.svelte.js
index de7f80e..fd7fdba 100644
--- a/ui/lib/outbox.svelte.js
+++ b/ui/lib/outbox.svelte.js
@@ -69,6 +69,14 @@ export class Outbox {
     // rather than spreading it across multiple methods.
     this.sending = this.drain().finally(() => {
       this.sending = null;
+
+      // If we encounter an exception processing the pending queue, it may have an operation left
+      // in it. If so, start over. The exception will still propagate out (though since nothing
+      // ever awaits the promise from this.sending, it'll ultimately leak out to the browser
+      // anyways).
+      if (this.pending.length > 0) {
+        this.start();
+      }
     });
   }
 
@@ -76,8 +84,11 @@ export class Outbox {
     while (this.pending.length > 0) {
       const operation = this.pending[0];
 
-      await operation.send();
-      this.pending.shift();
+      try {
+        await operation.send();
+      } finally {
+        this.pending.shift();
+      }
     }
   }
 }