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

implement simple error propagation for subscriptions #105

Closed

Conversation

dt665m
Copy link

@dt665m dt665m commented May 11, 2020

Propagating errors is currently not implemented. This PR tries to enable Client Subscriptions to signal downstream that a channel has been closed due to errors in the inner RawClient.

  • On RawClient error, clear all active subscriptions, which drops all the mpsc senders.
  • Implement future's Stream trait for Client::Subscription so that the inner mpsc receiver can be polled to completion. Ready::(None) when the mpsc sender is dropped.

The stream trait was implemented such that current libs depending on the Subscription.next() API, such as substrate-subxt, wont break. Users can choose to poll the Subscription as a wrapped stream to get notified of the closed internal mpsc receiver or use subscription.next() future, which will loop with "pending" forever on error.

@parity-cla-bot
Copy link

It looks like @dt665m signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@tomaka
Copy link
Contributor

tomaka commented Jun 17, 2020

Sorry for not responding to that. There are caveats deeper in the client regarding error propagation that need to be fixed as well, and I'll need to come back to this and look in the details.

@dt665m
Copy link
Author

dt665m commented Jun 18, 2020

@tomaka no worries, this isn't a complete implementation of error handling as the I didn't have much time to look deeper into the RawClient. For my use case, I just needed a way to be notified if an error occurred the temporary solution was to close the channel.

@dt665m dt665m force-pushed the feature/client-error-handling branch 3 times, most recently from 098bfa2 to f4408c2 Compare July 7, 2020 10:24
@dt665m dt665m force-pushed the feature/client-error-handling branch from 2bc5566 to 50b610b Compare July 23, 2020 05:55
@niklasad1
Copy link
Member

Fixed by #180

@niklasad1 niklasad1 closed this Jan 18, 2021
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

Successfully merging this pull request may close these issues.

4 participants