Rapptz / discord.py

An API wrapper for Discord written in Python.
http://discordpy.rtfd.org/en/latest
MIT License
14.71k stars 3.74k forks source link

Stop masking exceptions/inherit from masked exceptions #4064

Closed golyalpha closed 4 years ago

golyalpha commented 4 years ago

Summary

This issue doesn't affect a particular version of this library, nor a particular version of the Python interpreter.

Discord.py is using it's own custom exceptions to repackage standard Python exceptions into something else. These changes do not even follow SemVer (not sure if Discord.py uses that, but that's besides the point), and make upgrading to the next minor update a pain for developers. (This is literally the fourth time I'm updating my code, because someone has made breaking changes in the same major version.)

Proposals

  1. Follow semver guidelines for versioning - this would prevent me and other developers from updating DiscordPy, with the expectation of things working, only to find out our bots have crashed because changes within the same major release have broken the API.

  2. Use stdlib exceptions where possible. Masking stdlib exceptions with library-specific exceptions may make sense. In that case, inherit from the masked exception, so code written to handle said exception, can also handle the masked version.

Footnote

We're all consenting adults. We know what we're doing. And if we screw up, then it's our problem. But if you keep renaming exceptions under our noses every release, breaking our code, then we aren't the ones screwing up.

Sorry if this seems condescending or anything like that. But I am sick and tired of having to rewrite my entire code base every time a new update for DiscordPy is released, because someone decided to rename or completely change exceptions, rename methods, change around classes, and I don't even want to know what else. But, this really shouldn't be happening:

discord.ext.commands.errors.ExtensionFailed: Extension '' raised an error: AttributeError: 'NoneType' object has no attribute 'exec_module'
Rapptz commented 4 years ago

Hello.

I am unsure when the last time you've updated your code was -- but it must have been a long time ago. Version 1.0 of the library has come out about a year and a month ago by now, and with it came new version guarantees that you speak of. Hopefully that takes care of #1 for you.

As for #2, the exception design has not changed in a while. Likewise, I am once again unsure of the timeline of these frustrations of yours. This is a Git repository so history is open for all to see. This change in particular is 14 months old as seen here https://github.com/Rapptz/discord.py/commit/d9e54d7dd36368bb97b3c31225901ba80cb81a62 (before the 1.0 release). The last time this was tweaked was 11 months ago (here: https://github.com/Rapptz/discord.py/commit/0a21591d0c778f3f3596362fb1ae92ffc137e576) which fixed a bug where a separate exception type was thrown (see #2211).

As a so called "consenting adult" I hope you understand.

Cheers.

golyalpha commented 4 years ago

I've made an update to the library, which changed the AttributeError handling to discord.ext.commands.errors.NoEntryPoint handling 10 months ago.

Today, I've had to change that again to ExtensionFailed.

Now, I don't exactly know, what prompted that change, but either way, both exceptions are obviously masking the AttributeError stdlib exception, and cannot be caught an handled if the developer expects the AttributeError to be raised (which is happening) and presented (which is not, because of DiscordPy) to them.

golyalpha commented 4 years ago

ExtensionNotFound Extension could not be imported

There's an ImportError for that.

NoEntryPointError Extension does not have a setup function

That is called an AttributeError

ExtensionFailed The Extension setup function had an execution error

That's either a RuntimeError, or literally any expcetion that can be raised at runtime which is not already mentioned above. Now I don't even know what exception I am handling anymore, thanks.

You having to specifically write code to make sure, that you're raising the right exception in the right circumstances should've probably been the first hint, that something was wrong. So, no, as a consenting adult, I don't understand. But I guess I'll have to keep dealing with this.