Skip to content

Commit 61bb712

Browse files
committed
Address PR feedback
1 parent aeb7d72 commit 61bb712

File tree

9 files changed

+47
-51
lines changed

9 files changed

+47
-51
lines changed

sentry-android-core/api/sentry-android-core.api

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -560,11 +560,6 @@ public class io/sentry/android/core/anr/AnrCulpritIdentifier {
560560
public static fun isSystemFrame (Ljava/lang/String;)Z
561561
}
562562

563-
public class io/sentry/android/core/anr/AnrException : java/lang/Exception {
564-
public fun <init> ()V
565-
public fun <init> (Ljava/lang/String;)V
566-
}
567-
568563
public class io/sentry/android/core/anr/AnrProfile {
569564
public final field endTimeMs J
570565
public final field stacks Ljava/util/List;

sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,11 +139,18 @@ void reportANR(
139139
message = "Background " + message;
140140
}
141141

142-
final ApplicationNotResponding error = new ApplicationNotResponding(message, anr.getThread());
142+
final @Nullable Thread thread = anr.getThread();
143+
final @NotNull ApplicationNotResponding error;
144+
if (thread == null) {
145+
error = new ApplicationNotResponding(message);
146+
} else {
147+
error = new ApplicationNotResponding(message, anr.getThread());
148+
}
149+
143150
final Mechanism mechanism = new Mechanism();
144151
mechanism.setType("ANR");
145152

146-
return new ExceptionMechanismException(mechanism, error, error.getThread(), true);
153+
return new ExceptionMechanismException(mechanism, error, thread, true);
147154
}
148155

149156
@TestOnly

sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoEventProcessor.java

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,14 @@
4141
import io.sentry.SpanContext;
4242
import io.sentry.android.core.anr.AggregatedStackTrace;
4343
import io.sentry.android.core.anr.AnrCulpritIdentifier;
44-
import io.sentry.android.core.anr.AnrException;
4544
import io.sentry.android.core.anr.AnrProfile;
4645
import io.sentry.android.core.anr.AnrProfileManager;
4746
import io.sentry.android.core.anr.AnrProfileRotationHelper;
4847
import io.sentry.android.core.anr.StackTraceConverter;
4948
import io.sentry.android.core.internal.util.CpuInfoUtils;
5049
import io.sentry.cache.PersistingOptionsObserver;
5150
import io.sentry.cache.PersistingScopeObserver;
51+
import io.sentry.exception.ExceptionMechanismException;
5252
import io.sentry.hints.AbnormalExit;
5353
import io.sentry.hints.Backfillable;
5454
import io.sentry.protocol.App;
@@ -871,25 +871,28 @@ private void applyAnrProfile(
871871
}
872872

873873
// Capture profile chunk
874-
final SentryId profilerId = captureAnrProfile(anrTimestamp, anrProfile);
874+
final @Nullable SentryId profilerId = captureAnrProfile(anrTimestamp, anrProfile);
875+
final @NotNull StackTraceElement[] stack = culprit.getStack();
875876

