Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(GraphQL): fix interface conversion panic in v20.03 #5857

Merged
merged 2 commits into from
Jul 8, 2020

Conversation

abhimanyusinghgaur
Copy link
Contributor

@abhimanyusinghgaur abhimanyusinghgaur commented Jul 7, 2020

Fixes DGRAPH-1787.

This PR fixes an issue, where if some user accidentally deleted the dgraph.type edge for the GraphQL schema node and simultaneously added another GraphQL schema node with dgraph.type edge and restarted the alpha, it would end up crashing with a panic saying panic: interface conversion: interface {} is nil, not string.

Steps to reproduce:

  • Start zero and alpha.
  • Run following mutation:
    {
      set {
        _:newSchema <dgraph.type> "dgraph.graphql" .
      }
      delete {
        <0x1> <dgraph.type> * .
      }
    }
    
    Note that <0x1> is the uid of existing GraphQL schema node. What we did in this mutation is to remove the dgraph.type edge from the existing schema node, and add a new node with just dgraph.type edge.
  • Restart alpha.
  • It will panic, and crash.
    The reason it happens is that the code in 20.03 used to do the initial GraphQL schema upsert using the type query, and when you restart the alpha, it will find the new node which doesn't have the dgraph.graphql.schema edge. So, it comes as nil in query, and hence the panic.

This change is Reviewable

Docs Preview: Dgraph Preview

Copy link
Contributor

@martinmr martinmr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: Include steps to reproduce in PR description.

Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @abhimanyusinghgaur and @manishrjain)


graphql/admin/admin.go, line 453 at r1 (raw file):

existingGQLSchemaQryResp

this can be gqlSchemaQryResp

@danielmai danielmai merged commit 1e26c84 into release/v20.03 Jul 8, 2020
@danielmai danielmai deleted the abhimanyu/sentry-panic-fix branch July 8, 2020 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants