sentry-java

sentry-java copied to clipboard

All exceptions in Throwable.getSuppressed() should be reported and visible in Sentry. They might contain important information, that is not in the "main" exception.

Does Sentry itself have a mechanism for this? Or do they have to be reported separately?

This seems like a good idea.

At first glance I don't think we should add them to the main event stacktrace as that may be confusing and would affect issue grouping. I'm not even sure where in the existing exception chain they would be placed?

So I see a few options and I'd love some feedback:

• I'm wrong above and the suppressed exceptions should somehow be combined into the main exception.
• We could add some (basic, class and line number?) information to the event's extra data.
• We could add some (basic, class and line number?) breadcrumbs.
• They could go somewhere else on the event that already exists?
• We'd need a whole new place to store them on the event. (This one seems unlikely but maybe I'm wrong here too)

The main use-case for suppressed exceptions (if I'm not mistaken), is when you're processing some calls, that might produce a lot of errors, but you want to aggregate those errors and not kill the process itself. And at the end, you might want to throw a new exception, informing that part of the process failed and attach all the suppresed exceptions using Throwable.addSuppressed().

I'm wrong above and the suppressed exceptions should somehow be combined into the main exception.

IMHO they can be considered "cause" of the main exception, but because you cannot have an array of causes, they're "suppressed". IMHO there should be a separate "view" with informations about suppressed exceptions, they should not be rendered as part of stacktrace of the main exception.

We could add some (basic, class and line number?) information to the event's extra data.

"extra" makes sense as a temporary solution - because knowning about them is more important than having them printed nicely. It would be great, if Sentry itself could somehow render suppressed as separate exception blocks That would be awesome, but I suppose this request is unique to java and I'm not sure whether other languages have similar needs.

We could add some (basic, class and line number?) breadcrumbs. They could go somewhere else on the event that already exists?

I don't have opinion on this

We'd need a whole new place to store them on the event. (This one seems unlikely but maybe I'm wrong here too)

Yes, exactly! just like there is extra, there should be suppressed (or some other more generic name?) that Sentry itself could render properly.

Yeah, adding a new section in Sentry will (AFAIK) be a much larger process. In the short term I can definitely add them to extra or breadcrumbs on the SDK side. Maybe with an option to enable/disable if people really care.

I'll try to take a look at this soon, or as usual we're open to PRs. Thanks for the idea!

We're running through the same problem. We're using project reactor with webflux and for some Timeout/Null Pointer Exceptions. We don't get any stack trace but supressed exception contains the detailed info of where the error originated. Currently I'm manually adding it as a breadcrumb. Please let me know if there are any out of the box solution to serve the purpose.

io.sentry - 1.7.30
spring-boot-starter-webflux - 2.3.3

@vaibhavagnihotri you could create a chained exception instead of adding breadcrumbs, Sentry already supports chained exceptions thru the cause ctor https://docs.oracle.com/javase/7/docs/api/java/lang/Exception.html#Exception(java.lang.Throwable)

@marandaneto yes, but cause and suppressed have a different meaning.

And you can have always only one cause for each throwable, but each throwable can have a list of suppressed throwables.

yes, I meant, it's just a workaround, these exceptions would come thru and would be shown and have more info than just breadcrumbs.

@marandaneto Some times cause remains null but suppressed exception contains the assembly trace from producer.

Exception class - java.util.concurrent.TimeoutException

Exception.getMessage() - Did not observe any item or terminal signal within 1ms in 'Mono.flatMap ⇢ at org.springframework.web.reactive.function.client.DefaultWebClient$DefaultResponseSpec.bodyToMono(DefaultWebClient.java:471)' (and no fallback has been configured)

Exception.getCause() - null

Exception.getSuppressed()[0].getMessage() - Assembly trace from producer [reactor.core.publisher.MonoLift] : reactor.core.publisher.Mono.timeout ......Trace showing error origin

I encountered this today as well.

I think more users will be affected by this as they migrate away from RestTemplate to WebClient.

@marandaneto do you have any suggestions on how suppressed exceptions should be attached to SentryEvent?

One idea is to add them to exception queue in SentryExceptionFactory#extractExceptionQueue(..) with a mechanism of type "suppressed". What do you think?

yep, that's what I had on my mind too

be aware that we need an abstraction for Throwable.getSuppressed() because it does not work for all Android versions, only from API 19 on https://developer.android.com/reference/java/lang/Throwable.html#getSuppressed() so we have to check at runtime the OS version and either set the abstraction that returns nothing or not

Since this will require a lot of code to abstract away what's needed to work around the lack of API on Android, we decided to add this to the core sentry package once sentry-android-core min API level goes to 19 or higher.

For reference, API 16 (which we require for NDK) to 19 isn't a huge jump in terms of Android devices covered and we might be able to do that not too far in the future.

Until we are able to support suppressed exceptions in core, you can write custom EventProcessor that modifies reported exceptions to include not only causes but also suppressed ones. Unfortunately, some code has to be copy pasted, as it is package-private in Sentry SDK codebase. All needed classes are in this gist.

• SentryStackTraceFactory.java- is a 1:1 copy from Sentry Java SDK codebase
• SuppressedAwareSentryExceptionFactory.java - is a modified SentryExceptionFactory to extract suppressed exceptions
• SuppressedAwareEventProcessor.java - event processor that must be added to SentryOptions (done automatically in Spring Boot integration).

Hope to see this implemented. I somehow was under the impression that it was supported, until I noticed that it isn't.