Graphql-js: Allow subscribe function to return a Promise

[NeoPhi]

The current subscribe resolve logic does not permit returning a Promise that would resolve to an AsyncIterable. The use case is running asynchronous business logic to determine if the subscription should be allowed or additional information needed to return a valid AsyncIterable. While it would be possible to merge that logic into a high order iterable, I feel the ability to return a Promise would better match the existing GraphQL semantics.

[Urigo]

So if I understand correctly, the use case for adding additional API is in order to reject the subscription at the first place.
So there will never be a use case for the Promise to be resolved..
To achieve that today with a single API, you would just need to throw the AsyncIterator.
In this comment there are a few links for implementations of AsyncIterator, one of them around Promises.

Personally I feel that adding another API would just create more mess but I see that there are a lot of likes to your suggestion so I would love to be persuaded differently.

[NeoPhi]

We have 2 use cases. The first is the error use case. However, our more common use case is having to lookup information based on the subscription parameters to determine what channels to subscribe to.

I don't see this as introducing a new API. This is just allowing the AsyncIterable to be returned at a later time via a Promise instead of immediately.

[robzhu]

@NeoPhi thanks for filing this issue. Can you help me understand your use-case in more depth? What's wrong with handling an AsyncIterator that throws on the first invocation of next()? We've been talking through a few options, and I think we have at least one proposal on the table that addresses your point:

subscribe() will return a Promise. GraphQLResponse will capture 400-class errors that contain messages that might be useful to the client. Exception will capture 500-class errors that the client cannot fix (for example, underlying event-bus errors). AsyncIterable would only be returned if the source stream was successfully constructed and the server has a confidence to expect future publishes on the source stream.

It's certainly more verbose of an API, but it explicitly captures all the success/error cases. Thoughts?

[NeoPhi]

The primary use case is that we have a subscription in which the client passes in a couple of arguments. We have asynchronous business logic that runs which translates those arguments into a set of channel names that an AsyncIterable then listens on, hence our desire to be able to have the subscribe() function return a Promise. Your proposal of Promise<AsyncIterable | GraphQLResponse | Exception> would address this.

[jperl]

I just ran into this as well. The use case is to validate the user has permission to subscribe based on the arguments provided -- before starting the subscription.

[robzhu]

@jperl, @NeoPhi, I submitted a PR to address this issue, please take a look and let me know if this it fixes the problem for you: https://github.com/graphql/graphql-js/pull/918

[jperl]

It does. Thank you!