Cannot return null for non-nullable field TreeEntry.object


#1

Hi there,

Fetching the tree { entries { object { oid } } } of a commit can sometimes result in errors Cannot return null for non-nullable field TreeEntry.object.

It seems that the object { oid } is the same as the oid of the TreeEntry, so I can work with that for now, but just thought I’d let you know.

Sample query for which I get errors:

query {
  repository(owner: "Stepsize" name: "layer_desktop") {
    pullRequests(last: 10 ) {
      edges {
        node {
          commits(last: 10 ) {
            edges {
              node {
                commit {
                  tree {
                    entries {
                      object {
                        oid
                      }
                    }
                  }
                }
              }
            }
            pageInfo { startCursor hasPreviousPage }
            totalCount
          }
        }
      }
    }
  }
}

Thanks
Nick


Cannot return null for non-nullable field ReviewRequestedEvent.subject
Cannot return null for non-nullable field License.body
#2

Hey @nomeyer!

Thanks for the report - I’ve gone ahead and created an internal issue for this, and I’m working on reproducing it for myself.

Is there anything ‘special’ about the PRs/Commits you’re hitting this bug on? Anything to make it easier to isolate the bug would be much appreciated!


#3

Hey @nickvanw, thanks for your reply!

I ran the query above with pullRequests(last: 1), pullRequests(last: 2), and pullRequests(last: 3).

last: 1
No errors here. This most recent PR modified 3 existing files over 3 commits but didn’t create any new ones.

last: 2
1 error here. The 2nd most recent PR created a new file with the first commit, and modified it with a second commit.

last: 3
5 errors here. The 3rd most recent PR had 8 commits, only one of which created new trees (3 files & 1 submodule).

So that’s 4 + 1 new trees between the 2nd & 3rd most recent PRs, which matches the number of errors. Seems like a good place to start the investigation.

last: >3
Here things get a bit weird. With last: 4 I get 4 errors, 6 errors with last: 5, 4 errors with last: 6, 5 errors with last: 7, 4 errors with last: 8, 6 errors with last: 9, and 4 errors with last: 10.
Not sure how to explain this – I’d expect the number of errors to grow as more PRs are retrieved. Because of this weirdness I haven’t gone over these other PRs.

Hope this helps – let me know if there’s any other helpful info I could send over.


#4

Hey again @nomeyer :slight_smile: This issue is next on my list, but I wonder, do you mind giving it another try?

It’s possible that the fix for the previous issue also fixes this one, since they both involve stray nulls, under the hood :weary:

Would you mind running your tests again and letting me know what you find? (It’s a bit hard for me to properly replicate, since it depends so much on the specific repo, etc.)


#5

Hi @rmosolgo, thanks for checking in.

TLDR; I’m observing similar but not identical behaviour.

Some context about the repo I’m testing this on – we’re not contributing to it anymore, so things have stayed the same since the last time I ran the tests except that I created two test PRs for reasons that aren’t relevant here.

So when running the same tests again, varying the last argument, I’m offsetting the argument by 2 to account for the new PRs.

  • I observe the same behaviour for last: 3 today as I did for last: 1 in my previous post (no errors).
  • I observe the same behaviour for last: 4 today as I did for last: 2 in my previous post (1 error).
  • I observe the same behaviour for last: 5 today as I did for last: 3 in my previous post (5 errors).
  • Beyond that, I stop observing the same behaviour.

One thing that’s worth mentioning: one of the 2 new PRs created new files, yet fetching its trees doesn’t throw any errors (it’s one of the last 3 PRs for the repo), so I’m starting to doubt whether my initial suggestion that it has something to do with new trees being created is correct.

This is probably not what you were hoping for, sorry about that. Let me know if I can help in any other way.


#6

That’s great, thanks for all the details!

I think this is just one of the tradeoffs of serving GraphQL: the strongly-typed system provides good guarantees for clients, but it gets harder when the underlying data is … tricky :laughing:

I’ll take another look and follow up here in a bit.


#7

Hey again @nomeyer,

After digging into our git client code, I think one of the issues where was the non-null requirement of TreeEntry.object. Certainly the underlying storage allows for nulls in this case. So, I removed the non-null constraint.

At least this should be a good first step – your queries should finish successfully! But let me know what you find, and if anything else seems out of order!