Repeated checkpoints added when using retry

[osi]

Expected Behavior

There should be a single checkpoint

Actual Behavior

There are multiple checkpoints.

Steps to Reproduce

@Test
void reproCase() {
        StepVerifier.create(Flux.error(() -> new RuntimeException("rawr"))
                                .cache()
                                .checkpoint("nom")
                                .retryWhen(Retry.max(3))
                    )
                    .expectErrorSatisfies(t -> assertThat(Arrays.asList(t.getCause().getSuppressed()))
                        .hasSize(1)
                        .first(THROWABLE)
                        .hasMessage("\n" +
                                    "Error has been observed at the following site(s):\n" +
                                    "\t|_ checkpoint ⇢ nom\n" +
                                    "Stack trace:"))
                    .verify(Duration.ofSeconds(10));
}

It is generating multiple checkpoints. Only a single one is expected

	|_ checkpoint ⇢ nom
	|_____ checkpoint ⇢ nom
	|_________ checkpoint ⇢ nom
	|_____________ checkpoint ⇢ nom

Your Environment

• Reactor version(s) used: 3.4.9

I observed this in our full application where the checkpointed stream is ultimately a r2dbc-postgresql notification stream prefixed by a query using mergeSequential. There was a problem connecting to the DB, and that was being retried. The same behavior was observed, and this is a distillation of that. As I don't have a full reproduction with our entire stack, my hope is that fixing this also fixes what we observed.

[smaldini]

In the FluxAssembly implementation, the operator is checking for the presence of checkpoint exception tuple. Problem is that i keeps being incremented and contains will always fail the check. I can't remember the rationale behind the increment but if the checkpoint is exactly identical (same parent, description) there shouldnt be a need for increment as part of the tuple. Without i, the tuple would match the correct hashcode.

[simonbasle]

without much tests nor comments nor git commit comments to provide more context, I have trouble understanding the rationale behind this i either :(

removing it doesn't seem to fail any test.

On a side note, the additional entries to the backtrace are only there because the "rawr" exception is effectively passed to checkpoint's onError multiple time due to the cache().
It would have been equivalent to do Flux.onError(new RuntimeException("rawr")).

[simonbasle]

actually, this one test fails:

reactor-core/reactor-core/src/test/java/reactor/core/publisher/HooksTest.java

Line 738 in 9996658

public void testMultiReceiver() throws Exception {

are we losing any information by removing these extra-indented "FluxPublish -> at ..." entries ?

[simonbasle]

I have a change that better cater to multiple paths in the Error has been observed at the following site(s) section, but I wonder how disruptive it it to change the representation of the tracebacks 🤔

To the best of my knowledge I don't know of a tool that would parse the traceback, but you never know.

Any insight to provide @osi @smaldini ?

(let's also cc @bsideup who worked on the agent and on tracebacks in the past, he might also have insights)

[simonbasle]

to clarify, from #2780 the current proposal looks like this:

Assembly trace from producer [reactor.core.publisher.MonoDefer] :
	reactor.core.publisher.Mono.defer(Mono.java:213)
	reactor.core.publisher.HooksTest.testMultiReceiver(HooksTest.java:747)
Error has been observed at the following site(s):
	*____Mono.defer ⇢ at reactor.core.publisher.HooksTest.testMultiReceiver(HooksTest.java:747)
	*_____Flux.then ⇢ at reactor.core.publisher.HooksTest.testMultiReceiver(HooksTest.java:747)
	|_    Mono.flux ⇢ at reactor.core.publisher.HooksTest.testMultiReceiver(HooksTest.java:749)
	|_ Flux.publish ⇢ at reactor.core.publisher.HooksTest.testMultiReceiver(HooksTest.java:749) (observed 2 times)
	|_     Flux.map ⇢ at reactor.core.publisher.HooksTest.testMultiReceiver(HooksTest.java:751)
	*_____Flux.then ⇢ at reactor.core.publisher.HooksTest.testMultiReceiver(HooksTest.java:747)
	|_    Mono.flux ⇢ at reactor.core.publisher.HooksTest.testMultiReceiver(HooksTest.java:749)
	|_ Flux.publish ⇢ at reactor.core.publisher.HooksTest.testMultiReceiver(HooksTest.java:749) (observed 2 times)
	|_  Flux.filter ⇢ at reactor.core.publisher.HooksTest.testMultiReceiver(HooksTest.java:761)

[osi]

@simonbasle I'm not parsing tracebacks, and don't have awareness of any tooling around it.

The new form looks great!!

[simonbasle]

this will be re-targeted to 3.4.x only