fix: Prevent unnecessary retries when checking repository existence b…

RELEASENOTES.md

@@ -1 +1 @@
-
+- Fixed an issue where repository existence checks would incorrectly retry on expected responses (200/404/301), causing unnecessary delays during migrations. The check now only retries on transient server errors (5xx status codes) while responding immediately to deterministic states.

src/Octoshift/Services/GithubClient.cs

@@ -45,7 +45,13 @@ public GithubClient(OctoLogger log, HttpClient httpClient, IVersionProvider vers
         }
     }
 
-    public virtual async Task<string> GetNonSuccessAsync(string url, HttpStatusCode status) => (await GetWithRetry(url, expectedStatus: status)).Content;
+    public virtual async Task<string> GetNonSuccessAsync(string url, HttpStatusCode status)
+    {
+        return (await _retryPolicy.HttpRetry(
+            async () => await SendAsync(HttpMethod.Get, url, expectedStatus: status),
+            ex => ex.StatusCode.HasValue && (int)ex.StatusCode.Value >= 500
+        )).Content;
+    }
 
     public virtual async Task<string> GetAsync(string url, Dictionary<string, string> customHeaders = null) => (await GetWithRetry(url, customHeaders)).Content;
 

src/OctoshiftCLI.Tests/Octoshift/Services/GithubClientTests.cs

@@ -1063,27 +1063,120 @@ public async Task GetNonSuccessAsync_Logs_The_GitHub_Request_Id_Header_Value()
     }
 
     [Fact]
