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

1.14.0 using introspection creates duplicate defer directive #3417

Closed
loganblevins opened this issue Jul 23, 2024 · 22 comments · Fixed by apollographql/apollo-ios-dev#440
Closed
Assignees
Labels
bug Generally incorrect behavior

Comments

@loganblevins
Copy link

loganblevins commented Jul 23, 2024

Summary

See #2395 (comment)

Version

1.14.0

Steps to reproduce the behavior

I cannot share our private schema, but our schema line 1 contains directive defer.

Then I run introspection on 1.14.0 and it makes yet another defer directive and the json schema validation fails.

Logs

No response

Anything else?

No response

@calvincestari
Copy link
Member

@loganblevins, I'm not able to replicate the error you describe when the schema only has a single defer directive definition. Here is a sample project I put together that contains three schemas:

  • the first (single-directive-schema.graphqls) has a single defer directive definition and codegen completes as expected without error.
  • the second (multiple-directive-schema.graphqls) has multiple defer directive definitions and fails codegen as expected with the error you describe. The second definition in that file is the one you shared in your comment on #2395.
  • the third (no-directive-schema.graphqls) has no defer directive definition and codegen completes as expected without error.

We also have a test on our GraphQLCompiler module to detect this scenario and fail CI if our logic for this behaviour is changed. All tests passed on the release PR for 1.14.0.

If you're able to change the sample project to fail in the same way as you're seeing it would help in debugging this further. Otherwise if your schema is large it may be worth doing another search for directive @defer to ensure it's not buried somewhere deeper in the schema.

Then I run code gen on 1.14.0 and it makes yet another defer directive and the json schema validation fails.

What leads to you to believe that codegen is adding another directive definition? I don't think we output the schema, or store it locally for inspection, at any stage of code generation.

@calvincestari
Copy link
Member

@calvincestari Thanks for quick reply! Is there a way I can disable this experimental feature in meantime? I couldn't find that key for my config json

No, there is no way to disable defer support. If there are no operations that use the defer directive then codegen outputs models as previous versions.

@loganblevins
Copy link
Author

loganblevins commented Jul 23, 2024

@calvincestari Here is my json config for context:

The endpoint is not public so you won't be able to introspect.

{
  "schemaNamespace" : "Federation",
  "input" : {
    "operationSearchPaths" : [
      "./Schema/**/*.graphql"
    ],
    "schemaSearchPaths" : [
      "./Schema/**/*.graphqls"
    ]
  },
  "output" : {
    "testMocks" : {
      "none" : {
      }
    },
    "schemaTypes" : {
    "path" : "./Sources/FederationAPI/Generated/Federation",
      "moduleType" : {
        "embeddedInTarget" : {
          "name" : "FederationAPI"
        }
      }
    },
    "operations" : {
      "inSchemaModule" : {
      }
    }
  },
  "schemaDownloadConfiguration": {
      "downloadMethod": {
          "introspection": {
              "endpointURL": "https://api.angelstudios.io/graphql",
              "httpMethod": {
                  "POST": {}
              },
              "includeDeprecatedInputValues": false,
              "outputFormat": "SDL"
          }
      },
      "downloadTimeout": 60,
      "headers": [],
      "outputPath": "./Schema/schema.graphqls"
  }
}

@loganblevins
Copy link
Author

@calvincestari Here is my script I run to introspect and run code gen.

This command on 1.14.0 is creating the duplicate defer directive in my schema file.

if ! [ -x "$(command -v ./apollo-ios-cli)" ]; then
  echo 'apollo-ios-cli is not installed..installing now.'
  swift package --allow-writing-to-package-directory apollo-cli-install
fi

./apollo-ios-cli generate --verbose --path .././apollo-codegen-config.json --fetch-schema

@loganblevins
Copy link
Author

If you're able to change the sample project to fail in the same way as you're seeing it would help in debugging this further. Otherwise if your schema is large it may be worth doing another search for directive @defer to ensure it's not buried somewhere deeper in the schema.

I mentioned in the other GH thread that our schema file already contains a defer directive prior to updating to 1.14.0 and the above command I run is adding another one.

@loganblevins
Copy link
Author

Sorry if I said code gen... I run the plugin CLI tool with introspect and code gen at once so it may just be the introspect step that is the issue

@calvincestari
Copy link
Member

OK, so is it the schema file that's output from the --fetch-schema stage that contains two defer directive definitions?

That schema file is simply the result of introspection and output. There is no defer-related manipulation done to the schema before output.

@loganblevins
Copy link
Author

@calvincestari that is correct but only on 1.14.0

If I revert back to my prior tag 1.12.0 and introspect the diff is unchanged.
If I introspect on 1.14.0 my diff shows
Screenshot 2024-07-23 at 16 56 49

@loganblevins
Copy link
Author

Screenshot 2024-07-23 at 16 57 47

This directive is already in my schema file prior to introspecting on 1.14.0....but 1.14.0 adds on a duplicate

@calvincestari
Copy link
Member

This is certainly weird. I'm confused how fetch-schema would be adding the directive.

Are you able to validate this with a separate tool like Apollo Studio or GraphiQL that will let you view the schema off your server too? Either of those should let you view, and download, the schema.

@loganblevins
Copy link
Author

loganblevins commented Jul 23, 2024

@calvincestari I don't know how to use the sample project you sent to replicate if it's the --fetch-schema that is the problem.

It definitely is a blocker bug because I am unable to update the package on my end and am blocked. I've been using this Apollo tool for a long time and never had this issue.

I just introspected now on 1.12.0 and have no issues.

@loganblevins
Copy link
Author

loganblevins commented Jul 23, 2024

@calvincestari While it is possible I could use Apollo Studio that is a very manual process and leaves the CLI in a broken state and unusable

@calvincestari
Copy link
Member

@calvincestari I don't know how to use the sample project you sent to replicate if it's the --fetch-schema that is the problem.

No, if --fetch-schema is the issue that sample project won't help. It demonstrates that the code generation build phase does not add a duplicate directive - but to an already existing schema file with only a single defer directive definition.

While it is possible I could use Apollo Studio that is a very manual process and leaves the CLI in a broken state and unusable

The CLI will get fixed if there is an issue there. I'm still trying to figure out the root cause.

@calvincestari
Copy link
Member

OK, I've gotten to a state where I can replicate the diff via introspection. Give me some time to try figure out how this is happening.

@loganblevins
Copy link
Author

@calvincestari Awesome thanks

@calvincestari
Copy link
Member

calvincestari commented Jul 23, 2024

I think the problem is in graphql-js which we use for validating a schema, compiling a schema, and converting introspection JSON into GraphQL SDL. The downloaded introspection JSON only contains a single defer directive definition but after the call to printSchemaToSDL the output then contains two instances of the defer directive definition.

@loganblevins loganblevins changed the title 1.14.0 using code gen creates duplicate defer directive 1.14.0 using introspection creates duplicate defer directive Jul 24, 2024
@calvincestari
Copy link
Member

calvincestari commented Jul 25, 2024

I've got a draft PR up at apollographql/apollo-ios-dev#440. with one outstanding JS test to get working then I think we'll be good to go.

@loganblevins
Copy link
Author

@calvincestari Cool thanks!

Copy link
Contributor

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo iOS usage and allow us to serve you better.

@calvincestari
Copy link
Member

The PR to resolve this has been merged and the fix will go out in the next release, we're aiming to publish that this week still.

@loganblevins
Copy link
Author

@calvincestari super grateful - thank you!

@loganblevins
Copy link
Author

Working!! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Generally incorrect behavior
Projects
None yet
2 participants