Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop masking exceptions/inherit from masked exceptions #4064

Closed
golyalpha opened this issue May 5, 2020 · 3 comments
Closed

Stop masking exceptions/inherit from masked exceptions #4064

golyalpha opened this issue May 5, 2020 · 3 comments
Labels
invalid This is not right.

Comments

@golyalpha
Copy link

golyalpha commented May 5, 2020

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'
@golyalpha golyalpha changed the title Stop masking exception/inherit from masked exceptions Stop masking exceptions/inherit from masked exceptions May 5, 2020
@Rapptz
Copy link
Owner

Rapptz commented May 5, 2020

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 d9e54d7 (before the 1.0 release). The last time this was tweaked was 11 months ago (here: 0a21591) which fixed a bug where a separate exception type was thrown (see #2211).

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

Cheers.

@Rapptz Rapptz closed this as completed May 5, 2020
@Rapptz Rapptz added the invalid This is not right. label May 5, 2020
@golyalpha
Copy link
Author

golyalpha commented May 5, 2020

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This is not right.
Projects
None yet
Development

No branches or pull requests

2 participants