graphql / graphiql

GraphiQL & the GraphQL LSP Reference Ecosystem for building browser & IDE tools.
MIT License
16.13k stars 1.73k forks source link

How to unsubscribe from graphql-ws subscription #1827

Open JeanLucPons opened 3 years ago

JeanLucPons commented 3 years ago

Hello,

When I do a subscription (using graphql-ws fetcher), it works, the results are well printed in the result panel but when I push the stop button, the results stop to be printed on the panel but it seems that nothing is done on the wsClient. GraphiQL does not tell to the wsClient to unsubscribe and events are still sent by the ws server. If I repush the play button, a second subscription using a different UUID is sent to the server on the same websocket connection which if I'm not mistaken is illegal. Is there a way to unsubscribe when the stop button is pressed ? Thx

const URL = "http://debian9acu:8000/graphql";
const URLws = "ws://debian9acu:8000/graphql-ws";
var fetcher = createGraphiQLFetcher( {url:URL,subscriptionUrl:URLws} );
const container = document.getElementById("graphiql");
const defaultQuery = `
{
}
`
render(
  <GraphiQL fetcher={fetcher} defaultQuery={defaultQuery} />,
  container
);
acao commented 3 years ago

@enisdenjo can you answer this question? What might we be missing?

JeanLucPons commented 3 years ago

Hello, It would be nice also that GraphiQL displays the error message(s) in case of subscription failure. To do that I had to modify the fetcher, otherwise nothing is printed in the GraphiQL panel.

export const createWebsocketsFetcherFromClient = (wsClient: Client) => (
  graphQLParams: FetcherParams,
) =>
  makeAsyncIterableIteratorFromSink<FetcherResult>(sink =>
    wsClient!.subscribe(graphQLParams, {
      next: sink.next,
      error: (err) => {
        // Print the message in the GraphiQL panel
        if( err instanceof Error) {
          sink.next((err as Error).message);
        } else if (err instanceof CloseEvent) {
          let evt = err as CloseEvent;
          sink.next(evt.code + " " + evt.reason);
        } else { // GraphQLError[]
          sink.next((err as GraphQLError[])[0].message);
        }
        // Call error function
        sink.error(err);
      },
      complete: sink.complete
    }),
  );
enisdenjo commented 3 years ago

Well, calling return of the makeAsyncIterableIteratorFromSink iterator does nothing. It should, however, dispose of the subscription. @n1ru4l could there be a bug in @n1ru4l/push-pull-async-iterable-iterator?

Replacing makeAsyncIterableIteratorFromSink with the client usage with AsyncIterator recipe fixes the issue.

n1ru4l commented 3 years ago

Hey, this is indeed right, @n1ru4l/push-pull-async-iterable-iterator did not correctly dispose of the sink. I created https://github.com/n1ru4l/push-pull-async-iterable-iterator/pull/54 which should address this. I also published v2.1.3 which has the fix included https://github.com/n1ru4l/push-pull-async-iterable-iterator/releases/tag/v2.1.3

In the meantime, the discussion here will clear whether push-pull-async-iterable-iterator should stay in GraphiQL or not https://github.com/graphql/graphiql/pull/1841#pullrequestreview-639229360

JeanLucPons commented 3 years ago

Hello,

Thanks I tried the PR and it works as expected :) Thanks for the fix.

@acao I tried graphiql 1.4.1 and I fall into ESM modules issue, it was working before !?

./node_modules/graphql-language-service-interface/esm/getAutocompleteSuggestions.js 72:43
File was processed with these loaders:
 * ./node_modules/babel-loader/lib/index.js
You may need an additional loader to handle the result of these loaders.
|         label: argDef.name,
|         detail: String(argDef.type),
>         documentation: argDef.description ?? undefined,
|         kind: CompletionItemKind.Variable,
|         type: argDef.type

I build by myself graphiql modules (npm run build-bundles) and it worked:

If i look at ESM generation I can see that in my case it has generated esm script without using the "advanced questionmark notation".

Ex: getAutocompleteSuggestions

GraphiQL 1.4.1:

                label: argDef.name,
                detail: String(argDef.type),
                documentation: argDef.description ?? undefined,
                kind: CompletionItemKind.Variable,
                type: argDef.type,

My build:

                   label: argDef.name,
                    detail: String(argDef.type),
                    documentation: (_a = argDef.description) !== null && _a !== void 0 ? _a : undefined,
                    kind: CompletionItemKind.Variable,
                    type: argDef.type,

Thanks for help ;)

Edit: It seems linked to this issue https://github.com/graphql/graphiql/pull/1816#issuecomment-818349142 I succesfully build graphiql from the PR (@enisdenjo) ???

[vm-web-lab.pons] > git branch
  main
* sub-async-iterator
dushaniw commented 2 years ago

@JeanLucPons @acao

So the initial issue of not displaying error messages, is this fixed now?

JeanLucPons commented 2 years ago

Yes it works.

dushaniw commented 2 years ago

Appreciate if someone can answer my question in follow.

I am using the legacy SubscriptionClient in my GraphiQL UI and I have recently upgraded to the latest graphiql npm package. Just get the issue mentioned here fixed. But after upgrade, once I click the Run button I see a blank screen in response box, but previous version of graphiql alteast showed me a ack message stating subscription data will appear here after server publication in the graphiql response ui. This message no longer appears and its kind of not user friendly because user doesn't know if the subscription request was a success or not. Was it intentional to remove showing the message in UI ? If so why?

JeanLucPons commented 2 years ago

If you are moving from the old client to the new one, you may need to modify how the client is instantiated. You can have a look at my client: https://gitlab.com/tango-controls/TangoGraphQL/-/blob/master/graphiql/src/index.tsx Hope this helps

acao commented 2 years ago

@JeanLucPons is there something we need to document here?

dushaniw commented 2 years ago

@JeanLucPons thanks for the prompt response. Does your example show any observable log/message in graphiql ui once you click on run button? My question is actually why dont the GraphiQL UI doesn't show any message stating that subscription is in-progress as below. image

At least some older version showed me a message as above, in later versions there is no message its just a blank screen. Once a subscribed data is received then it is shown there. Any idea why this observable logs/messages were removed in latter versions? Is there a way that I could customize the fetcher to show a customized message? (I am using a reactjs app)

JeanLucPons commented 2 years ago

Unfortunately I don't have time to simulate a subscription error that GaphiQL should display but yes it was working when I tested it. With the new client, it the subscription is ok and if you don't get any ws frame, If I remember well, the interface will stay blank and this is what I expect. Once it receives a ws frame, it decodes it and display it. You can find an exemple of what I can see here: https://gitlab.com/tango-controls/TangoGraphQL/-/tree/master/#subscribing-to-tango-event

Concerning the documentation, a simple client using createGraphiQLFetcher() (as mine) could be useful.