Simplify HotChocolate Subscription api implementation supporting IAsyncEnumerable<T>

[tretyakov-d]

Product

Hot Chocolate

Is your feature request related to a problem?

At this moment it's supported in quite an awkard way which overcomplicates unti testing a lot.

At this moment, I can declare my subscription and provide name of a private function that returns IAsyncEnumerable using With property of SubscribeAttribute.

If I could initially describe my subscription method to return IAsyncEnumerab would not just make code way cleaner, it would allow to unit test it in way cleaner manner.

The solution you'd like

Instead of (or in addition to):

public class RootSubscription
{
    async IAsyncEnumerable<EventPayload> SubscribeToMessage([Service] ITopicEventReceiver receiver)
    {
        await using ISourceStream<EventPayload> stream = await receiver.SubscribeAsync<EventPayload>("TOPIC_NAME");

        await foreach (var eventPayload in stream.ReadEventsAsync())
        {
            // logic ommited for simplicity
            yield return eventPayload;
        }
    }
    
    [Subscribe(With = nameof(SubscribeToMessage))]
    public EventPayload OnMessage([EventMessage] EventPayload payload)
    {
        return payload
    }
}

Support following:

public class RootSubscription
{
    [Subscribe]
    public IAsyncEnumerable<EventPayload> OnMessage([Service] ITopicEventReceiver receiver)
    {
        await using ISourceStream<EventPayload> stream = await receiver.SubscribeAsync<EventPayload>("TOPIC_NAME");

        await foreach (var eventPayload in stream.ReadEventsAsync())
        {
            // logic ommited for simplicity
            yield return eventPayload;
        }
    }
}

[PascalSenn]

Hi @tretyakov-d

Thanks for creating this issue

We are in fact thinking of a new subscription API, yet it most likely will not be as simple as you posted it here.

The 'subscribe with' method defines the source stream and is scoped to the whole subscription. The subscribe resolver is scoped to a single event.

The reason why this is separated is simply to not encourage users to use e.g. dataloaders in the source stream, as this would easily lead to unexpected behaviour and memory leaks.

we are not yet sure what the new API looks like, yet we want to restrict the capabilities of the source stream (by default) even more, just so that the user at least gets informed when they do something wrong

[michaelstaib]

Its not only DataLoader but any service would not be released over the time the stream runs. DBContexts for instance would keep allocating. This often lead to memory leaks by users. We actually had this exact API with Hot Chocolate 10 but have deprecated it.

[michaelstaib]

I am closing this issue. We are at the moment internally testing API proposals and will have a revision in a future update.

[tretyakov-d]

Well, that's sad. I understand issues you mentioned, however current version with With="FnName" has all the same issues.
IMHO you cannot prevent lame people from coding :|
Maybe it coul be another attribute like [SubscribePleaseKeepInMindTheMethodRunsContiniouslyWhileSubscriptionIsActiveAndPleaseThinkTwiceWhatExactlyYouAreInjecting] :)