Higher-level exception types

[seanpearsonuk]

Design and use higher-level exceptions that specify the actual issue rather than emitting bland, lower-level built-in exception types.

E.g., fluent_container.py

Raises
    ------
    KeyError
        If license server is not specified through an environment variable or in ``container_dict``.
    ValueError
        If server info file is specified through both a command-line argument inside ``container_dict`` and the  ``container_server_info_file`` parameter.
    ValueError
        If ``fluent_image`` or ``image_tag`` and ``image_name`` are not specified.

[raph-luc]

I think some nuance and discussion is required on which exceptions actually need custom types, and which ones will just end up being unnecessary additional lines of code that no one actually ever catches and handles.

I agree that we overuse some exception types (like RuntimeError) and those should be changed to more appropriate types (not necessarily custom types).

In general, sticking to the built-in concrete exceptions as much as possible seems to be the most efficient way to go about this, and makes the most sense, to me.

In the example above and based on the built-in exceptions documentation:

• KeyError is raised "when a mapping (dictionary) key is not found in the set of existing keys", which is exactly the situation there.

• ValueError is raised "when an operation or function receives an argument that has the right type but an inappropriate value", which IMO also accurately describes both situations there, as arguments have the right types but inappropriate values. Types can be NoneType, so it is not a TypeError when these parameters aren't specified, but in these cases the values or value combinations are inappropriate.

I don't think custom exceptions for these examples would be helpful. With that said, I do agree there may be exceptions types/classes that need to be re-evaluated in the code, and I think we should need an actual reason/use to add custom exceptions (i.e., say if we need to differentiate between exception types and catch a specific one that may be raised simultaneously to others, or the built-in exceptions just aren't appropriate at all).

Edit: also note that exceptions are usually raised with an associated text message, and this message is where the actual helpful/specific error information is located, in terms of informing the user of what went wrong. In my understanding, it is not the job of the exception type/class to necessarily be very informative or thorough, that is the job of the exception message.

[seanpearsonuk]

So

try:
  # call high-level operation that defines no exception types
  some_high_level_operation()
# I know what low-level methods are called in the implementation and what they raise, and neither will ever change
except ValiueError as ex:
  # I know all the various possibilities - and the details of the messages won't ever change.
  # Also, there's surely only one method that can raise a ValueError.
  if error_indicator_1 in str(ex):
     handle_error_indicator_1()
  elif error_indicator_2 in str(ex):
     handle_error_indicator_2()
except KeyError:
  # etc

[raph-luc]

@seanpearsonuk yes, I believe differentiating by the exception message (e.g., check if "server info file" in str(ex)) as you propose there should work, but assuming we expect an user might realistically need to differentiate between those two different ValueErrors and might have a way to automatically handle those differently, that is one of the situations I mentioned above where it would make more sense to add a custom exception class for one of those exceptions.

But I don't know what this potential custom exception would be called, and I also don't know whether it would realistically ever be used. I would expect these basic configuration exceptions will usually be resolved manually and not automatically (also considering they can be differentiated by the message as well, so the custom type may not be needed at all even for automatic solutions, as you noted).

[seanpearsonuk]

Hi @raph-luc just one point for clarification: regarding 'check if "server info file" in str(ex)) as you propose there should work', I was not proposing to do this.