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

There is no exception propagation to TaskCompletionSource in some cases. And CancellationToken is not passed to TrySetCanceled. #158

Closed
Scormave opened this issue May 15, 2023 · 4 comments

Comments

@Scormave
Copy link

Hi!
I've found that in some cases exceptions that are caught in HubConnection aren't propagated to TaskCompletionSource, thus Task never ends.
For example, if some serialization IProtocol fires an exception, it is caught in method SendMessage of a HubConnection and sent to log without any notification to the upper systems. The only way users can handle such a situation is to pass some CancellationToken with timeout to InvokeAsync or SendAsync methods.

Also, I've noticed that CancellationToken that is passed to InvokeAsync or SendAsync methods is not passed to TrySetCanceled method in the cancellationToken.Register call. In this case it is impossible to implement some logic based on cancellation tokens.

@Scormave Scormave changed the title There is no exception propagation to TaskCompletionSource in some cases There is no exception propagation to TaskCompletionSource in some cases. And CancellationToken is not passed to TrySetCanceled. May 15, 2023
@Benedicht
Copy link
Owner

Thanks for the report!

Could you try out the file from here? https://gist.github.com/Benedicht/144b78494f8acf778cee6492edb19239
(You have to overwrite the old one in the \Best HTTP\Source\SignalRCore\ folder.)

It should fix both issues.

@Scormave
Copy link
Author

That's funny, I've made exactly the same changes locally to get everything work. But, are you sure that it is safe to throw exception from SendMessage for all code paths? I've added throw only for specific code paths, which go from InvokeAsync and SendAsync.

@Benedicht
Copy link
Owner

Added two additional cases catching the exception, but i think it's going to be OK. If not in the HubConnection then they will be catched and logged in the transport layer, but they will not cause any serious issue.

@Scormave
Copy link
Author

Ok, thank you! I will use the provided HubConnection for now and looking forward to the next update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants