Skip to content
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

Breaking change "SubscribeToAllAsync is no longer supported. Use SubscribeToAll instead" #290

Closed
stefer opened this issue Mar 4, 2024 · 5 comments · Fixed by #291
Closed
Assignees
Labels

Comments

@stefer
Copy link

stefer commented Mar 4, 2024

Describe the bug

Breaking change introduced in Grpc.Streams 23.2.0: "SubscribeToAllAsync is no longer supported. Use SubscribeToAll instead"

This require a major rewrite of our code as we hook into Azure WebJob triggers to handle the events.

It's a bit harsh to introduce a breaking change like this without a grace period as it would be with a Obsolete message as warning.
Why is the obsolete message set to Error=true?

	        [Obsolete("SubscribeToAllAsync is no longer supported. Use SubscribeToAll instead.", true)]
		public Task<StreamSubscription> SubscribeToAllAsync(

While I am here, I wonder what exceptions we will get if the subscription is dropped when using the new SubscribeToAll method.
The example code on https://developers.eventstore.com/clients/grpc/subscriptions.html#handling-subscription-drops handles the drop like this:

} catch (Exception ex) {
    Console.WriteLine($"Subscription was dropped: {ex}");
    goto Subscribe;
}

But an untyped exception could come from anywhere, so how do we know that it is an actual drop of the subscription?

To Reproduce
Steps to reproduce the behavior:

  1. Compile code that worked using Grpc.Streams 23.1.0.

Expected behavior
It should still be possible to compile the code successfully, with a deprecation warning that SubscribeToAllAsync is obsolete.

Actual behavior
Compile error (not warning) "'SubscribeToAllAsync is no longer supported. Use SubscribeToAll instead.'"

Severity	Code	Description	Project	File	Line	Suppression State
Error	CS0619	'EventStoreClient.SubscribeToAllAsync(FromAll, Func<StreamSubscription, ResolvedEvent, CancellationToken, Task>, bool, Action<StreamSubscription, SubscriptionDroppedReason, Exception?>?, SubscriptionFilterOptions?, UserCredentials?, CancellationToken)' is obsolete: 'SubscribeToAllAsync is no longer supported. Use SubscribeToAll instead.'	Collector.CreditEvaluation.Infrastructure	C:\Development\CollectorCreditEvaluation\src\Collector.CreditEvaluation.Infrastructure\Messaging\EventStore\GrpcEventStoreSubscriber.cs	58	Active

Config/Logs/Screenshots

EventStore details

  • EventStore server version: 22.10

  • Operating system:
    Windows

  • EventStore client version (if applicable):
    23.2.0

Additional context

We are using dependabot to update packages and it created a PR for bumping EventStore.Client.Grpc.Streams from 23.1.0 to 23.2.0.

@stefer
Copy link
Author

stefer commented Mar 4, 2024

As a temporary work-around, we have marked our own functions calling SubscribeToAllAsync as [Obsolete("", false)]
It seems as obsoleted method are allowed to call other obsoleted methods.

@w1am
Copy link
Contributor

w1am commented Mar 4, 2024

@stefer Appreciate you bringing this to our attention. It was indeed an oversight. We'll make sure to correct it in the upcoming patch release shortly

@w1am
Copy link
Contributor

w1am commented Mar 4, 2024

I wonder what exceptions we will get if the subscription is dropped when using the new SubscribeToAll method

The subscription could have been dropped for all kinds of reasons, so it's up to the user to handle it.

@stefer
Copy link
Author

stefer commented Mar 4, 2024

I wonder what exceptions we will get if the subscription is dropped when using the new SubscribeToAll method

The subscription could have been dropped for all kinds of reasons, so it's up to the user to handle it.

But the exception could also be produced by any kind of error in HandleEvent, so handling it like in the example would be misleading. But you are saying that there are no wrapped exceptions for dropped subscription from EventStore?

@w1am
Copy link
Contributor

w1am commented Mar 4, 2024

That's correct. It has been like this the entire time for regular subscriptions. The code is now much cleaner :)

@w1am w1am closed this as completed in #291 Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants