octokit / octokit.graphql.net

A GitHub GraphQL client library for .NET
MIT License
143 stars 50 forks source link

System.InvalidOperationException: Cannot access child value on Newtonsoft.Json.Linq.JValue #234

Open terrajobst opened 4 years ago

terrajobst commented 4 years ago

Repro

This query crashes. The involved repo is public, so you should be able to repro it from your end.

Code ```xml ``` ```C# class Program { static async Task Main(string[] args) { var apiKey = ""; var start = DateTimeOffset.Parse("2020-06-26T09:30:00"); var filter = new IssueFilters() { Assignee = "*", Milestone = "*", Since = start, }; var query = new Query() .Repository("runtime", "dotnet") .Issues(filterBy: filter) .AllPages() .Select(i => new FeedbackIssue { Owner = i.Repository.Owner.Login, Repo = i.Repository.Name, Number = i.Number, Title = i.Title, CreateAt = i.CreatedAt, Author = i.Author.Login, State = i.State, Url = i.Url, TimelineItems = i .TimelineItems(null, null, null, null, null, start, null) .AllPages() .Select(tl => tl.Switch(when => when.IssueComment(ic => new ApiTimelineComment { CreatedAt = ic.CreatedAt }).LabeledEvent(l => new ApiTimelineLabel { CreatedAt = l.CreatedAt }).ClosedEvent(c => new ApiTimelineClosure { CreatedAt = c.CreatedAt }))).ToList() }).Compile(); var productInformation = new ProductHeaderValue("octokit.graphql.net bug report"); var connection = new Connection(productInformation, apiKey); var vars = new Dictionary(); vars["owner"] = "dotnet"; vars["repo"] = "runtime"; try { var current = await connection.Run(query, vars); } catch (Exception ex) { Console.WriteLine(ex); } } } internal sealed class FeedbackIssue { public string Owner { get; set; } public string Repo { get; set; } public int Number { get; set; } public DateTimeOffset CreateAt { get; set; } public string Author { get; set; } public string Title { get; set; } public IssueState State { get; set; } public string Url { get; set; } public List TimelineItems { get; set; } public override string ToString() { return $"{Owner}/{Repo}#{Number}: {Title}"; } } internal abstract class ApiTimelineItem { public string Actor { get; set; } public DateTimeOffset CreatedAt { get; set; } } internal sealed class ApiTimelineComment : ApiTimelineItem { public string Id { get; set; } public string Body { get; set; } public string Url { get; set; } } internal sealed class ApiTimelineLabel : ApiTimelineItem { public string Name { get; set; } } internal sealed class ApiTimelineClosure : ApiTimelineItem { } ```

Expected behavior

The query succeeds.

Actual behavior

The query crashes System.InvalidOperationException:

