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

Use of __typename affects presence of fragment spread data on interfaces #2587

Closed
carldunham opened this issue Feb 9, 2023 · 8 comments · Fixed by #3718
Closed

Use of __typename affects presence of fragment spread data on interfaces #2587

carldunham opened this issue Feb 9, 2023 · 8 comments · Fixed by #3718
Assignees

Comments

@carldunham
Copy link

carldunham commented Feb 9, 2023

Describe the bug

We are seeing a difference in behavior between Apollo Gateway and Router that looks like a bug in Router.
Not sure what the spec really should be, but we have these in the wild.

Given schema:

schema {
  query: Query
}

type Query {
  animal: Animal!
  dog: Dog!
}

interface Animal {
  id: String!
}

type Dog implements Animal {
  id: String!
  name: String!
}

Issuing the query

query dog {
  dog {
    ...on Animal {
      id
      # __typename
      ...on Dog {
        name
      }
    }
  }
}

Gives the result

{
  "data": {
    "dog": {
      "id": "123"
    }
  }
}

Uncomment __typename (or use Gateway as the server), and you get

{
  "data": {
    "dog": {
      "id": "123",
      "name": "Rover"
    }
  }
}

Also, changing the query to not nest will work:

query test {
  dog {
    ... on Animal {
      id
    }
    ... on Dog {
      name
    }
  }
}

To Reproduce
Implement the above schema in a Router service and run the given query(ies).

Expected behavior
We expect that the results from Router would be the same as from Gateway.

Output
See above.

Desktop (please complete the following information):

  • OS: Linux
  • Version: v1.10.3

Additional context

@Geal
Copy link
Contributor

Geal commented Feb 10, 2023

could you show the request and response coming in and out of the subgraph?

@carldunham
Copy link
Author

Happy to. I've tried doing the experimental logger thing, but that didn't work for me. May have to wire shark it.

carldunham added a commit to carldunham/apollo-router-2587 that referenced this issue Feb 10, 2023
@carldunham
Copy link
Author

carldunham commented Feb 10, 2023

So I edited this issue, after taking the time to actually isolate it, rather than deriving from production code.

This reproduces the issue: https://github.com/carldunham/apollo-router-2587

@carldunham
Copy link
Author

carldunham commented Feb 10, 2023

OK, so I was able to get the logging to work. Router is definitely asking for (and getting) the right data from the subgraph, but seems to be dropping it for its response.

https://github.com/carldunham/apollo-router-2587/blob/main/log.json

@carldunham
Copy link
Author

Any updates here?

@BrynCooke
Copy link
Contributor

Marking as high priority as it is a correctness issue.

@abernix abernix removed the triage label Aug 9, 2023
@abernix
Copy link
Member

abernix commented Aug 9, 2023

Let's look at the reproduction provided above and see if this still exists or if it's been resolved with some of our query planner updates in recent versions

@carldunham: if you already know that it's resolved or not resolved, please let us know! 😄

@abernix abernix changed the title Issue with fragments on interfaces? Use of __typename affects presence of fragment spread data on interfaces Aug 9, 2023
@o0Ignition0o o0Ignition0o self-assigned this Aug 31, 2023
@o0Ignition0o
Copy link
Contributor

o0Ignition0o commented Aug 31, 2023

Here's a couple of fresh screenshots, using router 1.28.1:

image

image

image

The bug is still present, I'm going to look into it!

I'd also like to spend a minute to show gratitude for such a well written issue, the STRs, and even the repository. Super appreciated!

I'm going to write a failing test and check what happens

For info here's the subgraph request being made, and the subgraph response received:

# request
query dog__animal__0{
  dog {
    id
    name
  }
}
{
  "data":{
    "dog":{
      "id":"4321",
      "name":"Spot"
    }
  }
}

o0Ignition0o added a commit that referenced this issue Sep 1, 2023
Fix #2587

Operations would over rely on the presence of __typename to resolve selection sets on interface implementers.
This changeset checks for the parent type in an InlineFragment, so we don't drop relevant selection set when applicable.
o0Ignition0o added a commit that referenced this issue Sep 6, 2023
#3718)

Fix #2587

Operations would over rely on the presence of __typename to resolve
selection sets on interface implementers. This changeset checks for the
parent type in an InlineFragment, so we don't drop relevant selection
set when applicable.
garypen pushed a commit that referenced this issue Sep 12, 2023
Fix #2587

Operations would over rely on the presence of __typename to resolve selection sets on interface implementers.
This changeset checks for the parent type in an InlineFragment, so we don't drop relevant selection set when applicable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants