Skip to content

Subscription argumennt Error using juniper_graphql_transport_ws #1186

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

Closed
kimutaiRop opened this issue Sep 4, 2023 · 2 comments · Fixed by #1196
Closed

Subscription argumennt Error using juniper_graphql_transport_ws #1186

kimutaiRop opened this issue Sep 4, 2023 · 2 comments · Fixed by #1196
Assignees
Labels
bug Something isn't working k::integration Related to integration with third-party libraries or systems
Milestone

Comments

@kimutaiRop
Copy link

kimutaiRop commented Sep 4, 2023

I have

async fn subscriptions(
    req: HttpRequest,
    stream: web::Payload,
    schema: web::Data<Schema>,
) -> Result<HttpResponse, Error> {
    let context = Context::new()
    let schema = schema.into_inner();
    let config = ConnectionConfig::new(context);
    let config = config.with_keep_alive_interval(Duration::from_secs(15));

    subscriptions_handler(req, stream, schema, config).await
}

but subscriptions_handler(req, stream, schema, config).await the config argument gives error

expected a `FnOnce<(HashMap<std::string::String, InputValue>,)>` closure, found `ConnectionConfig<context::Context>`
the trait `FnOnce<(HashMap<std::string::String, InputValue>,)>` is not implemented for `ConnectionConfig<context::Context>`
the trait `juniper_graphql_ws::Init<S, CtxT>` is implemented for `juniper_graphql_ws::ConnectionConfig<CtxT>`
required for `ConnectionConfig<context::Context>` to implement `juniper_graphql_ws::Init<DefaultScalarValue, context::Context>`
@tyranron tyranron self-assigned this Sep 21, 2023
@tyranron tyranron added bug Something isn't working k::integration Related to integration with third-party libraries or systems labels Sep 21, 2023
@tyranron tyranron added this to the 0.16.0 milestone Sep 21, 2023
@tyranron
Copy link
Member

@kimutaiRop yes, there are interoperability problems between old juniper_graphql_ws crate and new juniper_graphql_transport_ws crate introduced in #1158. Such types as ConnectionConfig are fully duplicated, thus represent different types. Furthermore, in #1158 no solution for juniper_actix crate to integrate new juniper_graphql_transport_ws crate was landed, so what you're trying to do is just not possible.

I've polished interoperability a bit in #1191, and introduced juniper_actix crate integration. So now you could just do:

async fn subscriptions(
    req: HttpRequest,
    stream: web::Payload,
    schema: web::Data<Schema>,
) -> Result<HttpResponse, Error> {
    let context = Context::new()
    let schema = schema.into_inner();
    let config = ConnectionConfig::new(context);
    let config = config.with_keep_alive_interval(Duration::from_secs(15));

    subscriptions::ws_handler(req, stream, schema, config).await
}

and it should work just fine, as showed in the juniper_actix/example/subscription.rs.
It also implements protocol auto-selection out-of-the-box, based on the Sec-Websocket-Protocol HTTP header value.

After #1191 I do also plan some more adjustments to things, but they should be much easier to catch up with:

  1. Unite juniper_graphql_ws and juniper_graphql_transport_ws, since they share a substantial codebase anyway.
  2. Reimplement juniper_actix::subscritions via actorless actix-ws crate instead of the current actix-web-actors::ws one. The whole thing with actors cooperates very badly with juniper_graphql_ws state machine, introducing too excessive polling, which leads to panics and abnormal WebSocket closures instead of normal ones. It also gives a feeling of fighting with actors, trying to pair them with Stream/Sink, instead of easing it. So, the actorless actix-ws crate should go just fine, as we have in the juniper_warp crate.

@kimutaiRop
Copy link
Author

@tyranron amazing thanks that worked.. looking forward to the coming changes seem like will make using the all thing way easier

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working k::integration Related to integration with third-party libraries or systems
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants