Re-raising a RuntimeError - good practice?

[Steven D'Aprano]

On Thu, 24 Oct 2013 20:17:24 -0500, Andrew Berg wrote:

> On 2013.10.24 20:09, Victor Hooi wrote:
>> Also, @Andrew Berg - you mentioned I'm just swallowing the original
>> exception and re-raising a new RuntimeError - I'm guessing this is a
>> bad practice, right? 

Well, maybe, maybe not. It depends on how much of a black-box you want 
the method or function to be, and whether or not the exception you raise 
gives the caller enough information to fix the problem.

For instance, consider this:

# Pseudocode
def read(uri):
    try:
        scheme, address = uri.split(":", 1)
        if scheme == "file":
            obj = open(address)
        elif scheme == "http":
            obj = httplib.open(address)
        elif scheme == "ftp":
            obj = ftplib.open(address)
        return obj.read()
    except Exception:
        raise URIReadError(some useful message here)

since it swallows too much: anything from file not found, permissions 
errors, low-level network errors, high-level HTTP errors, memory errors, 
type errors, *everything* gets swallowed and turned into a single 
exception.

But it isn't that the principle is wrong, just that the execution is too 
greedy. The designer of this function needs to think hard about which 
exceptions should be swallowed, and which let through unchanged. It may 
be, that after thinking it through, the designer decides to stick with 
the "black box" approach. Or perhaps she'll decide to make the function a 
white box, and not catch any exceptions at all. There's no right answer, 
it partly depends on how you intend to use this function, and partly on 
personal taste.

My personal taste would be to let TypeError, OS-, IO- and networking 
errors through unchanged, and capture other errors (like malformed URIs) 
and raise a custom exception for those.



>> If I use just "raise"
>> 
>>         except Exception as err:  # catch *everything*
>>             logger.error(err)
>>             raise
>> 
>> that will just re-raise the original exception right?
> 
> Yes. However, if you are doing logging higher up where you actually
> handle the exception, then logging here is redundant, and you can simply
> eliminate the try/catch block completely.


Yes, this! Try to avoid having too many methods responsible for logging. 
In an ideal world, each job should be the responsibility for one piece of 
code. Sometimes you have to compromise on that ideal, but you should 
always aim for it.


-- 
Steven