Prevented errors being incorrectly considered as rate limiting errors (…

…#784)

<!--
For the checkboxes below you must check each one to indicate that you
either did the relevant task, or considered it and decided there was
nothing that needed doing
-->

Fixes #722 

- [x] Did you write/update appropriate tests
- [x] Release notes updated (if appropriate)
- [x] Appropriate logging output
- [x] Issue linked
- [x] Docs updated (or issue created)

<!--
For docs we should review the docs at:

https://docs.github.com/en/early-access/github/migrating-with-github-enterprise-importer
and the README.md in this repo

If a doc update is required based on the changes in this PR, it is
sufficient to create an issue and link to it here. The doc update can be
made later/separately.

The process to update the docs can be found here:
https://github.com/github/docs-early-access#opening-prs

The markdown files are here: 

https://github.com/github/docs-early-access/tree/main/content/github/migrating-with-github-enterprise-importer
-->

RELEASENOTES.md

@@ -3,3 +3,4 @@
 - Fix log output so we don't say we've finished upload to Azure Blob Storage when you're actually using Amazon S3
 - Extend the expiration of blob storage signed URLs from 24hrs to 48hrs so migration can still be successful even if there is a long queue of migrations
 - Skip the upload to Azure/AWS blob storage when migrating from GHES 3.8+, as GHES will now handle putting the archives into blob storage
+- Fixed a bug where bad credentials were incorrectly being treated as rate-limit errors

src/Octoshift/GithubClient.cs

@@ -161,7 +161,7 @@ public virtual async Task<string> PatchAsync(string url, object body, Dictionary
                 _log.LogDebug($"RESPONSE HEADER: {header.Key} = {string.Join(",", header.Value)}");
             }
 
-            if (GetRateLimitRemaining(headers) <= 0)
+            if (GetRateLimitRemaining(headers) <= 0 && content.ToUpper().Contains("API RATE LIMIT EXCEEDED"))
             {
                 SetRetryDelay(headers);
             }

src/OctoshiftCLI.Tests/GithubClientTests.cs

@@ -235,7 +235,7 @@ public async Task GetAsync_Applies_Retry_Delay()
 
             using var secondHttpResponse = new HttpResponseMessage(HttpStatusCode.OK)
             {
-                Content = new StringContent("SECOND_RESPONSE")
+                Content = new StringContent("API RATE LIMIT EXCEEDED blah blah blah")
             };
 
             secondHttpResponse.Headers.Add("X-RateLimit-Reset", retryAt.ToString());
@@ -285,7 +285,7 @@ public async Task GetAsync_Applies_Retry_Delay_If_Forbidden()
 
             using var secondHttpResponse = new HttpResponseMessage(HttpStatusCode.Forbidden)
             {
-                Content = new StringContent("SECOND_RESPONSE")
+                Content = new StringContent("API RATE LIMIT EXCEEDED blah blah blah")
             };
 
             secondHttpResponse.Headers.Add("X-RateLimit-Reset", retryAt.ToString());
@@ -332,6 +332,65 @@ public async Task PostAsync_Returns_String_Response()
             actualContent.Should().Be(EXPECTED_RESPONSE_CONTENT);
         }
 
