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.

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

Fix: associate event with transaction when thrown exception is not a direct cause #1463

Merged
merged 6 commits into from
May 10, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

* Fix: sentry-android-timber package sets sentry.java.android.timber as SDK name (#1456)
* Fix: When AppLifecycleIntegration is closed, it should remove observer using UI thread (#1459)
* Fix: associate event with transaction when thrown exception is not a direct cause (#1463)

Breaking Changes:

Expand Down
5 changes: 5 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -1864,6 +1864,11 @@ public final class io/sentry/util/CollectionUtils {
public static fun size (Ljava/lang/Iterable;)I
}

public final class io/sentry/util/ExceptionUtils {
public fun <init> ()V
public static fun findRootCause (Ljava/lang/Throwable;)Ljava/lang/Throwable;
}

public final class io/sentry/util/LogUtils {
public fun <init> ()V
public static fun logIfNotFlushable (Lio/sentry/ILogger;Ljava/lang/Object;)V
Expand Down
4 changes: 3 additions & 1 deletion sentry/src/main/java/io/sentry/Hub.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import io.sentry.protocol.SentryId;
import io.sentry.protocol.SentryTransaction;
import io.sentry.protocol.User;
import io.sentry.util.ExceptionUtils;
import io.sentry.util.Objects;
import io.sentry.util.Pair;
import java.io.Closeable;
Expand Down Expand Up @@ -175,7 +176,8 @@ public SentryId captureEnvelope(

private void assignTraceContext(final @NotNull SentryEvent event) {
if (event.getThrowable() != null) {
final Pair<ISpan, String> pair = throwableToSpan.get(event.getThrowable());
final Pair<ISpan, String> pair =
throwableToSpan.get(ExceptionUtils.findRootCause(event.getThrowable()));
if (pair != null) {
if (event.getContexts().getTrace() == null) {
event.getContexts().setTrace(pair.getFirst().getSpanContext());
Expand Down
23 changes: 23 additions & 0 deletions sentry/src/main/java/io/sentry/util/ExceptionUtils.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package io.sentry.util;

import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;

@ApiStatus.Internal
public final class ExceptionUtils {

/**
* Returns exception root cause or the exception itself if there are no causes
*
* @param throwable - the throwable
* @return the root cause
*/
public static @NotNull Throwable findRootCause(final @NotNull Throwable throwable) {
Objects.requireNonNull(throwable, "throwable cannot be null");
Throwable rootCause = throwable;
while (rootCause.getCause() != null && rootCause.getCause() != rootCause) {
rootCause = rootCause.getCause();
}
return rootCause;
}
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
}
16 changes: 16 additions & 0 deletions sentry/src/test/java/io/sentry/HubTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,22 @@ class HubTest {
verify(mockClient).captureEvent(eq(event), any(), eq(hint))
}

@Test
fun `when captureEvent is called and event has exception which root cause has been previously attached with span context, sets span context to the event`() {
maciejwalkowiak marked this conversation as resolved.
Show resolved Hide resolved
val (sut, mockClient) = getEnabledHub()
val rootCause = RuntimeException()
val span = mock<Span>()
whenever(span.spanContext).thenReturn(SpanContext("op"))
sut.setSpanContext(rootCause, span, "tx-name")

val event = SentryEvent(RuntimeException(rootCause))

val hint = { }
sut.captureEvent(event, hint)
assertEquals(span.spanContext, event.contexts.trace)
verify(mockClient).captureEvent(eq(event), any(), eq(hint))
}

@Test
fun `when captureEvent is called and event has exception which has been previously attached with span context and trace context already set, does not set new span context to the event`() {
val (sut, mockClient) = getEnabledHub()
Expand Down
22 changes: 22 additions & 0 deletions sentry/src/test/java/io/sentry/util/ExceptionUtilsTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package io.sentry.util

import java.lang.RuntimeException
import kotlin.test.Test
import kotlin.test.assertEquals

class ExceptionUtilsTest {

@Test
fun `returns same exception when there is no cause`() {
val ex = RuntimeException()
assertEquals(ex, ExceptionUtils.findRootCause(ex))
}

@Test
fun `returns first cause when there are multiple causes`() {
val rootCause = RuntimeException()
val cause = RuntimeException(rootCause)
val ex = RuntimeException(cause)
assertEquals(rootCause, ExceptionUtils.findRootCause(ex))
}
}