Skip to content

Fix Subscription GraphQLRequest deserialization #249

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

Merged

Conversation

kobylynskyi
Copy link
Contributor

@kobylynskyi kobylynskyi commented May 5, 2020

Fixes the following error while subscription request deserialization:

2020-05-04 21:46:34:935 [https-jsse-nio-8079-exec-7] ERROR g.k.servlet.GraphQLWebsocketServlet - Error executing websocket query for session: 0
java.lang.IllegalArgumentException: Cannot construct instance of `graphql.kickstart.execution.GraphQLRequest` (although at least one Creator exists): no String-argument constructor/factory method to deserialize from String value ('{"query":"subscription Logs { logs(eventId: \"87654\") { id } }"}')
 at [Source: UNKNOWN; line: -1, column: -1]
	at com.fasterxml.jackson.databind.ObjectMapper._convert(ObjectMapper.java:3938)
	at com.fasterxml.jackson.databind.ObjectMapper.convertValue(ObjectMapper.java:3869)
	at graphql.kickstart.execution.subscriptions.GraphQLSubscriptionMapper.readGraphQLRequest(GraphQLSubscriptionMapper.java:16)
	at java.lang.Thread.run(Thread.java:748) [22 skipped]

The error is seen in 9.1.0 as well as 9.2.0=SNAPSHOT.

Probably related to:
graphql-java-kickstart/graphql-spring-boot#366

Copy link

@arkadyz1 arkadyz1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can I get jar with this fix ?

@oliemansm oliemansm merged commit 31aaebf into graphql-java-kickstart:master May 16, 2020
@oliemansm
Copy link
Member

Thanks!

@oliemansm oliemansm added this to the 9.2.0 milestone May 16, 2020
@oliemansm
Copy link
Member

@kobylynskyi I had to revert this commit, because it broke my subscriptions instead of fixing them. It therefore sounds the error you're facing is not caused by this, but is caused by something else.

@arkadyz1
Copy link

This commit fixed my subscriptions. May be not to revert it ?

@arkadyz1
Copy link

@kobylynskyi I had to revert this commit, because it broke my subscriptions instead of fixing them. It therefore sounds the error you're facing is not caused by this, but is caused by something else.

This commit fixed my subscriptions. May be not to revert it ?

@oliemansm
Copy link
Member

oliemansm commented May 18, 2020

It breaks default subscriptions when using it with GraphiQL and Altair. The only reported issue similar to your issues was graphql-java-kickstart/graphql-spring-boot#366 which was because it was manually defining dependencies instead of letting the project pull them in which caused compatibility issues. If you do that and still have a problem then raise an issue and ideally create a PR to fix that, but it must not break functionality that's being used without any problem by everybody else.

See this subscription sample (plain servlet, no tools/spring-boot used there): https://github.com/graphql-java-kickstart/samples/tree/master/servlet-hello-world

@arkadyz1
Copy link

It breaks default subscriptions when using it with GraphiQL and Altair. The only reported issue similar to your issues was graphql-java-kickstart/graphql-spring-boot#366 which was because it was manually defining dependencies instead of letting the project pull them in which caused compatibility issues. If you do that and still have a problem then raise an issue and ideally create a PR to fix that, but it must not break functionality that's being used without any problem by everybody else.

See this subscription sample (plain servlet, no tools/spring-boot used there): https://github.com/graphql-java-kickstart/samples/tree/master/servlet-hello-world

Thank you very much for brief example. Are you sure it is working with version 9.1.0 ?

public class SubscriptionEndpoint extends GraphQLWebsocketServlet {

public SubscriptionEndpoint() {
super(GraphQLConfigurationProvider.getInstance().getConfiguration());
}

}

GraphQLWebsocketServlet doesn't have such a constructor in this version.

@oliemansm
Copy link
Member

That example requires 9.2.0-SNAPSHOT. There's also an example using Spring Boot that doesn't require it: https://github.com/graphql-java-kickstart/samples/tree/master/subscription-with-authentication.

It showcases subscription authentication, although that's not the part you're interested in here.

@arkadyz1
Copy link

Can you please tell which dependencies exactly solve the issue ? We are not working with gradle/maven. We are working with Wildfly 11 and put jars manually into its folders.

I upgraded jackon jars to 2.10.0 but it didn't help.

Thanks in advance.

@oliemansm
Copy link
Member

I don't have any experience with Wildfly. If you're manually managing your jars that could easily lead to compatibility problems as indicated above as the cause for this problem in the related issue. Your best bet would be to take a look at the sample projects I shared with you and compare the dependencies used there with the ones used by you right now. Otherwise you might be better off raising an issue on Stackoverflow for this particular use case.

The samples and test cases related to subscriptions are working correctly so from my point of view the functionality as provided by this library and using the mechanisms supported by this library are working as expected.

@arkadyz1
Copy link

Do you have an example of using rxjava (instead of reactor-core) for creating publishers ?

@oliemansm
Copy link
Member

No, I don't.

@kobylynskyi kobylynskyi deleted the fix-graphqlrequest-deser branch May 19, 2020 00:45
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

Successfully merging this pull request may close these issues.

3 participants