From 84c1fa7ccef4e8e49f17d643f360ec0f184683fb Mon Sep 17 00:00:00 2001 From: Owen Jacobson Date: Mon, 5 May 2025 23:12:22 -0400 Subject: Don't retry operations where we received an unacceptable response. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- ui/lib/apiServer.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) (limited to 'ui/lib/apiServer.js') 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 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; } -- cgit v1.2.3