On Sun, 28 Apr 2024 09:54:34 GMT, Viktor Klang <vkl...@openjdk.org> wrote:
> Primarily offering this PR for discussion, as Throwables throwing exceptions
> on toString(), getLocalizedMessage(), or getMessage() seems like a rather
> unreasonable thing to do.
>
> Nevertheless, there is some things we can do, as witnessed in this PR.
src/java.base/share/classes/java/util/concurrent/CompletableFuture.java line
352:
> 350: return wrapping;
> 351: }
> 352:
This is the main thing—and the big question is if this approach is the best
path or not.
src/java.base/share/classes/java/util/concurrent/CompletableFuture.java line
2653:
> 2651: * its completion state. The state, in brackets, contains the
> 2652: * String {@code "Completed normally"} or the String {@code
> 2653: * "Completed exceptionally"}, or the String {@code "Not
These two looks like a discrepancy between spec and implementation, and it
seemed safer to amend the spec rather than the implementation, as code might be
relying on *actual behavior* rather than the spec:ed one.
@DougLea You might have some thoughts to offer on this :)
src/java.base/share/classes/java/util/concurrent/CompletableFuture.java line
2670:
> 2668: : "[Not completed, " + count + " dependents]")
> 2669: : (((r instanceof AltResult) && ((AltResult)r).ex != null)
> 2670: ? "[Completed exceptionally: " +
> ((AltResult)r).ex.getClass().getName() + "]"
@DougLea @AlanBateman It seemed safer to switch to outputting the cause FQCN
rather than wrapping in a try-catch and trying to come up with a fallback text.
The reason for amending this is for things like JShell, results are
toString():ed which would mean that the current session would break if
toString():ing the result throws (also further signalling that throwing on
toString() seems unreasonable in general).
test/jdk/java/util/concurrent/tck/CompletableFutureTest.java line 87:
> 85: // even if thrown exceptions end up throwing exceptions from
> their String
> 86: // representations.
> 87: @Override public String getMessage() {
I decided to do this to make sure that *all pre-existing test cases* would
still work even if the exception used has a misbehaving toString() (toString()
transitively calls getLocalizedMessage() which in turn calls getMessage())
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18988#discussion_r1582075756
PR Review Comment: https://git.openjdk.org/jdk/pull/18988#discussion_r1582075209
PR Review Comment: https://git.openjdk.org/jdk/pull/18988#discussion_r1582075561
PR Review Comment: https://git.openjdk.org/jdk/pull/18988#discussion_r1582075630
