Consistently convert GraphQL errors to OctoshiftCliExceptions with helpful error messages across GithubApi (#868)

In `GithubApi`, we have the helpful `EnsureSuccessGraphQLResponse`
helper which checks the GraphQL response for errors, and if an error is
found, it raises a `OctoshiftCliException`.

Currently, this helper is not used consistently across GraphQL API
calls, which means that we have some cases where the GraphQL API returns
an error and the CLI crashes with an unhelpful
`System.InvalidOperationException: Cannot access child value on
Newtonsoft.Json.Linq.JValue.`.

To find out what actually went wrong, the user has to either:

* (a) use the `--verbose` option or
* (b) manually check the verbose log file

This PR creates a new `PostGraphQLAsync` method on `GitHubClient` which
includes the error handling logic from `EnsureSuccessGraphQLResponse`.

We then use the `PostGraphQLAsync` method instead of `PostAsync` for
GraphQL requests, which means that we get this consistent error handling
for free.

I do not apply this to `ReclaimMannequin` (which has its own error
handling) and `GetMannequins` (which has some complex pagination logic
which I didn't want to mess with right now).

Fixes #867.

<!--
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
-->

- [x] Did you write/update appropriate tests
- [x] Release notes updated (if appropriate)
- [x] Appropriate logging output
- [x] Issue linked
- [ ] Docs updated (or issue created)
- [ ] New package licenses are added to `ThirdPartyNotices.txt` (if
applicable)

<!--
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
-->

---------

Co-authored-by: Arin Ghazarian <aringhazarian@github.com>

Author: timrogers

RELEASENOTES.md

@@ -1,3 +1,4 @@
 - Adds retry logic during GHES archive generation in cases of transient failure
 - Added log output linking to migration log URL after migration completes
 - Add support for specifying `--archive-download-host` with `gh bbs2gh migrate-repo` and `gh bbs2gh generate-script`, rather than taking the host from the `--bbs-server-url`
+- Improve handling of GraphQL errors, throwing an exception with the specific error message returned by the API

src/Octoshift/GithubApi.cs

@@ -167,10 +167,7 @@ public virtual async Task<string> GetOrganizationId(string org)
 
             var response = await _retryPolicy.Retry(async () =>
             {
-                var httpResponse = await _client.PostAsync(url, payload);
-                var data = JObject.Parse(httpResponse);
-
-                EnsureSuccessGraphQLResponse(data);
+                var data = await _client.PostGraphQLAsync(url, payload);
 
                 return (string)data["data"]["organization"]["id"];
             });
@@ -192,10 +189,7 @@ public virtual async Task<string> GetEnterpriseId(string enterpriseName)
 
             var response = await _retryPolicy.Retry(async () =>
             {
-                var httpResponse = await _client.PostAsync(url, payload);
-                var data = JObject.Parse(httpResponse);
-
-                EnsureSuccessGraphQLResponse(data);
+                var data = await _client.PostGraphQLAsync(url, payload);
 
                 return (string)data["data"]["enterprise"]["id"];
             });
@@ -227,8 +221,7 @@ public virtual async Task<string> CreateAdoMigrationSource(string orgId, string
                 operationName = "createMigrationSource"
             };
 
-            var response = await _client.PostAsync(url, payload);
-            var data = JObject.Parse(response);
+            var data = await _client.PostGraphQLAsync(url, payload);
 
             return (string)data["data"]["createMigrationSource"]["migrationSource"]["id"];
         }
@@ -253,8 +246,7 @@ public virtual async Task<string> CreateBbsMigrationSource(string orgId)
                 operationName = "createMigrationSource"
             };
 
-            var response = await _client.PostAsync(url, payload);
-            var data = JObject.Parse(response);
+            var data = await _client.PostGraphQLAsync(url, payload);
 
             return (string)data["data"]["createMigrationSource"]["migrationSource"]["id"];
         }
@@ -279,8 +271,7 @@ public virtual async Task<string> CreateGhecMigrationSource(string orgId)
                 operationName = "createMigrationSource"
             };
 
-            var response = await _client.PostAsync(url, payload);
-            var data = JObject.Parse(response);
+            var data = await _client.PostGraphQLAsync(url, payload);
 
             return (string)data["data"]["createMigrationSource"]["migrationSource"]["id"];
         }
@@ -351,10 +342,7 @@ mutation startRepositoryMigration(
                 operationName = "startRepositoryMigration"
             };
 
-            var response = await _client.PostAsync(url, payload);
-            var data = JObject.Parse(response);
-
-            EnsureSuccessGraphQLResponse(data);
+            var data = await _client.PostGraphQLAsync(url, payload);
 
             return (string)data["data"]["startRepositoryMigration"]["repositoryMigration"]["id"];
         }
@@ -395,10 +383,7 @@ mutation startOrganizationMigration (
                 operationName = "startOrganizationMigration"
             };
 
-            var response = await _client.PostAsync(url, payload);
-            var data = JObject.Parse(response);
-
-            EnsureSuccessGraphQLResponse(data);
+            var data = await _client.PostGraphQLAsync(url, payload);
 
             return (string)data["data"]["startOrganizationMigration"]["orgMigration"]["id"];
         }
@@ -414,10 +399,7 @@ mutation startOrganizationMigration (
 
             var response = await _retryPolicy.Retry(async () =>
             {
-                var httpResponse = await _client.PostAsync(url, payload);
-                var data = JObject.Parse(httpResponse);
-
-                EnsureSuccessGraphQLResponse(data);
+                var data = await _client.PostGraphQLAsync(url, payload);
 
                 return (
                     State: (string)data["data"]["node"]["state"],
@@ -458,10 +440,7 @@ public virtual async Task<string> StartBbsMigration(string migrationSourceId, st
 
             var response = await _retryPolicy.Retry(async () =>
             {
-                var httpResponse = await _client.PostAsync(url, payload);
-                var data = JObject.Parse(httpResponse);
-
-                EnsureSuccessGraphQLResponse(data);
+                var data = await _client.PostGraphQLAsync(url, payload);
 
                 return (
                     State: (string)data["data"]["node"]["state"],
@@ -494,10 +473,7 @@ public virtual async Task<string> GetMigrationLogUrl(string org, string repo)
 
             var response = await _retryPolicy.Retry(async () =>
             {
-                var httpResponse = await _client.PostAsync(url, payload);
-                var data = JObject.Parse(httpResponse);
-
-                EnsureSuccessGraphQLResponse(data);
+                var data = await _client.PostGraphQLAsync(url, payload);
 
                 var nodes = (JArray)data["data"]["organization"]["repositoryMigrations"]["nodes"];
 
@@ -555,8 +531,7 @@ public virtual async Task<bool> GrantMigratorRole(string org, string actor, stri
 
             try
             {
-                var response = await _client.PostAsync(url, payload);
-                var data = JObject.Parse(response);
+                var data = await _client.PostGraphQLAsync(url, payload);
 
                 return (bool)data["data"]["grantMigratorRole"]["success"];
             }
@@ -582,8 +557,7 @@ public virtual async Task<bool> RevokeMigratorRole(string org, string actor, str
 
             try
             {
-                var response = await _client.PostAsync(url, payload);
-                var data = JObject.Parse(response);
+                var data = await _client.PostGraphQLAsync(url, payload);
 
                 return (bool)data["data"]["revokeMigratorRole"]["success"];
             }
@@ -697,10 +671,9 @@ public virtual async Task<string> GetUserId(string login)
             };
 
             // TODO: Add retry logic here, but need to inspect the actual error message and differentiate between transient failure vs user doesn't exist (only retry on failure)
-            var response = await _client.PostAsync(url, payload);
-            var data = JObject.Parse(response);
+            var data = await _client.PostGraphQLAsync(url, payload);
 
-            return data["data"]["user"].Any() ? (string)data["data"]["user"]["id"] : null;
+            return (string)data["data"]["user"]["id"];
         }
 
         public virtual async Task<MannequinReclaimResult> ReclaimMannequin(string orgId, string mannequinId, string targetUserId)
@@ -828,16 +801,6 @@ private static Mannequin BuildMannequin(JToken mannequin)
             };
         }
 
-        private void EnsureSuccessGraphQLResponse(JObject response)
-        {
-            if (response.TryGetValue("errors", out var jErrors) && jErrors is JArray { Count: > 0 } errors)
-            {
-                var error = (JObject)errors[0];
-                var errorMessage = error.TryGetValue("message", out var jMessage) ? (string)jMessage : null;
-                throw new OctoshiftCliException($"{errorMessage ?? "UNKNOWN"}");
-            }
-        }
-
         private static GithubSecretScanningAlert BuildSecretScanningAlert(JToken secretAlert) =>
             new()
             {

src/Octoshift/GithubClient.cs

@@ -73,6 +73,18 @@ public virtual async IAsyncEnumerable<JToken> GetAllAsync(string url, Dictionary
         public virtual async Task<string> PostAsync(string url, object body, Dictionary<string, string> customHeaders = null) =>
             (await SendAsync(HttpMethod.Post, url, body, customHeaders: customHeaders)).Content;
 
+        public virtual async Task<JToken> PostGraphQLAsync(
+            string url,
+            object body,
+            Dictionary<string, string> customHeaders = null)
+        {
+            var response = await PostAsync(url, body, customHeaders);
+            var data = JObject.Parse(response);
+            EnsureSuccessGraphQLResponse(data);
+
+            return data;
+        }
+
         public virtual async IAsyncEnumerable<JToken> PostGraphQLWithPaginationAsync(
             string url,
             object body,
@@ -237,5 +249,15 @@ private void SetRetryDelay(IEnumerable<KeyValuePair<string, IEnumerable<string>>
 
             _retryDelay = (int)(resetUnixSeconds - currentUnixSeconds);
         }
+
+        private void EnsureSuccessGraphQLResponse(JObject response)
+        {
+            if (response.TryGetValue("errors", out var jErrors) && jErrors is JArray { Count: > 0 } errors)
+            {
+                var error = (JObject)errors[0];
+                var errorMessage = error.TryGetValue("message", out var jMessage) ? (string)jMessage : null;
+                throw new OctoshiftCliException($"{errorMessage ?? "UNKNOWN"}");
+            }
+        }
     }
 }

src/Octoshift/ReclaimService.cs

@@ -99,10 +99,6 @@ public virtual async Task ReclaimMannequin(string mannequinUser, string mannequi
             }
 
             var targetUserId = await _githubApi.GetUserId(targetUser);
-            if (targetUserId == null)
-            {
-                throw new OctoshiftCliException($"Target user {targetUser} not found.");
-            }
 
             var success = true;