Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

A few Mono operators can lead to bad signal combination of onNext+onError #1853

Closed
simonbasle opened this issue Aug 19, 2019 · 0 comments
Closed

A few Mono operators can lead to bad signal combination of onNext+onError #1853

simonbasle opened this issue Aug 19, 2019 · 0 comments
Labels
type/bug A general bug
Milestone
3.3.0.RC1

Comments

@simonbasle
Copy link
Member

Expected Behavior

Mono authorized sequences of events are onNext+onComplete, onComplete or onError (also, technically, infinite Monos with onNext or no signal at all).

onNext should never be followed by onError.

Actual Behavior

Mono.just("foo").doOnTerminate(() -> { throw new IllegalStateException(); }) results in the undesirable signals sequence.

Mono.using with a completeHandler that throws similarly produces bad signals.

Possible Solution

  • for Mono#doOnComplete, rework into an alias to doOnSuccessOrError that ignores both inputs, make it prominent in the javadoc (ie. onNext is discarded rather than propagated if the callback fails, and thus the callback is executed before actual.onNext rather than between onNext and onComplete)
  • for Mono#using, like usingWhen it would delay the emission of onNext to until the onComplete handler has finished.

Relates to #1832
@simonbasle simonbasle added this to the 3.2.12.RELEASE milestone Aug 19, 2019
@simonbasle simonbasle mentioned this issue Aug 19, 2019
Closed
simonbasle added a commit that referenced this issue Aug 21, 2019
@simonbasle
see #1853 Prevent Mono#using to trigger onNext+onError if cleanup fails
2a99b3f 
This commit slightly changes the behavior of Mono#using so that, in
eager mode, the cleanup is applied inside `onNext`. Thus it is applied
before the value is passed to downstream (which at this point is no
longer in the protected scope of the `sourceSupplier`).

This is done to prevent the case where the value would be passed down
yet the cleanup throws, resulting in an invalid sequence of onNext then
onError signals.
@simonbasle simonbasle modified the milestones: 3.2.12.RELEASE, 3.3.0.RC1 Aug 22, 2019
simonbasle added a commit that referenced this issue Aug 22, 2019
@simonbasle
see #1853 doOnTerminate for Mono is same as doOnSuccess
4d059fd
simonbasle added a commit that referenced this issue Aug 22, 2019
@simonbasle
see #1853 Prevent Mono#using to trigger onNext+onError if cleanup fails
98f0f0e 
This commit slightly changes the behavior of Mono#using so that, in
eager mode, the cleanup is applied inside `onNext`. Thus it is applied
before the value is passed to downstream (which at this point is no
longer in the protected scope of the `sourceSupplier`).

This is done to prevent the case where the value would be passed down
yet the cleanup throws, resulting in an invalid sequence of onNext then
onError signals.
simonbasle added a commit that referenced this issue Aug 22, 2019
@simonbasle
see #1853 doOnTerminate for Mono is same as doOnSuccess
f9f686a
simonbasle added a commit that referenced this issue Aug 22, 2019
@simonbasle
fix #1853 Prevent Mono#using to trigger onNext+onError if cleanup fails
7d4522a 
This commit slightly changes the behavior of Mono#using so that, in
eager mode, the cleanup is applied inside `onNext`. Thus it is applied
before the value is passed to downstream (which at this point is no
longer in the protected scope of the `sourceSupplier`).

This is done to prevent the case where the value would be passed down
yet the cleanup throws, resulting in an invalid sequence of onNext then
onError signals.
simonbasle added a commit that referenced this issue Aug 22, 2019
@simonbasle
fix #1752 Make doOnTerminate for Mono same as doOnSuccess
6436811 
This means that the handler is invoked during processing of the `onNext`
signal.

Relates-to: #1853
simonbasle added a commit that referenced this issue Aug 27, 2019
@simonbasle
fix #1853 Prevent Mono#using to trigger onNext+onError if cleanup fails
fa86abe 
This commit slightly changes the behavior of Mono#using so that, in
eager mode, the cleanup is applied inside `onNext`. Thus it is applied
before the value is passed to downstream (which at this point is no
longer in the protected scope of the `sourceSupplier`).

This is done to prevent the case where the value would be passed down
yet the cleanup throws, resulting in an invalid sequence of onNext then
onError signals.
simonbasle added a commit that referenced this issue Aug 27, 2019
@simonbasle
fix #1752 Make doOnTerminate for Mono same as doOnSuccess
8d29500 
This means that the handler is invoked during processing of the `onNext`
signal.

This commit also slightly rework MonoPeekTerminal to make the onSuccess
and onError handlers canonical, instead of the (successOrError) one.

Finally, it makes `doOnError` directly use MonoPeekTerminal instead of
MonoPeek, and modifies the marble diagrams (adding a convention for the
null value / empty set).

Relates-to: #1853
simonbasle added a commit that referenced this issue Aug 27, 2019
@simonbasle
fix #1752 Make doOnTerminate for Mono same as doOnSuccess
a93c96c 
This means that the handler is invoked during processing of the `onNext`
signal.

This commit also slightly rework MonoPeekTerminal to make the onSuccess
and onError handlers canonical, instead of the (successOrError) one.

Finally, it makes `doOnError` directly use MonoPeekTerminal instead of
MonoPeek, and modifies the marble diagrams (adding a convention for the
null value / empty set).

Relates-to: #1853
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type/bug A general bug
Projects
None yet
Milestone
3.3.0.RC1
Development

No branches or pull requests
1 participant
@simonbasle