diff --git a/CHANGELOG.md b/CHANGELOG.md index f3835bc43c..48568805a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ - Avoid forking `rootScopes` for Reactor if current thread has `NoOpScopes` ([#4793](https://github.com/getsentry/sentry-java/pull/4793)) - This reduces the SDKs overhead by avoiding unnecessary scope forks +### Fixes + +- Fix missing thread stacks for ANRv1 events ([#4918](https://github.com/getsentry/sentry-java/pull/4918)) + ## 8.27.1 ### Fixes diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 8f9220eebe..5db3028983 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -3865,7 +3865,7 @@ public final class io/sentry/SentryStackTraceFactory { } public final class io/sentry/SentryThreadFactory { - public fun (Lio/sentry/SentryStackTraceFactory;Lio/sentry/SentryOptions;)V + public fun (Lio/sentry/SentryStackTraceFactory;)V } public final class io/sentry/SentryTraceHeader { diff --git a/sentry/src/main/java/io/sentry/MainEventProcessor.java b/sentry/src/main/java/io/sentry/MainEventProcessor.java index ddf7b3828e..5ad19c79e3 100644 --- a/sentry/src/main/java/io/sentry/MainEventProcessor.java +++ b/sentry/src/main/java/io/sentry/MainEventProcessor.java @@ -35,7 +35,7 @@ public MainEventProcessor(final @NotNull SentryOptions options) { new SentryStackTraceFactory(this.options); sentryExceptionFactory = new SentryExceptionFactory(sentryStackTraceFactory); - sentryThreadFactory = new SentryThreadFactory(sentryStackTraceFactory, this.options); + sentryThreadFactory = new SentryThreadFactory(sentryStackTraceFactory); } MainEventProcessor( @@ -255,17 +255,20 @@ private void setThreads(final @NotNull SentryEvent event, final @NotNull Hint hi if (options.isAttachThreads() || HintUtils.hasType(hint, AbnormalExit.class)) { final Object sentrySdkHint = HintUtils.getSentrySdkHint(hint); boolean ignoreCurrentThread = false; + boolean attachStacktrace = options.isAttachStacktrace(); if (sentrySdkHint instanceof AbnormalExit) { ignoreCurrentThread = ((AbnormalExit) sentrySdkHint).ignoreCurrentThread(); + attachStacktrace = true; } event.setThreads( - sentryThreadFactory.getCurrentThreads(mechanismThreadIds, ignoreCurrentThread)); + sentryThreadFactory.getCurrentThreads( + mechanismThreadIds, ignoreCurrentThread, attachStacktrace)); } else if (options.isAttachStacktrace() && (eventExceptions == null || eventExceptions.isEmpty()) && !isCachedHint(hint)) { // when attachStacktrace is enabled, we attach only the current thread and its stack traces, // if there are no exceptions, exceptions have its own stack traces. - event.setThreads(sentryThreadFactory.getCurrentThread()); + event.setThreads(sentryThreadFactory.getCurrentThread(options.isAttachStacktrace())); } } } diff --git a/sentry/src/main/java/io/sentry/SentryThreadFactory.java b/sentry/src/main/java/io/sentry/SentryThreadFactory.java index 8af5f0aa0c..0b8f258499 100644 --- a/sentry/src/main/java/io/sentry/SentryThreadFactory.java +++ b/sentry/src/main/java/io/sentry/SentryThreadFactory.java @@ -20,21 +20,14 @@ public final class SentryThreadFactory { /** the SentryStackTraceFactory */ private final @NotNull SentryStackTraceFactory sentryStackTraceFactory; - /** the SentryOptions. */ - private final @NotNull SentryOptions options; - /** * ctor SentryThreadFactory that takes a SentryStackTraceFactory * * @param sentryStackTraceFactory the SentryStackTraceFactory - * @param options the SentryOptions */ - public SentryThreadFactory( - final @NotNull SentryStackTraceFactory sentryStackTraceFactory, - final @NotNull SentryOptions options) { + public SentryThreadFactory(final @NotNull SentryStackTraceFactory sentryStackTraceFactory) { this.sentryStackTraceFactory = Objects.requireNonNull(sentryStackTraceFactory, "The SentryStackTraceFactory is required."); - this.options = Objects.requireNonNull(options, "The SentryOptions is required"); } /** @@ -44,12 +37,12 @@ public SentryThreadFactory( * @return a list of SentryThread with a single item or null */ @Nullable - List getCurrentThread() { + List getCurrentThread(final boolean attachStackTrace) { final Map threads = new HashMap<>(); final Thread currentThread = Thread.currentThread(); threads.put(currentThread, currentThread.getStackTrace()); - return getCurrentThreads(threads, null, false); + return getCurrentThreads(threads, null, false, attachStackTrace); } /** @@ -63,13 +56,18 @@ List getCurrentThread() { */ @Nullable List getCurrentThreads( - final @Nullable List mechanismThreadIds, final boolean ignoreCurrentThread) { - return getCurrentThreads(Thread.getAllStackTraces(), mechanismThreadIds, ignoreCurrentThread); + final @Nullable List mechanismThreadIds, + final boolean ignoreCurrentThread, + final boolean attachStackTrace) { + return getCurrentThreads( + Thread.getAllStackTraces(), mechanismThreadIds, ignoreCurrentThread, attachStackTrace); } @Nullable - List getCurrentThreads(final @Nullable List mechanismThreadIds) { - return getCurrentThreads(Thread.getAllStackTraces(), mechanismThreadIds, false); + List getCurrentThreads( + final @Nullable List mechanismThreadIds, final boolean attachStackTrace) { + return getCurrentThreads( + Thread.getAllStackTraces(), mechanismThreadIds, false, attachStackTrace); } /** @@ -87,7 +85,8 @@ List getCurrentThreads(final @Nullable List mechanismThreadI List getCurrentThreads( final @NotNull Map threads, final @Nullable List mechanismThreadIds, - final boolean ignoreCurrentThread) { + final boolean ignoreCurrentThread, + final boolean attachStackTrace) { List result = null; final Thread currentThread = Thread.currentThread(); @@ -109,7 +108,7 @@ List getCurrentThreads( && mechanismThreadIds.contains(thread.getId()) && !ignoreCurrentThread); - result.add(getSentryThread(crashed, item.getValue(), item.getKey())); + result.add(getSentryThread(crashed, item.getValue(), item.getKey(), attachStackTrace)); } } @@ -127,7 +126,8 @@ List getCurrentThreads( private @NotNull SentryThread getSentryThread( final boolean crashed, final @NotNull StackTraceElement[] stackFramesElements, - final @NotNull Thread thread) { + final @NotNull Thread thread, + final boolean attachStacktrace) { final SentryThread sentryThread = new SentryThread(); sentryThread.setName(thread.getName()); @@ -137,15 +137,17 @@ List getCurrentThreads( sentryThread.setState(thread.getState().name()); sentryThread.setCrashed(crashed); - final List frames = - sentryStackTraceFactory.getStackFrames(stackFramesElements, false); + if (attachStacktrace) { + final List frames = + sentryStackTraceFactory.getStackFrames(stackFramesElements, false); - if (options.isAttachStacktrace() && frames != null && !frames.isEmpty()) { - final SentryStackTrace sentryStackTrace = new SentryStackTrace(frames); - // threads are always gotten either via Thread.currentThread() or Thread.getAllStackTraces() - sentryStackTrace.setSnapshot(true); + if (frames != null && !frames.isEmpty()) { + final SentryStackTrace sentryStackTrace = new SentryStackTrace(frames); + // threads are always gotten either via Thread.currentThread() or Thread.getAllStackTraces() + sentryStackTrace.setSnapshot(true); - sentryThread.setStacktrace(sentryStackTrace); + sentryThread.setStacktrace(sentryStackTrace); + } } return sentryThread; diff --git a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt index 33393be2a2..6fc2c8c243 100644 --- a/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/MainEventProcessorTest.kt @@ -249,7 +249,7 @@ class MainEventProcessorTest { } @Test - fun `when attach threads is disabled, but the hint is Abnormal, still sets threads`() { + fun `when attach threads is disabled, but the hint is Abnormal, still sets threads and stacktraces`() { val sut = fixture.getSut(attachThreads = false, attachStackTrace = false) var event = SentryEvent(RuntimeException("error")) @@ -258,6 +258,20 @@ class MainEventProcessorTest { assertNotNull(event.threads) assertEquals(1, event.threads!!.count { it.isCrashed == true }) + assertNotNull(event.threads!!.first().stacktrace) + } + + @Test + fun `when attach threads is enabled, but stacktraces is not its reflected`() { + val sut = fixture.getSut(attachThreads = true, attachStackTrace = false) + + var event = SentryEvent(RuntimeException("error")) + val hint = Hint() + event = sut.process(event, hint) + + assertNotNull(event.threads) + assertEquals(1, event.threads!!.count { it.isCrashed == true }) + assertNull(event.threads!!.first().stacktrace) } @Test diff --git a/sentry/src/test/java/io/sentry/SentryThreadFactoryTest.kt b/sentry/src/test/java/io/sentry/SentryThreadFactoryTest.kt index 76a4743c79..c2eda19efc 100644 --- a/sentry/src/test/java/io/sentry/SentryThreadFactoryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryThreadFactoryTest.kt @@ -10,13 +10,9 @@ import kotlin.test.assertTrue class SentryThreadFactoryTest { class Fixture { - internal fun getSut(attachStacktrace: Boolean = true) = + internal fun getSut() = SentryThreadFactory( - SentryStackTraceFactory(SentryOptions().apply { addInAppExclude("io.sentry") }), - with(SentryOptions()) { - isAttachStacktrace = attachStacktrace - this - }, + SentryStackTraceFactory(SentryOptions().apply { addInAppExclude("io.sentry") }) ) } @@ -25,26 +21,29 @@ class SentryThreadFactoryTest { @Test fun `when getCurrentThreads is called, not empty result`() { val sut = fixture.getSut() - val threads = sut.getCurrentThreads(null) + val threads = sut.getCurrentThreads(null, false) assertNotEquals(0, threads!!.count()) } @Test fun `when currentThreads is called, current thread is marked crashed`() { val sut = fixture.getSut() - assertEquals(1, sut.getCurrentThreads(null)!!.filter { it.isCrashed == true }.count()) + assertEquals(1, sut.getCurrentThreads(null, false)!!.filter { it.isCrashed == true }.count()) } @Test fun `when currentThreads is called with ignoreCurrentThread, current thread is not marked crashed`() { val sut = fixture.getSut() - assertEquals(0, sut.getCurrentThreads(null, true)!!.filter { it.isCrashed == true }.count()) + assertEquals( + 0, + sut.getCurrentThreads(null, true, false)!!.filter { it.isCrashed == true }.count(), + ) } @Test fun `when currentThreads is called, thread state is captured`() { val sut = fixture.getSut() - assertTrue(sut.getCurrentThreads(null)!!.all { it.state != null }) + assertTrue(sut.getCurrentThreads(null, false)!!.all { it.state != null }) } @Test @@ -52,7 +51,7 @@ class SentryThreadFactoryTest { val sut = fixture.getSut() assertTrue( sut - .getCurrentThreads(null)!! + .getCurrentThreads(null, true)!! .filter { it.stacktrace != null } .any { it.stacktrace!!.frames!!.count() > 0 } ) @@ -63,7 +62,7 @@ class SentryThreadFactoryTest { val sut = fixture.getSut() assertTrue( sut - .getCurrentThreads(null)!! + .getCurrentThreads(null, true)!! .filter { it.stacktrace != null } .any { it.stacktrace!!.snapshot == true } ) @@ -71,10 +70,10 @@ class SentryThreadFactoryTest { @Test fun `when currentThreads and attachStacktrace is disabled, stack frames are not captured`() { - val sut = fixture.getSut(false) + val sut = fixture.getSut() assertFalse( sut - .getCurrentThreads(null)!! + .getCurrentThreads(null, false)!! .filter { it.stacktrace != null } .any { it.stacktrace!!.frames!!.count() > 0 } ) @@ -87,7 +86,7 @@ class SentryThreadFactoryTest { val currentThread = Thread.currentThread() stackTraces.remove(currentThread) - val threads = sut.getCurrentThreads(stackTraces, null, false) + val threads = sut.getCurrentThreads(stackTraces, null, false, false) assertNotNull(threads!!.firstOrNull { it.id == currentThread.id }) } @@ -95,7 +94,7 @@ class SentryThreadFactoryTest { @Test fun `When passing empty param to getCurrentThreads, returns null`() { val sut = fixture.getSut() - val threads = sut.getCurrentThreads(mapOf(), null, false) + val threads = sut.getCurrentThreads(mapOf(), null, false, false) assertNull(threads) } @@ -108,7 +107,7 @@ class SentryThreadFactoryTest { val stacktraces = emptyArray() val threadList = mutableMapOf(thread to stacktraces) - val threads = sut.getCurrentThreads(threadList, threadIds, false) + val threads = sut.getCurrentThreads(threadList, threadIds, false, false) assertNotNull(threads!!.firstOrNull { it.isCrashed == true }) } @@ -116,7 +115,7 @@ class SentryThreadFactoryTest { @Test fun `when getCurrentThread is called, returns current thread`() { val sut = fixture.getSut() - val threads = sut.currentThread + val threads = sut.getCurrentThread(false) assertEquals(1, threads!!.count()) } }