Skip to content
This repository was 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
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/rpc/src/author/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Contributor

@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.

.send_all(Compat::new(watcher))
.map(|_| ())
});
Expand Down