Exception in thread "main" graphql.AssertException: Internal error: should never happen: Directive values of type 'EnumValue' are not supported yet

[dspenceb]

This is an exception that is thrown by graphql-java when called from graphql-java-tools. The problem, however, is due to the way graphql-java-tools is using the graphql-java library internal API.

This is evidenced in this issue in graphql-java for a PR created to attempt to fix this issue: graphql-java/graphql-java#1706

Tools does not pass a directiveDefinition -- which is never mapped at any point in the SchemaParser
https://github.com/graphql-java-kickstart/graphql-java-tools/blob/master/src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt#L163
And this causes graphql-java to use a code path that, according to them, is dead code and actually is removed in v15.

I created a test application to demonstrate this issue here: https://github.com/dspenceb/graphqltest

For quick reference, the schema:

type Query {
    books: [Book!]
}

type Book {
    id: Int!
    name: String! @allowed(
        allowed: [ALLOWED]
    )
    author: Author!
}
...
directive @allowed(
    allowed: [AllowedState!]
) on FIELD_DEFINITION | OBJECT

enum AllowedState {
    ALLOWED
    DISALLOWED
}

wiring:

GraphQLSchema schema = SchemaParser.newParser()
                .file("schema.graphqls")

                .resolvers(new QueryResolver(new BookRepository()), new BookResolver(new AuthorRepository()))
                .directive("allowed", new AllowedDirective())
                .build()
                .makeExecutableSchema();

Stack:

Exception in thread "main" graphql.AssertException: Internal error: should never happen: Directive values of type 'EnumValue' are not supported yet
	at graphql.Assert.assertShouldNeverHappen(Assert.java:51)
	at graphql.schema.idl.SchemaGeneratorHelper.buildDirectiveInputType(SchemaGeneratorHelper.java:203)
	at graphql.schema.idl.SchemaGeneratorHelper.buildDirectiveInputType(SchemaGeneratorHelper.java:201)
	at graphql.schema.idl.SchemaGeneratorHelper.buildDirectiveArgument(SchemaGeneratorHelper.java:272)
	at graphql.schema.idl.SchemaGeneratorHelper.lambda$buildDirective$6(SchemaGeneratorHelper.java:251)
	at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
	at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1382)
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:482)
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:472)
	at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
	at graphql.schema.idl.SchemaGeneratorHelper.buildDirective(SchemaGeneratorHelper.java:252)
	at graphql.kickstart.tools.SchemaParser.buildDirectives(SchemaParser.kt:150)
	at graphql.kickstart.tools.SchemaParser.createField(SchemaParser.kt:278)
	at graphql.kickstart.tools.SchemaParser.access$createField(SchemaParser.kt:18)
	at graphql.kickstart.tools.SchemaParser$createObject$$inlined$forEach$lambda$1.apply(SchemaParser.kt:124)
	at graphql.kickstart.tools.SchemaParser$createObject$$inlined$forEach$lambda$1.apply(SchemaParser.kt:18)
	at graphql.schema.GraphQLObjectType$Builder.field(GraphQLObjectType.java:308)
	at graphql.kickstart.tools.SchemaParser.createObject(SchemaParser.kt:123)
	at graphql.kickstart.tools.SchemaParser.parseSchemaObjects(SchemaParser.kt:68)
	at graphql.kickstart.tools.SchemaParser.makeExecutableSchema(SchemaParser.kt:98)
	at dspenceb.graphqltest.Main.main(Main.java:17)

[vojtapol]

Thanks for reporting this. This will be likely fixed in the next version when we support GraphQL Java 15.

[dspenceb]

hi @vojtapol thanks for the response! Is there any kind of timeline or ETA on when that may be?

Thanks!

[jamesmartinpp]

@vojtapol do you know which version this would be fixed in? we're trying to plan around the fix.

[vojtapol]

@jamesmartinpp We may need to do a significant rewrite to fix all the issues around directives. In general, we need to defer more work to graphql-java so that it's easier to stay up to date with their releases. I have no ETA on when the work is going to be finished.

[dspenceb]

I saw in the latest 6.2.0 release that there is support for graphql java 15.0. Does that mean this issue is fixed? I haven't had time to investigate this on my own.

[williamwjs]

FYI: I tried with the latest 11.0.0 and still having this issue now

[anthochristen]

@vojtapol Any plans to supports this in the near future?

[wwang107]

Anybody has a workaround for this?

[sbarbs95]

Any news about this issue? Thanks

[jamesmartinpp]

Is this still an issue? We have updated to graphql-java v20 and graphql-java-tools v13 but are still seeing the exception. We have been doing a custom patch of graphql-java but we would prefer not to have to do that anymore, assuming that custom patch even works anymore.

[jamesmartinpp]

@oryan-block is this issue fixed by #763? If it is fixed is it only fixed in the latest version of 13.1.1?

[oryan-block]

@jamesmartinpp if the problem was with a directive containing an enum parameter it should be fix by #764

I'm not maintaining older versions so yes, only in 13.1.1.

[jamesmartinpp]

Got it, Thanks! @oryan-block

[crankydillo]

@oryan-block Is there no way to produce a 13.0 patch release with this change? Do you think that's not reasonably doable? How about if someone submits a PR?

[oryan-block]

@crankydillo I don't see what the purpose of a patch would be. The big difference between 13.0 and 13.1 was exactly fixing these issues with directive processing.

[jamesmartinpp]

@oryan-block the problem is the requirement to upgrade to graphql-java v21 and consequently java 11. We wanted this fix on a version that would work with java 8.

[oryan-block]

@crankydillo Okay I see what you mean. If you want to open a PR for this I can try releasing a "hotfix"

[crankydillo]

Hi @oryan-block It doesn't look like this requires much. I just downgraded pom versions and everything built (code). Unfortunately, I'm not able to test in some of our main regressions, but I assume this code will work for v20, like it will for v21.

I will submit a PR to a branch if you create a branch, but feel free to just toggle those versions:)

[crankydillo]

Hi @oryan-block . Wanted to touch base with you on this. I created this commit off your master. It built and passed all our graphql-java related tests.

I don't mind submitting a PR. I'm going to redo this work on top of the v13.0.4 branch and submit a PR to that. Let me know if that's not what you're thinking.

[crankydillo]

Ah, it looks like you might already be doing this!

[oryan-block]

@crankydillo Trying to. The "pipeline" is not built for this.

[crankydillo]

Hi @oryan-block . Sorry to pester and thanks for doing this. We just want to make sure you plan to do all the tagging and stuff like that you typically do for releases for the 13.0.4 Maven artifact I'm assuming you deployed:) That's going to happen, right?

Basically, we want to make sure that it's OK to use that 13.0.4 Maven artifact, and, if we do and have to fix something, we can submit a patch PR to your repo.

Thanks again!

[oryan-block]

@crankydillo right so there's a bug in the deploy script that I don't have the time to look into. However, since the artifact was pushed I guess I could create a release for it and it should technically be fine. Let me know if that's what you want.

[crankydillo]

Yes. That's what we're hoping for, along with a tag for the v13.0.4. I think you currently have that as a branch.

[oryan-block]

@crankydillo Alright I hope this works for you:
https://github.com/graphql-java-kickstart/graphql-java-tools/releases/tag/v13.0.4

[jamesmartinpp]

Thanks, @oryan-block

[oryan-block]

Closing for now