Added common Exceptions.throwIfAny to throw a collection of exceptions #2601
Conversation
| } else { | ||
| throw new CompositeException( | ||
| "Multiple exceptions" + whileText, exceptions); | ||
| } |
There was a problem hiding this comment.
I think this would be easier to read:
if (exceptions.size() == 1) {
Throwable t = exceptions.iterator().next();
if (t instanceof RuntimeException) {
throw (RuntimeException) t
}
if (t instanceof Error) {
throw (Error) t;
}
}
throw new CompositeException("Exception" + whileText, exceptions);
There was a problem hiding this comment.
Because the original places distinguished between single and multiple expections in the message of CompositeException.
There was a problem hiding this comment.
As per the discussion at #2602 ... this is an example where @akarnokd and I have slightly different styles, but it's okay. I always have else if on the same like, here it is split, but I can still read the code and it works so I choose to not make a big deal out of it.
Sometimes when @akarnokd and I end up modifying each others code we'll see some lines get modified to personal style if we touch them, but we don't go around explicitly trying to change code just to match a personal preference ... if for no other reason than to not clutter changelogs and diffs.
|
This is a nice change ... just the comments on using |
| throw (RuntimeException) t; | ||
| } else { | ||
| throw new CompositeException( | ||
| "Failed to unsubscribe to 1 or more subscriptions.", es); |
There was a problem hiding this comment.
This looks like a mistake to have been a CompositeException since it's not composite, there is only 1. If it was a RuntimeException on the other side of the branch we just threw it, so we should just be using propagate here.
Perhaps the reason this was done was to add a better error message? That's something an overloaded propagate could add.
|
Updated the code based on the comments above. |
| * @return because {@code propagate} itself throws an exception or error, this is a sort of phantom return | ||
| * value; {@code propagate} does not actually return anything | ||
| */ | ||
| public static RuntimeException propagate(Throwable t) { | ||
| return propagate(t, null); |
There was a problem hiding this comment.
Why do we have return values on these when they end up just throwing? I just noticed that. We can't change it, but it's very odd.
There was a problem hiding this comment.
It is explained by the javadoc.
|
That build failure is concerning ... it hung and timed out after 10 minutes at this test:
|
|
Most likely it died of stackoverflow while attempting to report stackoverflow. I've removed the custom message and manually inlined the propagate method. |
Added common Exceptions.throwIfAny to throw a collection of exceptions
Tried to be as flexible with the error text as possible.