876-
final StackTraceElement[] stack = culprit.getStack();
877877
if (stack.length > 0) {
878878
final StackTraceElement stackTraceElement = stack[0];
879879
final String message =
880880
stackTraceElement.getClassName() + "." + stackTraceElement.getMethodName();
881-
final AnrException exception = new AnrException(message);
881+
final ApplicationNotResponding exception = new ApplicationNotResponding(message);
882882
exception.setStackTrace(stack);
883883

884+
final Mechanism mechanism = new Mechanism();
885+
mechanism.setType("ANR");
886+
final ExceptionMechanismException error =
887+
new ExceptionMechanismException(mechanism, exception, null, false);
888+
884889
final @NotNull List<SentryException> sentryException =
885890
sentryExceptionFactory.getSentryExceptions(exception);
886-
for (final @NotNull SentryException e : sentryException) {
887-
final Mechanism mechanism = new Mechanism();
888-
mechanism.setType("ANR");
889-
e.setMechanism(mechanism);
890-
}
891-
// Replaces the original ANR exception with the profile-derived one,
892-
// as we assume the profiling stacktrace is more detailed
891+
892+
// Replace the original ANR exception with the profile-derived one,
893+
// as we assume the profiling culprit identification is more valuable
894+
// the event threads are kept as-is, so the original main thread stacktrace is still
895+
// available
893896
event.setExceptions(sentryException);
894897

895898
if (profilerId != null) {

sentry-android-core/src/main/java/io/sentry/android/core/ApplicationNotResponding.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,20 @@
1717
final class ApplicationNotResponding extends RuntimeException {
1818
private static final long serialVersionUID = 252541144579117016L;
1919

20-
private final @NotNull Thread thread;
20+
private final @Nullable Thread thread;
21+
22+
ApplicationNotResponding(final @Nullable String message) {
23+
super(message);
24+
this.thread = null;
25+
}
2126

2227
ApplicationNotResponding(final @Nullable String message, final @NotNull Thread thread) {
2328
super(message);
2429
this.thread = Objects.requireNonNull(thread, "Thread must be provided.");
2530
setStackTrace(this.thread.getStackTrace());
2631
}
2732

28-
public @NotNull Thread getThread() {
33+
public @Nullable Thread getThread() {
2934
return thread;
3035
}
3136
}

sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrException.java

Lines changed: 0 additions & 15 deletions
This file was deleted.

sentry-android-core/src/main/java/io/sentry/android/core/anr/AnrProfilingIntegration.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,12 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions
5757
"SentryAndroidOptions is required");
5858
this.logger = options.getLogger();
5959

60-
if (options.getCacheDirPath() == null) {
61-
logger.log(SentryLevel.WARNING, "ANR Profiling is enabled but cacheDirPath is not set");
62-
return;
63-
}
64-
6560
if (((SentryAndroidOptions) options).isEnableAnrProfiling()) {
61+
if (options.getCacheDirPath() == null) {
62+
logger.log(SentryLevel.WARNING, "ANR Profiling is enabled but cacheDirPath is not set");
63+
return;
64+
}
65+
6666
addIntegrationToSdkVersion("AnrProfiling");
6767
AppState.getInstance().addAppStateListener(this);
6868
}

sentry-android-core/src/test/java/io/sentry/android/core/ANRWatchDogTest.kt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ class ANRWatchDogTest {
5858
} while (anr == null && waitCount++ < 100)
5959

6060
assertNotNull(anr)
61-
assertEquals(expectedState, anr!!.thread.state)
62-
assertEquals(stacktrace.className, anr!!.stackTrace[0].className)
61+
assertEquals(expectedState, anr.thread!!.state)
62+
assertEquals(stacktrace.className, anr.stackTrace[0].className)
6363
} finally {
6464
sut.interrupt()
6565
es.shutdown()
@@ -137,8 +137,8 @@ class ANRWatchDogTest {
137137
} while (anr == null && waitCount++ < 100)
138138

139139
assertNotNull(anr)
140-
assertEquals(expectedState, anr!!.thread.state)
141-
assertEquals(stacktrace.className, anr!!.stackTrace[0].className)
140+
assertEquals(expectedState, anr.thread!!.state)
141+
assertEquals(stacktrace.className, anr.stackTrace[0].className)
142142
} finally {
143143
sut.interrupt()
144144
es.shutdown()

sentry/src/main/java/io/sentry/SentryExceptionFactory.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,9 +174,9 @@ Deque<SentryException> extractExceptionQueueInternal(
174174
final List<SentryStackFrame> frames =
175175
sentryStackTraceFactory.getStackFrames(
176176
currentThrowable.getStackTrace(), includeSentryFrames);
177+
final @Nullable Long threadId = thread != null ? thread.getId() : null;
177178
SentryException exception =
178-
getSentryException(
179-
currentThrowable, exceptionMechanism, thread.getId(), frames, snapshot);
179+
getSentryException(currentThrowable, exceptionMechanism, threadId, frames, snapshot);
180180
exceptions.addFirst(exception);
181181

182182
if (exceptionMechanism.getType() == null) {

sentry/src/main/java/io/sentry/exception/ExceptionMechanismException.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import io.sentry.util.Objects;
55
import org.jetbrains.annotations.ApiStatus;
66
import org.jetbrains.annotations.NotNull;
7+
import org.jetbrains.annotations.Nullable;
78

89
/**
910
* A throwable decorator that holds an {@link io.sentry.protocol.Mechanism} related to the decorated
@@ -15,7 +16,7 @@ public final class ExceptionMechanismException extends RuntimeException {
1516

1617
private final @NotNull Mechanism exceptionMechanism;
1718
private final @NotNull Throwable throwable;
18-
private final @NotNull Thread thread;
19+
private final @Nullable Thread thread;
1920
private final boolean snapshot;
2021

2122
/**
@@ -29,11 +30,11 @@ public final class ExceptionMechanismException extends RuntimeException {
2930
public ExceptionMechanismException(
3031
final @NotNull Mechanism mechanism,
3132
final @NotNull Throwable throwable,
32-
final @NotNull Thread thread,
33+
final @Nullable Thread thread,
3334
final boolean snapshot) {
3435
exceptionMechanism = Objects.requireNonNull(mechanism, "Mechanism is required.");
3536
this.throwable = Objects.requireNonNull(throwable, "Throwable is required.");
36-
this.thread = Objects.requireNonNull(thread, "Thread is required.");
37+
this.thread = thread;
3738
this.snapshot = snapshot;
3839
}
3940

@@ -47,7 +48,7 @@ public ExceptionMechanismException(
4748
public ExceptionMechanismException(
4849
final @NotNull Mechanism mechanism,
4950
final @NotNull Throwable throwable,
50-
final @NotNull Thread thread) {
51+
final @Nullable Thread thread) {
5152
this(mechanism, throwable, thread, false);
5253
}
5354

@@ -74,7 +75,7 @@ public ExceptionMechanismException(
7475
*
7576
* @return the Thread
7677
*/
77-
public @NotNull Thread getThread() {
78+
public @Nullable Thread getThread() {
7879
return thread;
7980
}
8081

0 commit comments

Comments
 (0)