tbot icon indicating copy to clipboard operation
tbot copied to clipboard

Base exception classed raised

Open avylove opened this issue 3 years ago • 3 comments

It looks like there are 40 cases where raise Exception is used rather than a subclass of Exception. This makes it impossible to catch Tbot-specific errors. Ideally, every exception raised should inherit from tbot.error.TbotException

avylove avatar Sep 19 '22 16:09 avylove

Hi,

yes, this is very true. As you saw, a while ago I added the tbot.error module with the intention to fix this mess and to have proper error types for each exception. The idea was (and still is) to have all those error types in tbot.error. So there is one common module for errors instead of them being sprinkled all over the codebase.

Unfortunately, I never got to finishing this work. I have some more changes related to this on a local branch. I'll see how much of it is still salvageable to finally get this over with. IIRC, I ran into some nasty cyclic import problems back then, but I'll have to check again...

Rahix avatar Sep 20 '22 22:09 Rahix

@avylove, if you have time, I started working on this in #79. I don't want to do everything at once now because we're committing to a huge part of the "API" here. I want to be a bit careful so we don't corner ourselves in a compatibility nightmare with the exception types...

Maybe you can give a few comments on which exceptions you'd need the most? I think CommandFailure was already the top priority from my side...

Rahix avatar Sep 27 '22 09:09 Rahix

Looks like a great start! Thanks for working this!

CommandFailure is definitely the top priority for me, too. I think the next two cases would be failures requesting a machine context and channel errors.

avylove avatar Sep 27 '22 10:09 avylove