feat: reconnect topic subscriptions on error #478
Conversation
Instead of returning an error message, attempt to reconnect to a topic on error by remaking the subscription.
| return _logger.LogTraceTopicRequestSuccess(RequestTypeTopicSubscribe, cacheName, topicName, | ||
| response); | ||
| } | ||
|
|
||
| private async ValueTask<TopicMessage?> GetNextRelevantMessageFromGrpcStreamAsync(AsyncServerStreamingCall<_SubscriptionItem> subscription, | ||
| CancellationToken cancellationToken, string cacheName, string topicName) | ||
| private class SubscriptionWrapper : IDisposable |
There was a problem hiding this comment.
I didn't do this since I wanted to keep the pr surface as small as possible, but what are people's thoughts on making this the IAsyncEnumerator that the subscription response returns?
https://github.com/momentohq/client-sdk-dotnet/blob/main/src/Momento.Sdk/Responses/TopicSubscribeResponse.cs#L67
That would mean the topic is only actually subscribed to while the enumerator is in use, but it feels like a more simple layer to encapsulate this grpc logic.
There was a problem hiding this comment.
If that feels more natural for DotNet users then I think be cool to start with that. I'm not the most experienced in dotnet though be good get @kvcache and also maybe ask a few of our customers in slack who use dotnet what they think. Can DM you with few good targets. Also ok w/ me starting here and fast following w/ wrapper or change to add this.
There was a problem hiding this comment.
There will be no user facing change, but it would make the subscription response object cleaner.
Instead of returning an error message, attempt to reconnect to a topic on error by remaking the subscription.