Introduce awaitReceive and awaitReceiveOrNull APIs

[GeorgePap-719]

Summary

ServerRequest.awaitBody* APIs have some problems:

1. ServerRequest.awaitBody forces you to think about the coroutines-bridge exception handling, since it uses
   kotlinx.coroutines.reactive.awaitSingle. Also, it is not documented that it might throw NoSuchElementException
   and IllegalArgumentException for coroutines-related errors.

2. ServerRequest.awaitBody might throw IllegalArgumentException for two different reasons, for serialization-related
   errors and for coroutines-related errors.

3. ServerRequest.awaitBodyOrNull should never throw as the signature indicates. The convention established by the
   stdlib mandate that an operation with the name xxxOrNull returns null instead of throwing in case there is an
   error.

Proposal

Introduce APIs that address all the above.

Add ServerRequest.awaitReceiveNullable.

• This API "abstracts" away the coroutine-bridging exception handling from the user
• Throws only in case of serialization error
• Returns null in case user is expecting the body to be "missing"

Add ServerRequest.awaitReceive.

• This API "abstracts" away the coroutine-bridging exception handling from the user
• Throws only in case of serialization error

Both APIs introduce a "mindset" that only serialization-wise can something go wrong, in other words, user input.

Extensions:

Based on these functions, the user can create their own function for a general catch-all and return null pattern

runCatching { awaitReceiveOrNull }.getOrNull()

Deprecations/Migrations

Deprecate ServerRequest.awaitBodyOrNull which can be replaced with ServerRequest.awaitReceiveNullable, as
behavior wise they are pretty close.

Deprecate ServerRequest.awaitBody with not direct replacement, but promote the usage of ServerRequest.awaitReceive.

Closes gh-30926

[sdeleuze]

Thanks for providing this PR, but I don't think we should merge the proposed changes.

ServerRequest.awaitBody forces you to think about the coroutines-bridge exception handling, since it uses
kotlinx.coroutines.reactive.awaitSingle. Also, it is not documented that it might throw NoSuchElementException
and IllegalArgumentException for coroutines-related errors.

ServerRequest.awaitBody might throw IllegalArgumentException for two different reasons, for serialization-related
errors and for coroutines-related errors.

Please refer for this comment for related feedback about current APIs and related documentation and see #31189 follow-up issue.

ServerRequest.awaitBodyOrNull should never throw as the signature indicates. The convention established by the
stdlib mandate that an operation with the name xxxOrNull returns null instead of throwing in case there is an
error.

I don't think this is correct, Mono<T>.awaitSingleOrNull can (and should) throw/emit exception at codec level for example, the null value is used when for example there is no body, which is something different.

For those reasons, I decline this PR, but thanks for the discussion.

[GeorgePap-719]

Hey, thanks for the feedback!

I don't think this is correct, Mono.awaitSingleOrNull can (and should) throw/emit exception at codec level for example, the null value is used when for example there is no body, which is something different.

I agree with this statement.

I tried to raise the issue about the naming in the function itself awaitBodyOrNull. As i see it, i found the problem kinda similar to the old Reactive<T>.awaitSingleOrNull , where it is deprecated for similar reasons, the function was named xxxOrNull but was throwing an exception for a specific error.

So in the same manner, imo, i expect a function named awaitBodyOrNull to return null in case of an error, nevertheless if it is at codec level or coroutines-related error.

[sdeleuze]

As far as I know, Reactive<T>.awaitSingleOrNull was deprecated because there is no garantee to have a single element, and is replaced by Mono<T>.awaitSingleOrNull which keeps the same behavior is term of exception management. See #31127 related issue.

[GeorgePap-719]

I was checking the deprecation section here: Deprecation note.

because there is no guarantee to have a single element

But that might be also the case. I cannot argue here since I'm not expert in coroutines.

[sdeleuze]

Interesting, the deprecation may be partially misleading because as far as I can tell Publisher<T>.awaitFirstOrNull() has a documentation saying if this publisher has produced an error, throws the corresponding exception and that's the case for Mono<T>.awaitSingleOrNull as well. I will discuss that with the Kotlin team.

[GeorgePap-719]

I'm quite interested as well in what the kotlin team has to say.

Nevertheless, I will drop my two cents just for the sake of the discussion:

From my understanding most coroutines operators propagate (and throw) all exceptions
in the chain and, also might throw a CancelationException as well (but this is documented always I
think). They are not trying to convey these kinds of exceptions in the signatures but exceptions the
functions/operators themselves throw for their own semantic value.

Examples:

Publisher<T>.awaitSingle(): Propagates all exceptions and also throws CancellationException.
The KDoc states that the function itself throws two additional exceptions for its own semantic
value: NoSuchElementException if the publisher does not emit any value, and
IllegalArgumentException if the publisher emits more than one value.
Which can be translated that the function itself does some validations to ensure some semantics
(emits only one value or throws the corresponding exception).

Publisher<T>.awaitSingleOrNull(): Propagates all exceptions and also throws
CancellationException. The KDoc states that the function itself for its own semantics throws an
IllegalArgumentException if the publisher emits more than one value or returns null in case
there were non emitted. The problem here was that all xxxOrNull functions should return null
in case of an error for their own semantic value, however here it is not the case.

Mono<T>.awaitSingleOrNull(): Propagates all exceptions and also throws CancellationException.
The KDoc states that the function itself for its own semantic value returns null in case of an
error, specifically: "If the Mono completed without a value, null is returned".

Publisher<T>.awaitFirstOrNull(): To avoid being repetitive, this function follows the same pattern
as the other ones and in case of an error for its own semantic value returns null.

[sdeleuze]

We got confirmation from the Kotlin team the wording from the deprecation notes is misleading.

[GeorgePap-719]

I see, nevertheless, thanks for the discussion.