Reactor stacktrace-mode is not configurable

[beltram]

Hi, according to #22105 I should be able to configure Reactor's stacktrace mode by configuration. After digging a bit I think it is not the case.
Spring's Webflux Handler does not take any property into account in DispatcherHandler here.
Moreover 'description' is always non-null which causes AssemblySnapshot to be always 'checkpointed' thus always appending a debug exception.
This is not the expected behaviour IMO.
Thanks in advance for having a look

[rstoyanchev]

You're saying stracktrace mode but pointing to a line where a checkpoint is inserted, so I'm not 100% sure I undertsand what you mean. For the stacktrace mode there is nothing specific in the Spring Framework. It is supported in Boot through a property and the devtools and in any it's just about calling Hooks.onOperatorDebug() on startup. Devtools seems like a perfect vehicle but if otherwise not much to it.

As for the checkpoint, it is always inserted because it is a very lightweight feature. That is my understanding based on discussions with the Reactor team. @simonbasle or @smaldini do you have any comments with regards to AssemblySnapshot appending a debug exception? Should checkpoint be inserted conditionally after all?

[simonbasle]

@rstoyanchev @beltram checkpoint(String) is indeed lightweight: it will create an instance of a stackless exception, the AssemblyLightSnapshot, that only bears the string as the message to be used in the "assembly backtrace".

[rstoyanchev]

Thanks for clarifying. @beltram it seems we're fine so I'll close but feel free to comment.

[beltram]

Thank you for the quick and thorough answer. I realize now I should have given more context 😄
I stumbled upon this once migrating from Spring Boot 2.1.X to 2.2.0. I am bound to a custom enterprise error model where basically each exception thrown is mapped to a json array in the http response. I had this test where, once zipping multiple exceptions like this :

@GetMapping
fun someErrors(): Mono<*> {
    val errorA = IllegalStateException("Error A").toMono<Any>()
    val errorB = IllegalStateException("Error B").toMono<Any>()
    return Mono.zipDelayError(errorA, errorB)
}

I was then catching them as a CompositeException in a ControllerAdvice like this Exceptions.unwrapMultiple(exception) then serializing to json.
According to my input I would expect 2 elements in my json array but after migrating to Spring Boot 2.2.0 got 3 instead.
In fact, what I meant by configurable is that I do not want this 'extra' (even though stackless) exception. I tried to get rid of it but OnAssemblyException isn't public so I couldn't leverage reflection to filter it out (had to rely upon hardcoded java classname instead). I also tried spring.reactor.debug-agent.enabled:false and spring.reactor.stacktrace-mode.enabled:falsewithout luck.

I have set up a reproducer for my issue available here.
If you could pinpoint what I'm missing here or provide a cleaner way to get rid of this exception, it would be highly appreciated. Thank you very much.

[simonbasle]

in reactor this stackless exception is the only mechanism through which we can display extra information (often the only meaningful information) about reactive parts. I guess with these settings you only get pinpointed traces from checkpoint(String) done by the Spring Framework, and you're looking for a way to suppress these. I don't think it is something we're going to do on reactor side though, due to the reason above. @rstoyanchev I guess adding an opt-out of these checkpoints in SPR is going to be complicated too?

[simonbasle]

reflection based on classname is one option, still. I'd argue that you WANT that information somewhere in your logs though...

[rstoyanchev]

@beltram thanks for the details and sample.

@simonbasle so the case is zipDelayError and then getting access to the errors for exception handling purposes. Arguably this should be possible to do without requiring checkpoints to be turned off or removed. The very premise of checkpoints is that they can remain embedded and always enhance the stack trace with information from critical paths. If however that gets in the way, we're creating incentive to turn them off and I'm just not sure we should have an on-off option.

Arguably there should be a way to get the actual errors only for exception handling purposes. One option would be for Exceptions.unwrapMultiple to filter out exceptions added by checkpoints, based on reflection of the classname. This would save applications from having to do that which is what they would be forced to do. Or provide two alternatives for unwrapMultiple, and let applications choose.

Either way the Javadoc of both zipDelayError and unwrapMultiple need to mention the impact of checkpoints and provide advice on how to deal with that. Or the Javadoc for checkpoint needs to mention its limitations but I would really hope that we go with the former.

[simonbasle]

@beltram @rstoyanchev would an Exceptions util method like isBacktrace help, in that case? That way one could optionally filter out such exceptions when calling unwrapMultiple?

json.put("errors",
    Exceptions.unwrapMultiple(zipDelayedError)
        .stream()
        .filter(e -> !Exceptions.isBacktrace(e))
);

It would probably not be perfect in the sense that the implementation would have to rely on reflection as well, due to package strucuture, but at least it would be a public API to check for checkpoint and Hooks.onOperatorDebug() related exceptions.

[rstoyanchev]

Could the message of the exception ("checkpoint") be used for the filtering? Either way given that this is an exceptional condition, the use of reflection is probably not a very big concern. This method would at least save the application from having to know the details of this.

I would also say that if such an option is provided, and an application can opt into this filtering, then there should be two variants of unwrapMultiple to avoid the need to even filter. Perhaps unwrapMultipe and unwrapMultipleWithoutCheckpoints(), or reversely unwrapMultipe (filters) and unwrapMultipleWithCheckpoints().

[simonbasle]

opened reactor/reactor-core#1953

[rstoyanchev]

Thanks @simonbasle, could you also add these as goals (as a reminder):

• Javadoc of operators that signal CompositeException to indicate how to unwrap, i.e. via Exceptions, and to include some mention of checkpoints depending on the outcome of the issue.
• Reference documentation about the checkpoint feature to explain the overlap with composite exceptions including similar advice so it can be found regardless of how you come across this.

[rstoyanchev]

I am closing this as the intent for WebFlux checkpoint is to always be on. Rather we're looking for a way to make it easy to separate actual errors from synthetic ones added by checkpoints.