Reactor-core: Enrich message of `elementAt`'s `IndexOutOfBoundsException`

[sjh836]

Motivation

MonoElementAt throws IndexOutOfBoundsException in the case below.

1. index is negative
2. ElementAtSubscriber.done is false until onComplete() is called. (when defaultValue is null)

I think IndexOutOfBoundsException is fine in case 1.
(Other operators in a similar situation use IllegalArgumentException. e.g. FluxTake, FluxSkip)

But I think NoSuchElementException is correct rather than IndexOutOfBoundsException in case 2.

Because it may not actually have exceeded the index range.

It can be confusing for developers.

Steps to Reproduce

It seems to occur when combined with certain conditions in an infinite stream.

// code
Flux.interval(Duration.ofMillis(500))
        .filter(i -> i % 2 == 0)
        .take(Duration.ofSeconds(3))
        .elementAt(8)
        .subscribe(System.out::println);

// console
01:42:18.027 [parallel-1] ERROR reactor.core.publisher.Operators - Operator called default onErrorDropped
reactor.core.Exceptions$ErrorCallbackNotImplemented: java.lang.IndexOutOfBoundsException
Caused by: java.lang.IndexOutOfBoundsException: null
    at reactor.core.publisher.MonoElementAt$ElementAtSubscriber.onComplete(MonoElementAt.java:160)
    at reactor.core.publisher.SerializedSubscriber.onComplete(SerializedSubscriber.java:146)
    at reactor.core.publisher.FluxTakeUntilOther$TakeUntilMainSubscriber.onComplete(FluxTakeUntilOther.java:244)
    at reactor.core.publisher.FluxTakeUntilOther$TakeUntilOtherSubscriber.onComplete(FluxTakeUntilOther.java:119)
    at reactor.core.publisher.FluxTakeUntilOther$TakeUntilOtherSubscriber.onNext(FluxTakeUntilOther.java:101)
    at reactor.core.publisher.MonoDelay$MonoDelayRunnable.run(MonoDelay.java:119)
    at reactor.core.scheduler.SchedulerTask.call(SchedulerTask.java:68)
    at reactor.core.scheduler.SchedulerTask.call(SchedulerTask.java:28)
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:180)
    at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
    at java.lang.Thread.run(Thread.java:748)

Desired solution

In case 2 above, throw NoSuchElementException instead of IndexOutOfBoundsException.

[sjh836]

If you give me positive feedback, I want to submit a PR :D

[simonbasle]

I don't think this is the right move

IndexOutOfBoundsException is legit in case 2, and doesn't violate the principle of least surprise since that's how e.g. List#get(int) behave as well...

In my opinion, the usage in case 1 is actually more debatable, but since users could currently rely on that particular exception to be thrown (eg. in a catch block), it is hard to change at this point anyway.

But let's get back to case 2, can we at least improve the message?

The operator throws IndexOutOfBoundsException in onComplete because that's the step where it can be sure no more values will come from the source.

However, the exception doesn't include a message and as such provides less context than it could. The trouble is that the operator works on a "countdown" logic, and doesn't retain neither the requested index nor the actual size of the source.

If we add a final int target; field, we can rework the logic and make it so the message displays something like:

source had 3 elements, expected at least 5

Note that the target index is 0-based, so eg. the 5 above is really (target+1).

[simonbasle]

@sjh836 do you want to try a PR for the above message change? if so, please branch out of 3.3.x for this one, and we'll forward the fix to master when merging.

[sjh836]

Thank you for your explanation. 👍
I tried it!