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

Make GraphQL server ignore unknown fields in queries #2676

Open
netrome opened this issue Feb 6, 2025 · 5 comments
Open

Make GraphQL server ignore unknown fields in queries #2676

netrome opened this issue Feb 6, 2025 · 5 comments
Assignees

Comments

@netrome
Copy link
Contributor

netrome commented Feb 6, 2025

Context

Currently, if someone requests a field from a GraphQL object that doesn't exist in the schema we return an error. This poses a problem, because it makes adding new fields to the schema a breaking change. I.e. someone running a new client version is unable to communicate with an older version of the API. If we instead start ignoring if someone requests extra fields, we'd run into less problems with breaking changes.

Definition of done

The GraphQL API is tolerant towards client requesting non-existing fields.

@acerone85 acerone85 self-assigned this Feb 6, 2025
@Dentosal
Copy link
Member

Dentosal commented Feb 9, 2025

I don't think this is a good idea. The whole point of GraphQL is that you only request fields that you need. Having a client that just always requests all fields goes against this idea. If you need the field but an older version doesn't provide it, then that request cannot be ran against the older version. If you don't need the field, don't request it.

If we insist on having a client that doesn't customize queries per use case, it should probably request version (or capability) information from the GraphQL API, and only send appropriate queries.

@netrome netrome mentioned this issue Feb 10, 2025
7 tasks
@netrome
Copy link
Contributor Author

netrome commented Feb 10, 2025

I don't think this is a good idea. The whole point of GraphQL is that you only request fields that you need. Having a client that just always requests all fields goes against this idea. If you need the field but an older version doesn't provide it, then that request cannot be ran against the older version. If you don't need the field, don't request it.

You make a good point. As you say, the problem is that we're maintaining a client library that hides GraphQL from the end-user. I like the idea of just requesting the necessary information. It seems like cynic has an attribute skip_serializing_if which we can use on optional fields to omit them from queries if they're not set: https://cynic-rs.dev/derives/input-objects#field-attributes

@acerone85 if you've started to look into this I'd be curious about your thoughts. I know you're a fan of builders, so perhaps we should provide builders for our query types so we can add fields without breaking existing downstream code. That would de-facto have the same effect as ignoring unknown fields on the server side, but be more robust and idiomatic.

@acerone85
Copy link
Contributor

@acerone85 if you've started to look into this I'd be curious about your thoughts. I know you're a fan of builders, so perhaps we should provide builders for our query types so we can add fields without breaking existing downstream code. That would de-facto have the same effect as ignoring unknown fields on the server side, but be more robust and idiomatic.

I agree with @Dentosal. In general, my philosophy is that complexity should be added to the less critical components when possible. In this case that would mean at least adding a endpoint to retrieve the schema in the node, and let clients do the filtering of unknown fields. Even better, we could add a endpoint in the node that simply returns a schema version, and let the client retrieve the right schema from the schema version, which is not far different from what @Dentosal proposes. In the latter case, we'll need a strategy about how we communicate changes to the schema so that third party clients can update accordingly, perhaps in the fuel-specs?

@acerone85
Copy link
Contributor

acerone85 commented Feb 17, 2025

Actually we can already return the whole graphql schema, I have tried this with a local node syncing with mainnet:

file graphql_schema_introspection.json, copied from (https://stackoverflow.com/questions/61468834/how-to-request-the-schema-from-a-graphql-service-using-curl)

{ 
  "query": "query IntrospectionQuery {
      __schema {
        queryType { name }
        mutationType { name }
        subscriptionType { name }
        types {
          ...FullType
        }
        directives {
          name
          description
          locations
          args {
            ...InputValue
          }
        }
      }
    }

    fragment FullType on __Type {
      kind
      name
      description
      fields(includeDeprecated: true) {
        name
        description
        args {
          ...InputValue
        }
        type {
          ...TypeRef
        }
        isDeprecated
        deprecationReason
      }
      inputFields {
        ...InputValue
      }
      interfaces {
        ...TypeRef
      }
      enumValues(includeDeprecated: true) {
        name
        description
        isDeprecated
        deprecationReason
      }
      possibleTypes {
        ...TypeRef
      }
    }

    fragment InputValue on __InputValue {
      name
      description
      type { ...TypeRef }
      defaultValue
    }

    fragment TypeRef on __Type {
      kind
      name
      ofType {
        kind
        name
        ofType {
          kind
          name
          ofType {
            kind
            name
            ofType {
              kind
              name
              ofType {
                kind
                name
                ofType {
                  kind
                  name
                  ofType {
                    kind
                    name
                  }
                }
              }
            }
          }
        }
      }
    }"
}

then run

curl -i -X POST http://localhost:4000/v1/graphql -H "Content-Type: application/json" -d @ graphql_schema_introspection

@netrome
Copy link
Contributor Author

netrome commented Feb 17, 2025

Actually we can already return the whole graphql schema, I have tried this with a local node syncing with mainnet:

file graphql_schema_introspection.json, copied from (https://stackoverflow.com/questions/61468834/how-to-request-the-schema-from-a-graphql-service-using-curl)

{
"query": "query IntrospectionQuery {
__schema {
queryType { name }
mutationType { name }
subscriptionType { name }
types {
...FullType
}
directives {
name
description
locations
args {
...InputValue
}
}
}
}

fragment FullType on __Type {
  kind
  name
  description
  fields(includeDeprecated: true) {
    name
    description
    args {
      ...InputValue
    }
    type {
      ...TypeRef
    }
    isDeprecated
    deprecationReason
  }
  inputFields {
    ...InputValue
  }
  interfaces {
    ...TypeRef
  }
  enumValues(includeDeprecated: true) {
    name
    description
    isDeprecated
    deprecationReason
  }
  possibleTypes {
    ...TypeRef
  }
}

fragment InputValue on __InputValue {
  name
  description
  type { ...TypeRef }
  defaultValue
}

fragment TypeRef on __Type {
  kind
  name
  ofType {
    kind
    name
    ofType {
      kind
      name
      ofType {
        kind
        name
        ofType {
          kind
          name
          ofType {
            kind
            name
            ofType {
              kind
              name
              ofType {
                kind
                name
              }
            }
          }
        }
      }
    }
  }
}"

}

then run

curl -i -X POST http://localhost:4000/v1/graphql -H "Content-Type: application/json" -d @ graphql_schema_introspection

As much as this could be helpful, I'm wondering if the overhead is worth it. This would create a lot of extra network communication when doing simple queries. And I'm not sure about the gain, since the caller would get a similar error in both cases. E.g. "missing field X on type Y".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants