Skip to content

Commit

Permalink
Fix: associate event with transaction when thrown exception is not a …
Browse files Browse the repository at this point in the history
…direct cause (#1463)

Fixes #1418
  • Loading branch information
maciejwalkowiak authored May 10, 2021
1 parent 5fa4b52 commit 6a7e4bc
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 5 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
# Unreleased

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

# 5.0.0-beta.2

* 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)
* Bump: AGP to 4.2.0 (#1460)
* Fix: NPE when adding Context Data with null values for log4j2 (#1465)


Breaking Changes:

* Remove: Settings.Secure.ANDROID_ID in favor of generated installationId (#1455)
Expand Down
5 changes: 5 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -1869,6 +1869,11 @@ public abstract interface class io/sentry/util/CollectionUtils$Predicate {
public abstract fun test (Ljava/lang/Object;)Z
}

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
13 changes: 9 additions & 4 deletions 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 Expand Up @@ -643,16 +645,19 @@ public void setSpanContext(
Objects.requireNonNull(throwable, "throwable is required");
Objects.requireNonNull(span, "span is required");
Objects.requireNonNull(transactionName, "transactionName is required");
// to match any cause, span context is always attached to the root cause of the exception
final Throwable rootCause = ExceptionUtils.findRootCause(throwable);
// the most inner span should be assigned to a throwable
if (!throwableToSpan.containsKey(throwable)) {
throwableToSpan.put(throwable, new Pair<>(span, transactionName));
if (!throwableToSpan.containsKey(rootCause)) {
throwableToSpan.put(rootCause, new Pair<>(span, transactionName));
}
}

@Nullable
SpanContext getSpanContext(final @NotNull Throwable throwable) {
Objects.requireNonNull(throwable, "throwable is required");
final Pair<ISpan, String> span = this.throwableToSpan.get(throwable);
final Throwable rootCause = ExceptionUtils.findRootCause(throwable);
final Pair<ISpan, String> span = this.throwableToSpan.get(rootCause);
if (span != null) {
return span.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;
}
}
33 changes: 33 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,39 @@ 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`() {
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 non-root cause has been previously attached with span context, sets span context to the event`() {
val (sut, mockClient) = getEnabledHub()
val rootCause = RuntimeException()
val exceptionAssignedToSpan = RuntimeException(rootCause)
val span = mock<Span>()
whenever(span.spanContext).thenReturn(SpanContext("op"))
sut.setSpanContext(exceptionAssignedToSpan, span, "tx-name")

val event = SentryEvent(RuntimeException(exceptionAssignedToSpan))

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))
}
}

0 comments on commit 6a7e4bc

Please sign in to comment.