Randomly unable to fetch commit author fields (`author`, `authoredDate`, `authoredByCommitter`) – API returns 502s


#1

Hi,

This relates to Sporadic 502s fetching PR data.

From our experience with the GraphQL API, these sporadic 502s are still happening quite frequently.

However, it appears that you’ve recently released something that results in consistent 502s for certain queries that used to work fine. Specifically, when fetching commits on pull request pages, for certain pages we get consistent 502 responses from your API when the fields author, authoredDate, and authoredByCommitter are requested. After removing them, the query no longer gets consistent 502 responses (it may still get some but a couple of retries resolve that).

Here are some ids your 502 responses asked to quote:

  • C936:1323:409A87:7D477D:5A9DAB49
  • C948:1320:1E989B:44816B:5A9DAB71
  • C95E:1322:33DBF5:81607F:5A9DAB99
  • B726:1322:372689:89D87F:5A9DB536
  • B732:1320:20872B:48E846:5A9DB55F
  • DA4E:1323:4546D6:862E15:5A9DB587

And here’s a sample failing query:

query {
  repository(owner: "redacted" name: "redacted") {
    name
    pullRequests(last: 30 before: "redacted") {
      edges {
        node {
          number
          author {
            avatarUrl
            login
            url
          }
          baseRefName
          body
          closed
          createdAt
          headRefName
          locked
          mergeCommit {
            author {
              avatarUrl
              date
              email
              name
              user {
                avatarUrl
                login
                name
                url
              }
            }
            authoredByCommitter
            committedDate
            committer {
              avatarUrl
              date
              email
              name
              user {
                avatarUrl
                login
                name
                url
              }
            }
            messageBody
            messageBodyHTML
            messageHeadline
            messageHeadlineHTML
            oid
            status {
              contexts {
                context
                createdAt
                creator {
                  avatarUrl
                  login
                  url
                }
                description
                state
                targetUrl
              }
              state
            }
            url
          }
          mergeable
          mergedAt
          potentialMergeCommit {
            status {
              contexts {
                context
                createdAt
                creator {
                  avatarUrl
                  login
                  url
                }
                description
                state
                targetUrl
              }
              state
            }
          }
          state
          suggestedReviewers {
            isAuthor
            isCommenter
            reviewer {
              avatarUrl
              login
              name
              url
            }
          }
          title
          url
          commits(last: 100 ) {
            edges {
              node {
                commit {
                  author {
                    avatarUrl
                    date
                    email
                    name
                    user {
                      avatarUrl
                      login
                      name
                      url
                    }
                  }
                  authoredDate
                  authoredByCommitter
                  commitUrl
                  committedDate
                  committer {
                    avatarUrl
                    date
                    email
                    name
                    user {
                      avatarUrl
                      login
                      name
                      url
                    }
                  }
                  message
                  messageBody
                  messageBodyHTML
                  messageHeadline
                  messageHeadlineHTML
                  oid
                  status {
                    contexts {
                      context
                      createdAt
                      creator {
                        avatarUrl
                        login
                        url
                      }
                      description
                      state
                      targetUrl
                    }
                    state
                  }
                  url
                }
                url
              }
            }
            pageInfo { startCursor hasPreviousPage }
            totalCount
          }
        }
      }
      pageInfo { startCursor hasPreviousPage }
      totalCount
    }
  }
}

and the corresponding successful query without the commit author-related fields:

query {
  repository(owner: "redacted" name: "redacted") {
    name
    pullRequests(last: 30 before: "redacted") {
      edges {
        node {
          number
          author {
            avatarUrl
            login
            url
          }
          baseRefName
          body
          closed
          createdAt
          headRefName
          locked
          mergeCommit {
            committedDate
            committer {
              avatarUrl
              date
              email
              name
              user {
                avatarUrl
                login
                name
                url
              }
            }
            messageBody
            messageBodyHTML
            messageHeadline
            messageHeadlineHTML
            oid
            status {
              contexts {
                context
                createdAt
                creator {
                  avatarUrl
                  login
                  url
                }
                description
                state
                targetUrl
              }
              state
            }
            url
          }
          mergeable
          mergedAt
          potentialMergeCommit {
            status {
              contexts {
                context
                createdAt
                creator {
                  avatarUrl
                  login
                  url
                }
                description
                state
                targetUrl
              }
              state
            }
          }
          state
          suggestedReviewers {
            isAuthor
            isCommenter
            reviewer {
              avatarUrl
              login
              name
              url
            }
          }
          title
          url
          commits(last: 100 ) {
            edges {
              node {
                commit {
                  commitUrl
                  committedDate
                  committer {
                    avatarUrl
                    date
                    email
                    name
                    user {
                      avatarUrl
                      login
                      name
                      url
                    }
                  }
                  message
                  messageBody
                  messageBodyHTML
                  messageHeadline
                  messageHeadlineHTML
                  oid
                  status {
                    contexts {
                      context
                      createdAt
                      creator {
                        avatarUrl
                        login
                        url
                      }
                      description
                      state
                      targetUrl
                    }
                    state
                  }
                  url
                }
                url
              }
            }
            pageInfo { startCursor hasPreviousPage }
            totalCount
          }
        }
      }
      pageInfo { startCursor hasPreviousPage }
      totalCount
    }
  }
}

This took down a bunch of production data pipelines. We’ve now patched them by not requesting commit author-related fields when we run into issues, but we still need a fix as soon as possible.

Thank you.
Nick


#2

Update: in some cases dropping commit author related fields is not sufficient :frowning: (e.g. D3A0:4F10:27330E3:5DECD0B:5AA001B7)


#3

Hi @nomeyer,

I just gave this a try on a repository that I have access to and was also able to trigger a timeout. Looking into the query, it’s quite large and has the potential to return an immense amount of data (>1MB payload) from a variety of places in GitHub’s architecture.

I’m not familiar with any changes that might have triggered this, but it’s certainly possible. It’s also possible that you’re iterating over particular pull requests that have a lot of data - say, for example, a large body attribute.

For queries as large as this one, I’d recommend reducing the total number of nodes that you’re returning by decreasing the last argument on the pullRequest connection, which has the trade off of needing to make more requests to the API.


#4

Hi @bswinnerton, thanks, we’ll give that a try.

Is returning a response that would identify such scenarios (as opposed to plain 502s) something you would consider? It would be quite handy to be able to isolate those scenarios and handle them by reducing page size.

Thank you
Nick


#5

Absolutely. In a perfect world we’d return an appropriate resource limit error before the request times out (ultimately with a 502), but tweaking the resource limits is a tricky process that has the potential to introduce breaking changes - for example, if we make an internal change that results in a higher “cost” for a query, it could break existing queries.

I’ll bring this up with the larger team :heart:.