Closed adrianhopebailie closed 7 years ago
+1 on the rename to InterledgerProtocolException
.
However, can you clarify the value of InterledgerCoreRuntimeException? Just wondering why not use RuntimeException
(I'm assuming that callers will need to know something deeper about the error, but am trying to reason through what exactly that is, if it's not a "Protocol" error).
A number of static analysis tools I have run on the code suggest changing this. There are two good reasons I can think of:
Makes sense. What if we just called it InterledgerRuntimeException
instead? Then we would have the following structure:
InterledgerRuntimeException extends Runtime Exception
(used for all exceptions, including protocol exceptions).InterledgerProtocolException extends InterledgerRuntimeException
(holds an InterledgerError
)(I kind-of agree with the above suggestion, but am open to an argument in-favor of keeping the two exceptions distinct).
Implemented like you suggest. See PR #58
We should extend
RuntimeException
so that users of the library can differentiate between exceptions from our lib and others.That said, we can't use
InterledgerException
as it is, because this is designed to be thrown in the case of anInterledgerError
(i.e. a protocol exception).Suggest we rename
InterledgerException
toInterledgerProtocolException
and add a sub-class ofRuntimeException
calledInterledgerCoreRuntimeException
that is used within the library and is package private.