Use Purpose-Built Exceptions

[howech]

While this example applies mainly to parsing psbts, the logic probably applies to more of the buidl library.

When I am parsing a psbt that has come to me by way of user input, I end up having to write code that looks like this:

    try:
        PSBT.parse(inputstream)
    except Exception as parsing_error:
        do_something_with(parsing_error)

The problem here is that except Exception is a pretty wide field of exceptions to be catching. But because PSBT.parse() can raise anything from a ValueError (built in python exception) to a MixedNetwork (buidl-defined exception based on Exception), I have to cast a pretty wide net to gather in all of the exceptions that parse might raise. The problem here is that I am now potentially catching exceptions that may well be actual bugs in my code or in the buidl library in exactly the same way that I am catching errors that buidl is trying to tell me about the psbt I am parsing.

Is there any way to make it so that when parsing a psbt, buidl will raise exceptions derived from a purpose-built base class?

[mflaxman]

Is the idea that you'd want something like an InvalidPSBTError that is always raised instead of the various ValueError, RuntimeError and SyntaxError that are currently raised in psbt.py? That way you could easily tell if it were an issue with the input vs an issue with the library?

It still might be hard to rely on if there are calls out to other helper modules that have their own (collection of) errors. For example, if you parse() a NamedPublicKey/NamedHDPublicKey it will call add_raw_path_data which will use the helper.py method parse_binary_path() which could throw its own exception.

I think it really depends on what kind of error(s) you're trying catch? I'm trying to understand the use-case better, because the perfect solution might be to litter the codebase with dozens of new raiseable exceptions (that should never be called).

Another messy solution if there's an error that only affects some rare corner case would be to parse the exception like this:

try:
    some_method()
except Exception as e:
    if "rare edge case description" in str(e):
        handle_rare_edge_case()

[howech]

Well, yeah, kind of. I want to be able to choke off the exceptions that PSBT.parse() might throw in part of its 'normal operation' - where here I may be extending the idea of 'normal operation' from what it's currently designed to be. As a transport layer, psbts are subject to being modified by users between when they are generated and signed. I would like to be able to distinguish things that arise from a bad psbt from things that arise from sone deeper coding problem. I need to be able to alert my user that he gave me a bat psbt, I need to crash and generate a stack trace if something more serious has occurred.

If buidl doesn't distinguish between 'errors that arise because of parsing bad inputs' and 'errors that arise because something went wrong deeper in the code', its effectively going to force me as a buidl-client to wrap my calls to buidl in a kind of anti-corruption layer that does a sort of crappy job of trying to do the same thing externally - basically, I'll have to wrap calls to PSBT.parse() and Tx.parse() or .parse_hex() into calls that raise my own flavors of ParseException() with a nested exception showing whatever it was that the buidl library raised. While this doesn't really protect me from the fact that the two types of exceptions are comingled in the buidl library, it does at least keep that from spreading out to my code.

I keep looking for a good place to use as a reference for 'python best practices for exceptions'. If you google it, there are several of them around, and they do tend to say contradictory things, like dont implement a new exception when IndexError works just fine, and write lots of Exceptions. I think that there is a case here around the fact that we are dealing with a piece of data (the serialized psbt) which we could well expect to be corrupted without there being any kind of coding error in either the buidl library or in the client code. As such it warrants some kind of exception that the client code can handle without having to either 1) know all of the different exceptions that parsing a bit of psbt can throw so it can handle all of the cases, or 2) resort to except Exception.

This logic might even carry forward to any calls to .validate() on the various buidl objects. If I build an object by passing data into its constructor and that data is inconsistent in some way, I can imagine that being acceptible to be called a coding error and have it throw a ValueError or something in my face for my efforts. However, if I say create an empty transaction and then mutate it by adding inputs and outputs to it. When I'm done, when I call validate() on the result and the transaction is not in fact valid, that is probably still some kind of a data error and not necessarily a coding error at all. Again, in this case I would expect to get something back in the way of an InvalidTransactionError instead of a KeyError (the former being a custom built error that would then become more or less part of the interface for the validate method on a transaction, where the latter is indistinguishable from indexing a python hash with a key that is not present.