Remove unnecessary exception catching #1224
Conversation
Exceptions generated by eth-abi are fairly descriptive. It seems a bit redundant to wrap encoding exceptions with this extra message and also filter out the original exception type.
davesque
force-pushed
the
exc-catching
branch
from
50e24d9
to
503564b
Compare
|
What's the reasoning behind that? Is it that it's annoying to have to import exceptions to catch them? I think I noticed that and tried to follow suit, but eventually I got frustrated that there isn't always a built-in exception that seems appropriate for the given situation. Then, I just broke down and went back to having a custom set of One reason I kinda like that is exceptions can more directly correspond to the task that's being done e.g. |
|
This situation is a good example of what I'm talking about, by the way. It's confusing that |
Yeah, that seems about right. Roughly that the global exceptions are a nice standard to rally around. When possible, it's better to stick to a global standard than a project-level one. Like I said, not a strong feeling though (at a basic level, someone naturally has to learn your API to work with your library).
Sounds good to me 👍 |
What was wrong?
Exceptions generated by eth-abi are fairly descriptive. It seems a bit
redundant to wrap encoding exceptions with this extra message and also
filter out the original exception type.
Also, some code wrapped in this try/except block could never throw
EncodingErrorexceptions.I poked around and it doesn't look like any dependent code is expecting a
TypeErrorspecifically. Also, the test suite still seems to pass. So I'm not sure if there's much risk here.How was it fixed?
Removed the relevant try/except block.
Cute Animal Picture