search

aatxe / irc

the irc crate – usable, async IRC for Rust
Mozilla Public License 2.0
528 stars 97 forks source link

Can't catch unwind on Stream #122

Closed Archina closed 6 years ago

Archina commented 6 years ago

I keep running into a panic through 'connection reset: no ping response' that I cannot recover from.

I tried to catch_unwind on the message stream from the IrcClient but it notifies me that the trait UnwindSafe is not implemented for RefCell.

I am a bit confused as catch_unwind is listed as a feature of Client::Stream in the docs.

I have also tried to use the reactor instead of the direct IrcClient, neither seem to be able to recover from the connection reset.

Am I overlooking something or is this indeed a bug?

aatxe commented 6 years ago

Can you give some example code (even if it's not totally minimal, as long as it won't take me several hours to grok it)? If you're panicking on a connection reset on v0.13, I think this is either something happening somewhere in your code (i.e. an unwrap call on running a reactor) or is the result of using IrcClient::new or IrcClient::from_config which the documentation should now advise against exactly because of the inability to handle errors well.

If it is indeed occurring within the library using the newer APIs, it's definitely a bug, but as far as I can tell from my deployments, connection resets should be propagated properly as an error using IrcReactor or IrcClient::new_future.

As for the UnwindSafe part, it's a bit more subtle. We're returning the concrete type irc::client::ClientStream which implements the trait futures::Stream (re-exported as irc::client::prelude::Stream for convenience). While the latter does support UnwindSafe, this is only true if the concrete stream itself does which is not the case here. Hence, you're not able to use catch_unwind on ClientStream.

Archina commented 6 years ago

Hello @aatxe,

thank you for the rather quick response. I have been using the old IrcClient-way so far but changed to a reactor just recently as I updated the software. I will try to make a short test setup and see if I can reproduce the error in it for you.

Thank you also for clearing up that UnwindSafe is indeed not supported. It may be nice to note that in the docs because else wise one could assume it is available (and functional).

Archina commented 6 years ago

I looked a bit further into it and it is indeed correct that the IrcReactor does not lock up the process through a connection reset panic.

This seems only to be happening when you use the old IrcClient.

With this, I think the issue can be closed. Though I would like to emphases nothing that Streams cannot be catched unwinding.