Timeout behavior in Mono.block(timeout) about reactor-core HOT 4 CLOSED

Hi, @ferrerogg! Thanks for bringing this up. I found a previous discussion regarding this and some interesting points there.

1. The idea that you bring about nesting the TimeoutException was suggested by @simonbasle in #2267 (comment)
2. Afterwards it was dismissed: #2267 (comment)

Apparently, it seems it is of importance to some users, including yourself. So let's reconsider it. Can you please share a little bit what the scenario is that you need this in? I don't want to frame guesses, but I have a feeling in 2024 this might be a more frequent case. However, I'd love to make sure that we are not promoting bad patterns with whatever the end solution ends up looking like.

Regarding a different Exception type, it would be a breaking change and I'd prefer not to do it.

However, adding a cause or a suppressed exception would be a behaviour change, yet I dare to say not a breaking one.

So I look forward to some usage examples and scenarios and unless there is pushback I'd be up for improving this situation. Obviously it won't be without cost, as adding something without changing the type of the thrown exception requires a bit more of runtime memory to be allocated – but that can be a preferable tradeoff compared to string matching. Therefore, if we decide to change this, I'd target this for 3.6.x line.

from reactor-core.

Hi @chemicL
I'm a newbie in Reactor, so I'm trying to experiment.
My use case is quite simple in pseudo-code

List  = Flux.fromIterable(myListOfItemsToSubmitToSpringWebClient)
  .flatMapSequential(item -> callWebClient(item))
  .collectList()
  .block(maxWorkTimeout);

This fragment calls a Spring WebClient that posts an "item" to a remote web service (not managed by me). Item responses are then collected into a List and then further processed in a old-style blocking code library (not managed by me).
Because remote service SLA is unknown, I would like to try the above processing with a timeout, so that caller could retry later if the remote service is overloaded.

Using block(timeout) seems the the intuitive and easiest way to achieve this requirement.
I also found that I can probably refactor previous code introducing a .timeout()

List  = Flux.fromIterable(myListOfItemsToSubmitToSpringWebClient)
  .flatMapSequential(item -> callWebClient(item))
  .collectList()
  .timeout(maxWorkTimeout);
  .block();

This code would throw a "wrapped" checked java.util.concurrent.TimeoutException, so I guess I can use this instead of digging into IllegalStateException message.

Nevertheless I think that both approaches (.timeout(Duration) and .block(Duration)) should lead to similar exception handling it they are interchangeable

from reactor-core.

@ferrerogg thanks for providing your use case, it sounds like a typical scenario. I agree it makes sense to provide a better default than having to add an additional operator. Therefore, adding a TimeoutException with the same message as the IllegalStateException as a cause is the aim here. I'll mark this issue as a good one for contribution, perhaps someone would like to give it a go.

from reactor-core.

Thank you for the nice issue and discussions!

However, adding a cause or a suppressed exception would be a behaviour change, yet I dare to say not a breaking one.
Therefore, adding a TimeoutException with the same message as the IllegalStateException as a cause is the aim here.

I checked all discussions and agree with this way, so created fix PR #3733 :)

from reactor-core.