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

Improve handling of selections that would only consist of __typename #6456

Open
lukastaegert opened this issue Aug 19, 2024 · 0 comments
Open

Comments

@lukastaegert
Copy link

Is your feature request related to a problem? Please describe.

In 33e8146, you implemented a change that aborted the query with an error if it would habe __typename in its selection set. That is good, as it finally allows us to handle a long-standing issue with our setup, but we would suggest to further refine the behavior.

In our project, we use a custom solution based on stitchSchemas and especially delegateToSchema to implement a strangler pattern where we want to gradually replace an old service A with a new service B behind a common GraphQL gateway.
Here, the schema of B is mostly a subset of the schema of A. If stitchSchemas determines that a top-level field is supported by both services, then we use delegateToSchema to send the request to both services and then implement a custom merge logic to merge the results.

There is one problem though. Assume we have these schemas:

# Service A
type Query {
  foo: Foo
}

type Foo {
  first: String
  second: String
}

# Service B
type Query {
  foo: Foo
}

type Foo {
  first: String
}

And this query:

query {
  foo {
    __typename
    second
  }
}

stichSchemas will determine that both services implement Query.foo and we will therefore pass the query to delegateToSchema, which in turn will find out that we want to send this query to B:

query {
  foo {
    __typename
  }
}

With the old logic, this generated a lot of useless requests while the new logic now throws an error that we can catch and handle! (Technically, this also means that this patch release was a breaking change for us, but we do not mind, we pinned the dependency for now)

However, catching this error means we need to rely on the exact wording of the error message as we still want to handle other errors as before. Also, using errors as expected control flow like this probably also carries some performance penalty (though we did not test this).

Describe the solution you'd like

We want a clean way to ignore requests that would only select __typename of any field. Ideally, these fields would return null. It would be ok for this behavior to be opt-in via option. Alternatively, there could be a hook to handle these cases.

Describe alternatives you've considered

If we want to stick with throwing errors, it would also help to add a stable error code to the error that we can test for.

Additional context

See above. If you think our approach could be improved in some way that would avoid this problem differently, let us know!

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

1 participant