Exceptions Documentation and Testing #479
Conversation
|
LGTM probably have @yschimke look too |
|
Forgot some tests. 😄 |
| super(message, cause); | ||
| super( | ||
| Objects.requireNonNull(message, "message must not be null"), | ||
| Objects.requireNonNull(cause, "cause must not be null")); |
There was a problem hiding this comment.
nit: this seems overly strict given that cause in RuntimeException is nullable. As it's in error handling code, being super strict here is likely to just translate some business errors into rarely seen NPE.
There was a problem hiding this comment.
I’d wondered about this as I did it. Specifically, this was codifying existing intent. I’m happy to remove if you think that it the intent was inaccurate.
There was a problem hiding this comment.
My concern wouldn't be with the ApplicationException created by users of the library ever passing in something like
new ApplicationException("failed reading from DB", e.getCause())
I'd weakly vote for removing, but wouldn't fight to the death over it.
There was a problem hiding this comment.
@yschimke I'd agree with you, and since I haven't seen any objections here I'll update.
This change adds Javadocs and tests to the exception hierarchy in rsocket- core. This change introduces the first JUnit 5 tests (and a fancy test interface!) as well as using AssertJ to make the assertions more fluent and understandable.
This change adds Javadocs and tests to the exception hierarchy in
rsocket-core. This change introduces the first JUnit 5 tests (and a fancy test interface!) as well as using AssertJ to make the assertions more fluent and understandable.NOTE: There are a couple of API incompatibilities in this change, and if any of them are objectionable, I'm happy to revert them.
CancelExceptionwas renamed toCanceledException) to match the spec naming and to conform to the naming style of the other exceptions.NoAvailableRSocketException,TimeoutExceptionandTransportExceptionwere only used in thereactor-load-balancerproject. In an effort to keep them local to their usage, and to ensure thatreactor-corestays as spec-focused as possible, these types were moved to thereactor-load-balancerproject.