-    public async Task GetNonSuccessAsync_Retries_On_Non_Success()
+    public async Task GetNonSuccessAsync_Does_Not_Retry_On_Expected_Status()
     {
         // Arrange
         var handlerMock = new Mock<HttpMessageHandler>();
         handlerMock
             .Protected()
-            .SetupSequence<Task<HttpResponseMessage>>(
+            .Setup<Task<HttpResponseMessage>>(
                 "SendAsync",
                 ItExpr.Is<HttpRequestMessage>(req => req.Method == HttpMethod.Get),
                 ItExpr.IsAny<CancellationToken>())
-            .ReturnsAsync(CreateHttpResponseFactory(statusCode: HttpStatusCode.InternalServerError))
-            .ReturnsAsync(CreateHttpResponseFactory(statusCode: HttpStatusCode.Found, content: EXPECTED_RESPONSE_CONTENT));
+            .ReturnsAsync(CreateHttpResponseFactory(statusCode: HttpStatusCode.NotFound, content: "Not Found"));
 
         using var httpClient = new HttpClient(handlerMock.Object);
         var githubClient = new GithubClient(_mockOctoLogger.Object, httpClient, null, _retryPolicy, _dateTimeProvider.Object, PERSONAL_ACCESS_TOKEN);
 
         // Act
-        var result = await githubClient.GetNonSuccessAsync(URL, HttpStatusCode.Found);
+        var result = await githubClient.GetNonSuccessAsync(URL, HttpStatusCode.NotFound);
 
         // Assert
-        result.Should().Be(EXPECTED_RESPONSE_CONTENT);
+        result.Should().Be("Not Found");
+        handlerMock.Protected().Verify(
+            "SendAsync",
+            Times.Once(),
+            ItExpr.Is<HttpRequestMessage>(req => req.Method == HttpMethod.Get),
+            ItExpr.IsAny<CancellationToken>());
+    }
+
+    [Fact]
+    public async Task GetNonSuccessAsync_Does_Not_Retry_On_Unexpected_Success_Status()
+    {
+        // Arrange
+        var handlerMock = new Mock<HttpMessageHandler>();
+        handlerMock
+            .Protected()
+            .Setup<Task<HttpResponseMessage>>(
+                "SendAsync",
+                ItExpr.Is<HttpRequestMessage>(req => req.Method == HttpMethod.Get),
+                ItExpr.IsAny<CancellationToken>())
+            .ReturnsAsync(CreateHttpResponseFactory(statusCode: HttpStatusCode.OK, content: "Success"));
+
+        using var httpClient = new HttpClient(handlerMock.Object);
+        var githubClient = new GithubClient(_mockOctoLogger.Object, httpClient, null, _retryPolicy, _dateTimeProvider.Object, PERSONAL_ACCESS_TOKEN);
+
+        // Act & Assert - should throw immediately without retry
+        await FluentActions
+            .Invoking(async () => await githubClient.GetNonSuccessAsync(URL, HttpStatusCode.NotFound))
+            .Should()
+            .ThrowExactlyAsync<HttpRequestException>()
+            .WithMessage("*OK*");
+
+        handlerMock.Protected().Verify(
+            "SendAsync",
+            Times.Once(),
+            ItExpr.Is<HttpRequestMessage>(req => req.Method == HttpMethod.Get),
+            ItExpr.IsAny<CancellationToken>());
+    }
+
+    [Fact]
+    public async Task GetNonSuccessAsync_Does_Not_Retry_On_Server_Error()
+    {
+        // Arrange
+        var handlerMock = new Mock<HttpMessageHandler>();
+        handlerMock
+            .Protected()
+            .SetupSequence<Task<HttpResponseMessage>>(
+                "SendAsync",
+                ItExpr.Is<HttpRequestMessage>(req => req.Method == HttpMethod.Get),
+                ItExpr.IsAny<CancellationToken>())
+            .ReturnsAsync(CreateHttpResponseFactory(statusCode: HttpStatusCode.InternalServerError, content: "Server Error"))
+            .ReturnsAsync(CreateHttpResponseFactory(statusCode: HttpStatusCode.NotFound, content: "Not Found"));
+
+        using var httpClient = new HttpClient(handlerMock.Object);
+        var githubClient = new GithubClient(_mockOctoLogger.Object, httpClient, null, _retryPolicy, _dateTimeProvider.Object, PERSONAL_ACCESS_TOKEN);
+
+        // Act - should retry on 5xx and eventually succeed
+        var result = await githubClient.GetNonSuccessAsync(URL, HttpStatusCode.NotFound);
+
+        // Assert
+        result.Should().Be("Not Found");
+        handlerMock.Protected().Verify(
+            "SendAsync",
+            Times.Exactly(2),
+            ItExpr.Is<HttpRequestMessage>(req => req.Method == HttpMethod.Get),
+            ItExpr.IsAny<CancellationToken>());
+    }
+
+    [Fact]
+    public async Task GetNonSuccessAsync_Does_Not_Retry_On_Redirect_Status()
+    {
+        // Arrange
+        var handlerMock = new Mock<HttpMessageHandler>();
+        handlerMock
+            .Protected()
+            .Setup<Task<HttpResponseMessage>>(
+                "SendAsync",
+                ItExpr.Is<HttpRequestMessage>(req => req.Method == HttpMethod.Get),
+                ItExpr.IsAny<CancellationToken>())
+            .ReturnsAsync(CreateHttpResponseFactory(statusCode: HttpStatusCode.MovedPermanently, content: "Moved"));
+
+        using var httpClient = new HttpClient(handlerMock.Object);
+        var githubClient = new GithubClient(_mockOctoLogger.Object, httpClient, null, _retryPolicy, _dateTimeProvider.Object, PERSONAL_ACCESS_TOKEN);
+
+        // Act & Assert - should throw immediately without retry on redirect
+        await FluentActions
+            .Invoking(async () => await githubClient.GetNonSuccessAsync(URL, HttpStatusCode.NotFound))
+            .Should()
+            .ThrowExactlyAsync<HttpRequestException>()
+            .WithMessage("*MovedPermanently*");
+
+        handlerMock.Protected().Verify(
+            "SendAsync",
+            Times.Once(),
+            ItExpr.Is<HttpRequestMessage>(req => req.Method == HttpMethod.Get),
+            ItExpr.IsAny<CancellationToken>());
     }
 
     [Fact]