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

Schemas with customized root operation type lead to execution errors due to incorrectly generated selection sets #2955

Closed
ynnadrules opened this issue Apr 14, 2023 · 7 comments · Fixed by #2983
Assignees
Labels
bug Generally incorrect behavior needs investigation planned-next Slated to be included in the next release

Comments

@ynnadrules
Copy link

ynnadrules commented Apr 14, 2023

Summary

(Apologies for the terrible title. I'm struggling with the terminology and producing a concise title.)

Having custom root operation type names in the schema (see below) introduces a parsing error because the operation level selection set gets generated with a non-nil __typename field which won't actually exist in the operation's result JSON and leads to an execution error stating that a JSON value is missing for a key.

Version

1.1.2

Steps to reproduce the behavior

Here's an example schema I extracted from a test I wrote to confirm the issue:

schema {
  query: RootQueryType
  mutation: RootMutationType
}

type RootQueryType {
  allAnimals: [Animal!]!
}

type RootMutationType {
  feedAnimal: Animal!
}

type Animal {
  fieldName: String!
}

Here's the document:

mutation FeedAnimal {
  feedAnimal {
    fieldName
  }
}

Here's the generated operation definition:

public struct Mutation: TestSchema.SelectionSet {
  public let __data: DataDict
  public init(_dataDict: DataDict) { __data = _dataDict }

  public static var __parentType: ApolloAPI.ParentType { TestSchema.Objects.RootMutationType }
  public static var __selections: [ApolloAPI.Selection] { [
    .field("__typename", String.self), // <== THIS SHOULDN'T BE HERE 
    .field("feedAnimal", FeedAnimal.self),
  ] }

  public var feedAnimal: FeedAnimal { __data["feedAnimal"] }

  /// FeedAnimal
  public struct FeedAnimal: TestSchema.SelectionSet {
    public let __data: DataDict
    public init(_dataDict: DataDict) { __data = _dataDict }

    public static var __parentType: ApolloAPI.ParentType { TestSchema.Objects.Animal }
    public static var __selections: [ApolloAPI.Selection] { [
      .field("__typename", String.self),
      .field("fieldName", String.self),
    ] }

    public var fieldName: String { __data["fieldName"] }
  }
}

Here's the an example of the underlying parsing error message from my actual app:

ApiError(
  errorDump: """
    ▿ ApolloCombine.GQLError.nonGQL
      ▿ nonGQL: Optional(Apollo.GraphQLExecutionError(path: __typename, underlying: ApolloAPI.JSONDecodingError.missingValue))
        ▿ some: Apollo.GraphQLExecutionError
          ▿ path: __typename
            ▿ head: Optional(Apollo.ResponsePath.(unknown context at $10116c1f4).Node)
              ▿ some: Apollo.ResponsePath.(unknown context at $10116c1f4).Node #0
                - previous: nil
                - key: "__typename"
                ▿ $__lazy_storage_$_joined: Optional("__typename")
                  - some: "__typename"
                - $__lazy_storage_$_components: nil
          - underlying: ApolloAPI.JSONDecodingError.missingValue
    
    """,
  file: "Networking/ApiClient.swift",
  line: 62,
  message: "The operation couldn’t be completed. (ApolloCombine.GQLError error 1.)"
)

The SelectionSetTemplate generates the __typename field based on the return value of:

private func shouldIncludeTypenameSelection(for scope: IR.ScopeDescriptor) -> Bool {
  return scope.scopePath.count == 1 && !scope.type.isRootFieldType
}

And here's how verifying a root field type is done in GraphQLCompositeType:

public class GraphQLCompositeType: GraphQLNamedType {
  public override var debugDescription: String {
    "Type - \(name)"
  }

  var isRootFieldType: Bool {
    ["Query", "Mutation", "Subscription"].contains(name)
  }
}

Given the example schema above, an operation of type RootMutationType is not in the list ["Query", "Mutation", "Subscription"], which leads to the incorrectly generated __typename field in the operation's selection set.

While I was trying to trace where the issue was stemming from, I cloned the repo and added a failing test case to Tests/ApolloCodegenTests/CodeGeneration/Templates/SelectionSet/SelectionSetTemplateTests.swift

func test__render_selections__givenCustomOperationRootSelectionSet_doesNotRenderTypenameSelection() throws {
  // given
  schemaSDL = """
  schema {
    query: RootQueryType
    mutation: RootMutationType
  }
  
  type RootQueryType {
    allAnimals: [Animal!]!
  }
  
  type RootMutationType {
    feedAnimal: Animal!
  }

  type Animal {
    fieldName: String!
  }
  """

  document = """
  mutation FeedAnimal {
    feedAnimal {
      fieldName
    }
  }
  """

  let expected = """
    public static var __selections: [ApolloAPI.Selection] { [
      .field("feedAnimal", FeedAnimal.self),
    ] }
  """

  // when
  try buildSubjectAndOperation(named: "FeedAnimal")
  let mutationRoot = try XCTUnwrap(
    operation[field: "mutation"] as? IR.EntityField
  )

  let actual = subject.render(field: mutationRoot)

  // then
  expect(actual).to(equalLineByLine(expected, atLine: 7, ignoringExtraLines: true))
}

Logs

ApiError(
  errorDump: """
    ▿ ApolloCombine.GQLError.nonGQL
      ▿ nonGQL: Optional(Apollo.GraphQLExecutionError(path: __typename, underlying: ApolloAPI.JSONDecodingError.missingValue))
        ▿ some: Apollo.GraphQLExecutionError
          ▿ path: __typename
            ▿ head: Optional(Apollo.ResponsePath.(unknown context at $10116c1f4).Node)
              ▿ some: Apollo.ResponsePath.(unknown context at $10116c1f4).Node #0
                - previous: nil
                - key: "__typename"
$__lazy_storage_$_joined: Optional("__typename")
                  - some: "__typename"
                - $__lazy_storage_$_components: nil
          - underlying: ApolloAPI.JSONDecodingError.missingValue
    
    """,
  file: "Networking/ApiClient.swift",
  line: 62,
  message: "The operation couldn’t be completed. (ApolloCombine.GQLError error 1.)"
)

Anything else?

No response

@ynnadrules ynnadrules added bug Generally incorrect behavior needs investigation labels Apr 14, 2023
@AnthonyMDev AnthonyMDev self-assigned this Apr 14, 2023
@AnthonyMDev AnthonyMDev added this to the Patch Releases (1.1.x) milestone Apr 14, 2023
@AnthonyMDev AnthonyMDev added the planned-next Slated to be included in the next release label Apr 14, 2023
@AnthonyMDev
Copy link
Contributor

Hahaha, I'm shocked it's finally happened!!! Using a typename other than Query, Mutation, or Subscription has been a long known limitation of the client. @calvincestari and I were talking about this last week and I was thinking about it again yesterday. I've thought it was crazy that no one has ever made an issue about it.

We've had so many other things to address and have just continued to put this off because literally no one has ever actually done this and encountered the error.

So thank you for finally getting us to have to fix this one. I'll get to it for the next release. :)

@ynnadrules
Copy link
Author

Haha I didn't realize it had a been long standing limitation. I've been using the same schema for years and it wasn't until the generated type name fields were added to the selection sets did that non standard naming come to bite haha.

I'm using the elixir graphql library Absinthe on my backend and it, by default, names the query, mutation, and subscription types like I showed in the example. It's pretty easy to change that on the backend side of things, but I agree with you: I can't believe no one else has brought this up haha.

Thanks in advance for addressing it in the next release!

@kietcthe
Copy link

I also have this issue when using this library today.
I fixed it by commenting the line below.
image

Hope it will be fixed soon.

@tahirmt
Copy link
Contributor

tahirmt commented Apr 15, 2023

I started seeing this when I tried updating to 1.1 last week and I thought I was crazy and I did something wrong that caused missing value errors on __typename on queries that were previously working up until 1.0.7. We still have 0.5x on production but I have a 1.0 branch going until 1.2 is out. I'm happy to see I'm not the only one

@yonaskolb
Copy link
Contributor

yonaskolb commented Apr 17, 2023

This is holding up our upgrade to 1.x as well. One of the generated schemas we use:

{
  "data": {
    "__schema": {
      "queryType": {
        "name": "query"
      },
      "mutationType": {
        "name": "mutation"
      },
      "types": [
        {
          "kind": "OBJECT",
          "name": "mutation",
          "fields": [
               ...
           ]
        },
        ...
      ]
    }
  }
}

@AnthonyMDev
Copy link
Contributor

Thanks for all the feedback everyone. This is at the top of my list!

@BobaFetters
Copy link
Member

The fix for this has been merged into main and will go out with the next release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Generally incorrect behavior needs investigation planned-next Slated to be included in the next release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants