Unable to get review requests

[kingofzeal]

I'm attempting to get a list of reviews for a given pull request, but for some reason the library never seems to return a value, even though I can see the request return correct data via Fiddler.

I've browsed the integration tests around this endpoint, and have even gone so far as to make the same call with identical arguments, and still can't get the library to return a value.

Just to ensure I'm not doing anything weird, I have attempted to get this data the following ways:

var reviews1 = await GitHubClient(accessToken).PullRequest.Review.GetAll("octokit", "octokit.net", 1648);
...
var reviews2 = await GithubClient(accessToken).Repository.PullRequest.Review.Getall("octokit", "octokit.net", 1648);

In both cases, if I set a breakpoint on the line prior to calling and step over, the debugger never resumes. Using fiddler, I can see a request being made to /repos/octokit/octokit.net/pulls/1648/reviews as expected and a well-formatted response. This issue also occurs if I point to a PR which has no reviews at all, where the API response is simply an empty array.

I am using a custom IHttpClient, similar to one at Octokit.Caching so I can take advantage of conditional requests. However, prior to making this particular call there are 50-100 other calls, majority of which return from cache, and the requests which do not come from the cache behave as expected, so I do not think it's my implementation of the client (in fact, the request seems to get lost when I call Send of the default HttpClientAdapter).

I've attempted to run the integration tests around these methods, but I can't seem to be able to debug or run them, even though I believe my environment is setup correctly as per the contributing guidelines.

[M-Zuber]

I threw this code into LinqPad and it seems to run fine

var g = new GitHubClient(new ProductHeaderValue("test-bug-report"));
var x = g.PullRequest.Review.GetAll("octokit", "octokit.net", 1648).GetAwaiter().GetResult();

// lingpad specific, instead of Console.Write...
x.Dump();

What happens if you remove your custom IHttpClient entirely instead of just calling the default one from within the custom one?

[kingofzeal]

I changed to use the default client, effectively doing:

var client = new GitHubClient(new ProductHeaderValue("GithubDashboard"));
var reviews = await client.PullRequest.Review.GetAll("octokit", "octokit.net", 1648);

And it is still hanging.

Spinning up a new Console Application, this doesn't seem to occur.

I reviewed and refactored my project to be a little easier to follow - I was doing some heavy linq, including an AsParallel() call, and after removing that and refactoring to use a single thread for the heavy lifting everything seems to work, but this isn't ideal as the application would need to get branch, pull and review information from multiple repositories (which in this case can take 1-2 minutes to obtain).

I'll see if I can find a way to replicate this issue more reliably (or determine if it's just user error).

[M-Zuber]

👍 Keep us updated so we can try and help

[kingofzeal]

After refactoring I can't seem to replicate the issue.

I have a feeling it was related to how I was making the requests (via long complex awaited linq statement, which has nested awaited linq statements that I probably wasn't handling properly). I've since changed it to be a little more sensible, and everything works as expected.

Thanks for the help!

[M-Zuber]

Where you able to work around the issue of things taking too long?

[kingofzeal]

Well, I was able to re-implement the way I was trying to speed things up (using .AsParallel() on virtually every collection), but it's still taking some time - mostly because there are so many requests (one for the user, plus two per repo they have access to (branch + PR list) and one per open PR (for reviews)).

It would be nice if I could batch requests to the API, or if more information came back on certain calls (like having Reviews come back as part of a PR request), but until the API supports it I'll have to live with what I have.

[ryangribble]

In terms of the original problem raised in the issue it sounds like you were getting into crazy async/await territory and getting a deadlock situation... we "should" have all of our awaits properly configured with .ConfigureAwait(false) as per #1248 but it's possible to have missed some. I'll flick through the particular calls and double check.

In terms of your comment about needing to make so many API calls, yes indeed it is the case with REST APIs but the good news is that GitHub have a GraphQL API available in preview, which is an entirely different paradigm and allows you to make single requests grabbing graphs of information (eg pull requests meeting some criteria, with user comment and commit details, but only the fields you want instead of the entire objects etc) rather than lots and lots of REST calls. There is also some great work going on in an Octokit implementation for this at Octokit.GraphQL which is currently in Alpha, if you wanted to kick the tyres 👍