TimeoutException instead of CancellationException on timeout #1031
Conversation
|
|
||
| /** | ||
| * This is an internal class and is only public for access. | ||
| */ | ||
| public class NatsRequestCompletableFuture extends CompletableFuture<Message> { | ||
| public static final boolean USE_TIMEOUT_EXCEPTION = Boolean.getBoolean(System.getProperty("nats.use-timeout-exception")); // TODO: Remove. Use TimeoutException instead of CancellationException. |
There was a problem hiding this comment.
Can we add nats.use-timeout-exception in the Option class
public static final String PROP_USE_TIMEOUT_EXCEPTION = PFX + "use.timeout.exception";
This will also mean that you will have to modify the NatsRequestCompletableFuture constructor to receive the setting from the connection and then change NatsConnection line 1170
There was a problem hiding this comment.
done. however, it would be nice if you labeled these changes as temporary to support legacy. in the next version, it's preferable to use TimeoutException without additional questions, IMO.
There was a problem hiding this comment.
I'm okay with adding a comment to make this the default in future versions.
| * Get whether to throw {@link java.util.concurrent.TimeoutException} on timeout instead of {@link java.util.concurrent.CancellationException}. | ||
| * @return the flag | ||
| */ | ||
| public boolean isUseTimeoutException() { |
There was a problem hiding this comment.
To be more consistent with other booleans in this class, please use useTimeoutException no is
|
|
||
| public boolean isUseTimeoutException() { | ||
| return useTimeoutException; | ||
| } |
There was a problem hiding this comment.
you don't need this method.
|
@maximbreitman Please update your branch. Also tests in |
It would be nice to take the first step in this direction - #797
Since this change is still "breaking", we can make it optional until next version