+        [Fact]
+        public async Task PostAsync_Does_Not_Apply_Retry_Delay_To_Bad_Credentials_Response()
+        {
+            // Arrange
+            var now = DateTimeOffset.Now.ToUnixTimeSeconds();
+            var retryAt = now + 4;
+
+            _dateTimeProvider.Setup(m => m.CurrentUnixTimeSeconds()).Returns(now);
+
+            using var badCredentialResponse1 = new HttpResponseMessage(HttpStatusCode.Unauthorized)
+            {
+                Content = new StringContent("{\"message\":\"Bad credentials\",\"documentation_url\":\"https://docs.github.com/graphql\"}")
+            };
+
+            badCredentialResponse1.Headers.Add("X-RateLimit-Reset", retryAt.ToString());
+            badCredentialResponse1.Headers.Add("X-RateLimit-Remaining", "0");
+
+            using var badCredentialResponse2 = new HttpResponseMessage(HttpStatusCode.Unauthorized)
+            {
+                Content = new StringContent("{\"message\":\"Bad credentials\",\"documentation_url\":\"https://docs.github.com/graphql\"}")
+            };
+
+            badCredentialResponse2.Headers.Add("X-RateLimit-Reset", retryAt.ToString());
+            badCredentialResponse2.Headers.Add("X-RateLimit-Remaining", "0");
+
+            var handlerMock = new Mock<HttpMessageHandler>();
+            handlerMock
+                .Protected()
+                .SetupSequence<Task<HttpResponseMessage>>(
+                    "SendAsync",
+                    ItExpr.Is<HttpRequestMessage>(req => req.Method == HttpMethod.Post),
+                    ItExpr.IsAny<CancellationToken>())
+                .ReturnsAsync(badCredentialResponse1)
+                .ReturnsAsync(badCredentialResponse2);
+
+            using var httpClient = new HttpClient(handlerMock.Object);
+            var githubClient = new GithubClient(_mockOctoLogger.Object, httpClient, null, _retryPolicy, _dateTimeProvider.Object, PERSONAL_ACCESS_TOKEN);
+
+            // Act
+            await FluentActions
+                .Invoking(async () =>
+                {
+                    await githubClient.PostAsync("http://example.com", "hello");
+                })
+                .Should()
+                .ThrowExactlyAsync<HttpRequestException>();
+
+            await FluentActions
+                .Invoking(async () =>
+                {
+                    await githubClient.PostAsync("http://example.com", "hello");
+                })
+                .Should()
+                .ThrowAsync<HttpRequestException>();
+
+            // Assert
+            _mockOctoLogger.Verify(m => m.LogWarning(It.IsAny<string>()), Times.Never);
+        }
+
         [Fact]
         public async Task PostAsync_Applies_Retry_Delay()
         {
@@ -348,7 +407,7 @@ public async Task PostAsync_Applies_Retry_Delay()
 
             using var secondHttpResponse = new HttpResponseMessage(HttpStatusCode.OK)
             {
-                Content = new StringContent("SECOND_RESPONSE")
+                Content = new StringContent("API RATE LIMIT EXCEEDED blah blah blah")
             };
 
             secondHttpResponse.Headers.Add("X-RateLimit-Reset", retryAt.ToString());
@@ -398,7 +457,7 @@ public async Task PostAsync_Applies_Retry_Delay_If_Forbidden()
 
             using var secondHttpResponse = new HttpResponseMessage(HttpStatusCode.Forbidden)
             {
-                Content = new StringContent("SECOND_RESPONSE")
+                Content = new StringContent("API RATE LIMIT EXCEEDED blah blah blah")
             };
 
             secondHttpResponse.Headers.Add("X-RateLimit-Reset", retryAt.ToString());
@@ -590,7 +649,7 @@ public async Task PutAsync_Applies_Retry_Delay()
 
             using var secondHttpResponse = new HttpResponseMessage(HttpStatusCode.OK)
             {
-                Content = new StringContent("SECOND_RESPONSE")
+                Content = new StringContent("API RATE LIMIT EXCEEDED blah blah blah")
             };
 
             secondHttpResponse.Headers.Add("X-RateLimit-Reset", retryAt.ToString());
@@ -778,7 +837,7 @@ public async Task PatchAsync_Applies_Retry_Delay()
 
             using var secondHttpResponse = new HttpResponseMessage(HttpStatusCode.OK)
             {
-                Content = new StringContent("SECOND_RESPONSE")
+                Content = new StringContent("API RATE LIMIT EXCEEDED blah blah blah")
             };
 
             secondHttpResponse.Headers.Add("X-RateLimit-Reset", retryAt.ToString());
@@ -952,7 +1011,7 @@ public async Task DeleteAsync_Applies_Retry_Delay()
 
             using var secondHttpResponse = new HttpResponseMessage(HttpStatusCode.OK)
             {
-                Content = new StringContent("SECOND_RESPONSE")
+                Content = new StringContent("API RATE LIMIT EXCEEDED blah blah blah")
             };
 
             secondHttpResponse.Headers.Add("X-RateLimit-Reset", retryAt.ToString());