mostlymaxi / twitch-interactive-things

random rust projects to make my twitch stream more interactive!
https://twitch.tv/mostlymaxi
MIT License
3 stars 6 forks source link

bot: Panic handling for commands #45

Closed lillianrubyrose closed 1 month ago

lillianrubyrose commented 1 month ago

Depends on #44 (I'll rebase after that gets squash merged here)

This is untested by me, puhLEASE test this one first maxi

This pr does as title says, uses std::panic::catch_unwind when running commands since one panic = bot dead.

Due to the UnwindSafe and RefUnwindSafe trait not being implemented on &RefCell<T> there are two routes I saw:

I went the second route since there's no multi-threading (past signal handling) and I don't /believe/ this would cause any issues to my knowledge. Also had to change the TwitchApiWrapper signatures to be &self instead of &mut self due to the UnwindSafe set of traits not being implemented on &mut T, now it just uses a RefCell for mutability of the wrapped Twitch API struct.

zuixalias commented 1 month ago

The panic handling seems redundant(?). The user submitted code could theoretically panic from various places, not just within the handle fn... the catch_unwind would need to be applied more broadly to prevent the bot from crashing, which could become cumbersome and lead to overuse of panic-catching. Also, we seem to be handling most potential failures well enough :shrug:

Well, but the handle fn is basically through which most of the code would run, it feels justified to implement it... idk

mostlymaxi commented 1 month ago

i feel bad not marge-ing because this looks awesome but the bot crashing is unironically fine, it spins itself back up. the thing that breaks the bot is usually franz 🥲