SnapdError is too generic

[Leo Arias]

We have an exception in snapcraft/internal/errors.py called SnapdError. This exception is raised in two different contexts, passing a different message for each case.

This exception should be removed or only be used as a base class. We should add individual exception for each possible error.

[Sergio Schvezov (sergiusens)]

Sent from Mailspring (https://link.getmailspring.com/link/local-78f2527c-9667@lindon/0?redirect=https%3A%2F%2Fgetmailspring.com%3Fref%3Dclient&recipient=1734235%40bugs.launchpad.net), the best free email app for work

On Nov 23 2017, at 11:55 pm, Leo Arias <email address hidden> wrote:
> Public bug reported:
>
> We have an exception in snapcraft/internal/errors.py called SnapdError.
> This exception is raised in two different contexts, passing a different
> message for each case.
>
> This exception should be removed or only be used as a base class. We
> should add individual exception for each possible error.

Why can't we have one exception that handles the whole thing? Imagine doing that for the store errors, we will go crazy.

[Leo Arias (elopio)]

If we follow this style of generic exceptions it is very hard to get them consistent, and showing all the information that they should include, because exception messages are spread all over the code base.

It's ok to factor the common things in a base class. But what we are doing is naming a very generic class and sending a full formatted message to it from the source code. In this case, what's wrong with Snapd? Is it not installed? Are we calling it without permissions? Is the socket or the binary command the one with problems?

In most of our error messages, what we are doing is to define a class that is as specific as possible. This class receives parameters with values of the particular details of the context. This has been very useful when analyzing what's wrong with the messages. And it offers a big room for improvement. We can document on the code which specific exceptions can be thrown when things go wrong, without reading the error message to understand what's happening. On the errors module, we can document what conditions can cause an error, without a hard link between the current implementation in the code and error conditions. That will be useful for reuse of errors, and for documenting the commands that will fix an error condition.

In the end, it's just a separation of concerns. The plugin handler module shouldn't know about the commands that need to be run when something it assumes is wrong. It should just know by reading at the docs of the possible errors that if assumption a is not met, throw exception a.