System.InvalidOperationException: Cannot access child value on Newtonsoft.Json.Linq.JValue.
   at Newtonsoft.Json.Linq.JToken.get_Item(Object key)
   at Octokit.GraphQL.Core.Builders.Rewritten.Value.Switch[TResult](JToken source, IDictionary`2 selectors)
   at System.Linq.Enumerable.SelectIListIterator`2.ToList()
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Octokit.GraphQL.Core.Builders.Rewritten.List.ToSubqueryList[T](IEnumerable`1 source, ISubqueryRunner context, ISubquery subquery)
   at i => new FeedbackIssue() {Owner = i.Repository.Owner.Login, Repo = i.Repository.Name, Number = i.Number, Title = i.Title, CreateAt = i.CreatedAt, Author = i.Author.Login, State = i.State, Url = i.Url, TimelineItems = i.TimelineItems(null, null, null, null, null, Convert(Convert(value(OctokitGraphQLRepro.Program+<>c__DisplayClass0_0).start, Arg`1), Nullable`1), null).AllPages().Select(tl => tl.Switch(when => when.IssueComment(ic => new ApiTimelineComment() {CreatedAt = ic.CreatedAt}).LabeledEvent(l => new ApiTimelineLabel() {CreatedAt = l.CreatedAt}).ClosedEvent(c => new ApiTimelineClosure() {CreatedAt = c.CreatedAt}))).ToList()}(Closure , JToken )
   at System.Linq.Enumerable.SelectIListIterator`2.ToList()
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at Octokit.GraphQL.Core.Builders.Rewritten.List.ToSubqueryList[T](IEnumerable`1 source, ISubqueryRunner context, ISubquery subquery)
   at lambda_method33(Closure , JObject )
   at Octokit.GraphQL.Core.Deserializers.ResponseDeserializer.Deserialize[TResult](Func`2 deserialize, JObject data)
   at Octokit.GraphQL.Core.PagedQuery`1.Runner.RunPage(CancellationToken cancellationToken)
   at Octokit.GraphQL.ConnectionExtensions.Run[T](IConnection connection, ICompiledQuery`1 query, Dictionary`2 variables, CancellationToken cancellationToken)
   at OctokitGraphQLRepro.Program.Main(String[] args) in C:\Users\immol\source\repos\OctokitGraphQLRepro\OctokitGraphQLRepro\Program.cs:line 63
jcansdale commented 4 years ago

Hi @terrajobst 👋

There appears to be strange things going on with PAT scopes and what the GraphQL query returns.

I've created a repository here with your repro: https://github.com/jcansdale-test/terrajobst-issue-234

When a use a PAT with no scopes, I see the same exception as you: https://github.com/jcansdale-test/terrajobst-issue-234/runs/828496722?check_suite_focus=true#step:4:76

When I run the same query with a repo scoped PAT, it works fine: https://github.com/jcansdale-test/terrajobst-issue-234/runs/828498317?check_suite_focus=true#step:4:76

There appears to be some information missing when a no-scope PAT is used. 😕

image

This can be fixed with the following null check: https://github.com/jcansdale-test/terrajobst-issue-234/pull/1/files

Unfortunately this doesn't make it with with the default ${{ secrets.GITHUB_TOKEN }}, which makes it throw a different exception:

System.AggregateException: One or more errors occurred. (Resource not accessible by integration) (Resource not accessible by integration)
 ---> Octokit.GraphQL.Core.Deserializers.ResponseDeserializerException: Resource not accessible by integration
   --- End of inner exception stack trace ---
   at Octokit.GraphQL.Core.Deserializers.ResponseDeserializer.Deserialize(String data)
   at Octokit.GraphQL.Core.PagedQuery`1.Runner.RunPage(CancellationToken cancellationToken)
   at Octokit.GraphQL.ConnectionExtensions.Run[T](IConnection connection, ICompiledQuery`1 query, Dictionary`2 variables, CancellationToken cancellationToken)
   at Program.Main() in /home/runner/work/terrajobst-issue-234/terrajobst-issue-234/Program.cs:line 57
 ---> (Inner Exception #1) Octokit.GraphQL.Core.Deserializers.ResponseDeserializerException: Resource not accessible by integration<---

https://github.com/jcansdale-test/terrajobst-issue-234/runs/828523782?check_suite_focus=true#step:4:76

If you're running this from a GitHub Actions workflow, you could simply embed a no-scope PAT in the code. GitHub will automatically delete any scoped PATs pushed to a public repository, but it will leave no-scope PATs alone. 🙂

A no-scope PAT should be able to access all public information. I'll ping the API team about this tomorrow and see if they know what's going on!

jcansdale commented 4 years ago

For reference, here is the query that it having problems:

query {
  repository(name: "runtime", owner: "dotnet") {
    id
    issues(filterBy:  {
      assignee: "*",createdBy: null,labels: null,mentioned: null,milestone: "*",since: "2020-06-26T09:30:00+01:00",states: null,viewerSubscribed: null
    }, first: 100) {
      pageInfo {
        hasNextPage
        endCursor
      }
      nodes {
        id
        owner: repository {
          owner {
            login
          }
        }
        repo: repository {
          name
        }
        number
        title
        createAt: createdAt
        author {
          login
        }
        state
        url
        timelineItems(since: "2020-06-26T09:30:00+01:00", first: 100) {
          pageInfo {
            hasNextPage
            endCursor
          }
          nodes {
            __typename
            ... on IssueComment {
              createdAt
            }
            ... on LabeledEvent {
              createdAt
            }
            ... on ClosedEvent {
              createdAt
            }
          }
        }
      }
    }
  }
}

It works fine in https://developer.github.com/v4/explorer/ but no-scope and GitHub Actions tokens trip it up. 😢

terrajobst commented 4 years ago

There appears to be strange things going on with PAT scopes and what the GraphQL query returns.

I've created a repository here with your repro: https://github.com/jcansdale-test/terrajobst-issue-234

When a use a PAT with no scopes, I see the same exception as you: https://github.com/jcansdale-test/terrajobst-issue-234/runs/828496722?check_suite_focus=true#step:4:76

In my case, I was using an OAuth2 token with the read:org scope. I've since added repo scope because I need to write as well. However, not all authenticated users will have write access to the repos the query is executed against. I still need those results to be reliable/complete though.

Your workaround only solved part of the issue; I don't seem to get complete query results. Something causes the query to exclude issues; maybe there is another issue with the IssuesFilter (#235)?

jcansdale commented 4 years ago

@terrajobst,

However, not all authenticated users will have write access to the repos the query is executed against. I still need those results to be reliable/complete though.

I think there are only a few event types missing. For example, any ReadColumnsInProjectEvent will come up null. Is this an important one for you?

Your workaround only solved part of the issue; I don't seem to get complete query results. Something causes the query to exclude issues; maybe there is another issue with the IssuesFilter (#235)?

If you run the following query in https://developer.github.com/v4/explorer/, do you notice any missing issue?

query {
  repository(name: "runtime", owner: "dotnet") {
    issues(first: 100) {
      nodes
      {
        number title
      }      
    }
  }
}
terrajobst commented 4 years ago

I think there are only a few event types missing. For example, any ReadColumnsInProjectEvent will come up null. Is this an important one for you?

No, the only important ones are the ones that I Switch-ed over.

If you run the following query in https://developer.github.com/v4/explorer/, do you notice any missing issue?

Nope, the query result looks good to me.

theteladras commented 2 years ago

This issue seams not to be resolved? I have encountered the very same thing recently.