Fix failed queries in cache not re-fetching if they have previously s…

…ucceeded
Jcparkyn · Nov 23, 2023 · ba9d1d3 · ba9d1d3
1 parent 83e20c6
commit ba9d1d3
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 9 deletions.
diff --git a/src/Phetch.Core/Query.cs b/src/Phetch.Core/Query.cs
@@ -317,9 +317,10 @@ public async Task<TResult> SetArgAsync(TArg arg)
             _currentQuery?.RemoveObserver(this);
             newQuery.AddObserver(this);
             _currentQuery = newQuery;
-            // TODO: Is this the best behavior?
 
-            if (!newQuery.IsFetching && newQuery.IsStaleByTime(_staleTime, DateTime.Now))
+            var shouldRefetch = !newQuery.IsFetching &&
+                (newQuery.Status == QueryStatus.Error || newQuery.IsStaleByTime(_staleTime, DateTime.Now));
+            if (shouldRefetch)
             {
                 return await newQuery.RefetchAsync(_options?.RetryHandler).ConfigureAwait(false);
             }
@@ -328,12 +329,10 @@ public async Task<TResult> SetArgAsync(TArg arg)
         {
             return await task;
         }
-        else
-        {
-            // Probably not possible to get here, but just in case
-            Debug.Fail("newQuery should have been invoked before this point");
-            return newQuery.Data!;
-        }
+
+        // Probably not possible to get here, but just in case
+        Debug.Fail("newQuery should have been invoked before this point");
+        return newQuery.Data!;
     }
 
     /// <inheritdoc/>

diff --git a/test/Phetch.Tests/MockQueryFunction.cs b/test/Phetch.Tests/MockQueryFunction.cs
@@ -27,7 +27,7 @@ public class MockQueryFunction<TArg, TResult>(int numSources)
 
     public async Task<TResult> Query(TArg arg)
     {
-        if (_queryCount > Sources.Count)
+        if (_queryCount >= Sources.Count)
             throw new Exception("Query function called too many times");
         var resultTask = Sources[_queryCount].Task;
         _queryCount++;

diff --git a/test/Phetch.Tests/Query/QueryTests.cs b/test/Phetch.Tests/Query/QueryTests.cs
@@ -394,6 +394,44 @@ public async Task Invoke_should_work()
         result.Should().Be("2");
     }
 
+    [UIFact]
+    public async Task SetArg_should_always_refetch_if_error()
+    {
+        var qf = new MockQueryFunction<int, string>(4);
+        var endpoint = new Endpoint<int, string>(qf.Query, new()
+        {
+            // Disable automatic refetching
+            DefaultStaleTime = TimeSpan.MaxValue,
+        });
+        var query = endpoint.Use();
+
+        // Trigger an initial success. This ensures that _dataUpdatedAt is set, because otherwise
+        // this test passes "for free".
+        var task1 = query.SetArgAsync(0);
+        qf.Sources[0].SetResult("0");
+        await task1;
+
+        // Refetch with failure
+        var task2 = query.RefetchAsync();
+        qf.Sources[1].SetException(new IndexOutOfRangeException("BOOM!"));
+        await task2.Invoking(t => t)
+            .Should().ThrowExactlyAsync<IndexOutOfRangeException>();
+
+        // Currently, setting the same arg twice will never refetch, which is intentional.
+        // Instead we just change the arg twice.
+        _ = query.SetArgAsync(1);
+        var task3 = query.SetArgAsync(0);
+        qf.Sources[3].SetResult("0 again");
+        await task3;
+
+        using (new AssertionScope())
+        {
+            query.Data.Should().Be("0 again");
+            AssertIsSuccessState(query);
+            qf.Calls.Should().Equal(0, 0, 1, 0);
+        }
+    }
+
     private static void AssertIsIdleState<TArg, TResult>(Query<TArg, TResult> query)
     {
         query.Status.Should().Be(QueryStatus.Idle);