Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Don't panic on a sink error #7666

Merged
merged 1 commit into from Dec 3, 2020
Merged

Don't panic on a sink error #7666

merged 1 commit into from Dec 3, 2020

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented Dec 3, 2020

No description provided.

@bkchr bkchr added A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Dec 3, 2020
@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Dec 3, 2020
@@ -215,7 +215,7 @@ impl<P, Client> AuthorApi<TxHash<P>, BlockHash<P>> for Author<P, Client>
Ok(watcher) => {
subscriptions.add(subscriber, move |sink| {
sink
.sink_map_err(|_| unimplemented!())
.sink_map_err(|e| log::debug!("Subscription sink failed: {:?}", e))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a target maybe?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other logs in the crate also don't use any target 🙈

But I also don't have a better target than the automatic sc_rpc. I could just come up with sc-rpc 🙈

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sc-rpc sounds good

Copy link
Member

@andresilva andresilva Dec 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do this in another PR if we really want it. None of the RPC logging calls in client/rpc have any target set (I checked), so if we want to change this we should update all of them.

@ordian ordian merged commit f73ac5e into master Dec 3, 2020
@ordian ordian deleted the bkchr-rpc-no-unimplemented branch December 3, 2020 16:11
darkfriend77 pushed a commit to mogwaicoin/substrate that referenced this pull request Jan 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. A2-insubstantial Pull request requires no code review (e.g., a sub-repository hash update). B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants