iota.py icon indicating copy to clipboard operation
iota.py copied to clipboard

Can exceptions be improved to provide more useful feedback?

Open scottbelden opened this issue 7 years ago • 1 comments

At the moment, when one calls an API incorrectly and an exception is raised, it usually ends with something like this:

ValueError: Request failed validation ({'transaction': ['wrong_type']}) (`exc.context["filter_errors"]` contains more information).

Okay, so I used the wrong type. However, to get the useful information, I have to modify my code from this:

call_api(wrong_type)

to this:

try:
    call_api(wrong_type)
except ValueError as e:
    print(e.context)
    raise

And now I get the useful information such as the following:

{'code': 'wrong_type', 'message': 'Transaction is not valid (allowed types: TryteString, bytearray, bytes, str).', 'context': {'value': <iota.transaction.base.Transaction object at 0x7f500de2c898>, ...}

So now I can fix the wrong_type and remove the exception wrapping. However, it seems like information should be in the original exception message.

Maybe once I use mypy it will catch these type errors, but at least for me it seems that these exceptions could be more helpful from the start.

scottbelden avatar Dec 21 '17 21:12 scottbelden

Hey @scottbelden thanks for raising this issue!

I'm inclined to agree with you. Most Python developers (probably developers in general) wouldn't think to look for a context object attached to an exception, so it violates the priciple of least astonishment.

I've tried to nudge developers to look for it by adding a hint to some of the exception messages ((exc.context has more info)), but in retrospect, if the context object is that important for diagnosing the issue, then maybe that information does belong in the exception message.

I think there are still some places where it makes sense to have a context object (e.g., errors related to bundle validation), and it may still be useful to have context even if it includes information already in the exception message (e.g., filter-related errors contain a ton of extra info aside from the error messages that can be used to diagnose issues).

How about we start by identifying which exception messages need to be improved (maybe look for incidences of "``exc.context`` has more info"), and then we can log individual issues to address each one (that way we can keep track, and if necessary have individual discussions about what changes to make).


Sidebar: For context (jej), on the last application I worked on, I built a logging framework that stored the context objects alongside the exception messages. Whenever a runtime error occurred, the exception message was short and generic (to make it easier to filter out noise when the same error occurs lots of times), and the context data were easily accessible. I approached PyOTA with the same mindset.

todofixthis avatar Dec 22 '17 21:12 todofixthis