Skip to content

Commit bb4a325

Browse files
markushimarkushi
authored
Merge 00442b2 into b03edbb 
2 parents b03edbb + 00442b2 commit bb4a325

File tree

6 files changed

+66
-47
lines changed

6 files changed

+66
-47
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
### Improvements
1616

17+
- Decouple SentryThreadFactory from SentryOptions ([#4918](https://github.com/getsentry/sentry-java/pull/4918))
1718
- Do not send manual log origin ([#4897](https://github.com/getsentry/sentry-java/pull/4897))
1819

1920
## 8.26.0

‎sentry/api/sentry.api‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3864,7 +3864,7 @@ public final class io/sentry/SentryStackTraceFactory {
38643864
}
38653865

38663866
public final class io/sentry/SentryThreadFactory {
3867-
public fun <init> (Lio/sentry/SentryStackTraceFactory;Lio/sentry/SentryOptions;)V
3867+
public fun <init> (Lio/sentry/SentryStackTraceFactory;)V
38683868
}
38693869

38703870
public final class io/sentry/SentryTraceHeader {

‎sentry/src/main/java/io/sentry/MainEventProcessor.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public MainEventProcessor(final @NotNull SentryOptions options) {
3535
new SentryStackTraceFactory(this.options);
3636

3737
sentryExceptionFactory = new SentryExceptionFactory(sentryStackTraceFactory);
38-
sentryThreadFactory = new SentryThreadFactory(sentryStackTraceFactory, this.options);
38+
sentryThreadFactory = new SentryThreadFactory(sentryStackTraceFactory);
3939
}
4040

4141
MainEventProcessor(
@@ -255,17 +255,20 @@ private void setThreads(final @NotNull SentryEvent event, final @NotNull Hint hi
255255
if (options.isAttachThreads() || HintUtils.hasType(hint, AbnormalExit.class)) {
256256
final Object sentrySdkHint = HintUtils.getSentrySdkHint(hint);
257257
boolean ignoreCurrentThread = false;
258+
boolean attachStacktrace = options.isAttachStacktrace();
258259
if (sentrySdkHint instanceof AbnormalExit) {
259260
ignoreCurrentThread = ((AbnormalExit) sentrySdkHint).ignoreCurrentThread();
261+
attachStacktrace = true;
260262
}
261263
event.setThreads(
262-
sentryThreadFactory.getCurrentThreads(mechanismThreadIds, ignoreCurrentThread));
264+
sentryThreadFactory.getCurrentThreads(
265+
mechanismThreadIds, ignoreCurrentThread, attachStacktrace));
263266
} else if (options.isAttachStacktrace()
264267
&& (eventExceptions == null || eventExceptions.isEmpty())
265268
&& !isCachedHint(hint)) {
266269
// when attachStacktrace is enabled, we attach only the current thread and its stack traces,
267270
// if there are no exceptions, exceptions have its own stack traces.
268-
event.setThreads(sentryThreadFactory.getCurrentThread());
271+
event.setThreads(sentryThreadFactory.getCurrentThread(options.isAttachStacktrace()));
269272
}
270273
}
271274
}

‎sentry/src/main/java/io/sentry/SentryThreadFactory.java‎

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,21 +20,14 @@ public final class SentryThreadFactory {
2020
/** the SentryStackTraceFactory */
2121
private final @NotNull SentryStackTraceFactory sentryStackTraceFactory;
2222

23-
/** the SentryOptions. */
24-
private final @NotNull SentryOptions options;
25-
2623
/**
2724
* ctor SentryThreadFactory that takes a SentryStackTraceFactory
2825
*
2926
* @param sentryStackTraceFactory the SentryStackTraceFactory
30-
* @param options the SentryOptions
3127
*/
32-
public SentryThreadFactory(
33-
final @NotNull SentryStackTraceFactory sentryStackTraceFactory,
34-
final @NotNull SentryOptions options) {
28+
public SentryThreadFactory(final @NotNull SentryStackTraceFactory sentryStackTraceFactory) {
3529
this.sentryStackTraceFactory =
3630
Objects.requireNonNull(sentryStackTraceFactory, "The SentryStackTraceFactory is required.");
37-
this.options = Objects.requireNonNull(options, "The SentryOptions is required");
3831
}
3932

4033
/**
@@ -44,12 +37,12 @@ public SentryThreadFactory(
4437
* @return a list of SentryThread with a single item or null
4538
*/
4639
@Nullable
47-
List<SentryThread> getCurrentThread() {
40+
List<SentryThread> getCurrentThread(final boolean attachStackTrace) {
4841
final Map<Thread, StackTraceElement[]> threads = new HashMap<>();
4942
final Thread currentThread = Thread.currentThread();
5043
threads.put(currentThread, currentThread.getStackTrace());
5144

52-
return getCurrentThreads(threads, null, false);
45+
return getCurrentThreads(threads, null, false, attachStackTrace);
5346
}
5447

5548
/**
@@ -63,13 +56,18 @@ List<SentryThread> getCurrentThread() {
6356
*/
6457
@Nullable
6558
List<SentryThread> getCurrentThreads(
66-
final @Nullable List<Long> mechanismThreadIds, final boolean ignoreCurrentThread) {
67-
return getCurrentThreads(Thread.getAllStackTraces(), mechanismThreadIds, ignoreCurrentThread);
59+
final @Nullable List<Long> mechanismThreadIds,
60+
final boolean ignoreCurrentThread,
61+
final boolean attachStackTrace) {
62+
return getCurrentThreads(
63+
Thread.getAllStackTraces(), mechanismThreadIds, ignoreCurrentThread, attachStackTrace);
6864
}
6965

7066
@Nullable
71-
List<SentryThread> getCurrentThreads(final @Nullable List<Long> mechanismThreadIds) {
72-
return getCurrentThreads(Thread.getAllStackTraces(), mechanismThreadIds, false);
67+
List<SentryThread> getCurrentThreads(
68+
final @Nullable List<Long> mechanismThreadIds, final boolean attachStackTrace) {
69+
return getCurrentThreads(
70+
Thread.getAllStackTraces(), mechanismThreadIds, false, attachStackTrace);
7371
}
7472

7573
/**
@@ -87,7 +85,8 @@ List<SentryThread> getCurrentThreads(final @Nullable List<Long> mechanismThreadI
8785
List<SentryThread> getCurrentThreads(
8886
final @NotNull Map<Thread, StackTraceElement[]> threads,
8987
final @Nullable List<Long> mechanismThreadIds,
90-
final boolean ignoreCurrentThread) {
88+
final boolean ignoreCurrentThread,
89+
final boolean attachStackTrace) {
9190
List<SentryThread> result = null;
9291

9392
final Thread currentThread = Thread.currentThread();
@@ -109,7 +108,7 @@ List<SentryThread> getCurrentThreads(
109108
&& mechanismThreadIds.contains(thread.getId())
110109
&& !ignoreCurrentThread);
111110

112-
result.add(getSentryThread(crashed, item.getValue(), item.getKey()));
111+
result.add(getSentryThread(crashed, item.getValue(), item.getKey(), attachStackTrace));
113112
}
114113
}
115114

@@ -127,7 +126,8 @@ List<SentryThread> getCurrentThreads(
127126
private @NotNull SentryThread getSentryThread(
128127
final boolean crashed,
129128
final @NotNull StackTraceElement[] stackFramesElements,
130-
final @NotNull Thread thread) {
129+
final @NotNull Thread thread,
130+
final boolean attachStacktrace) {
131131
final SentryThread sentryThread = new SentryThread();
132132

133133
sentryThread.setName(thread.getName());
@@ -137,15 +137,17 @@ List<SentryThread> getCurrentThreads(
137137
sentryThread.setState(thread.getState().name());
138138
sentryThread.setCrashed(crashed);
139139

140-
final List<SentryStackFrame> frames =
141-
sentryStackTraceFactory.getStackFrames(stackFramesElements, false);
140+
if (attachStacktrace) {
141+
final List<SentryStackFrame> frames =
142+
sentryStackTraceFactory.getStackFrames(stackFramesElements, false);
142143

143-
if (options.isAttachStacktrace() && frames != null && !frames.isEmpty()) {
144-
final SentryStackTrace sentryStackTrace = new SentryStackTrace(frames);
145-
// threads are always gotten either via Thread.currentThread() or Thread.getAllStackTraces()
146-
sentryStackTrace.setSnapshot(true);
144+
if (frames != null && !frames.isEmpty()) {
145+
final SentryStackTrace sentryStackTrace = new SentryStackTrace(frames);
146+
// threads are always gotten either via Thread.currentThread() or Thread.getAllStackTraces()
147+
sentryStackTrace.setSnapshot(true);
147148

148-
sentryThread.setStacktrace(sentryStackTrace);
149+
sentryThread.setStacktrace(sentryStackTrace);
150+
}
149151
}
150152

151153
return sentryThread;

‎sentry/src/test/java/io/sentry/MainEventProcessorTest.kt‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ class MainEventProcessorTest {
249249
}
250250

251251
@Test
252-
fun `when attach threads is disabled, but the hint is Abnormal, still sets threads`() {
252+
fun `when attach threads is disabled, but the hint is Abnormal, still sets threads and stacktraces`() {
253253
val sut = fixture.getSut(attachThreads = false, attachStackTrace = false)
254254

255255
var event = SentryEvent(RuntimeException("error"))
@@ -258,6 +258,20 @@ class MainEventProcessorTest {
258258

259259
assertNotNull(event.threads)
260260
assertEquals(1, event.threads!!.count { it.isCrashed == true })
261+
assertNotNull(event.threads!!.first().stacktrace)
262+
}
263+
264+
@Test
265+
fun `when attach threads is enabled, but stacktraces is not its reflected`() {
266+
val sut = fixture.getSut(attachThreads = true, attachStackTrace = false)
267+
268+
var event = SentryEvent(RuntimeException("error"))
269+
val hint = Hint()
270+
event = sut.process(event, hint)
271+
272+
assertNotNull(event.threads)
273+
assertEquals(1, event.threads!!.count { it.isCrashed == true })
274+
assertNull(event.threads!!.first().stacktrace)
261275
}
262276

263277
@Test

‎sentry/src/test/java/io/sentry/SentryThreadFactoryTest.kt‎

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,9 @@ import kotlin.test.assertTrue
1010

1111
class SentryThreadFactoryTest {
1212
class Fixture {
13-
internal fun getSut(attachStacktrace: Boolean = true) =
13+
internal fun getSut() =
1414
SentryThreadFactory(
15-
SentryStackTraceFactory(SentryOptions().apply { addInAppExclude("io.sentry") }),
16-
with(SentryOptions()) {
17-
isAttachStacktrace = attachStacktrace
18-
this
19-
},
15+
SentryStackTraceFactory(SentryOptions().apply { addInAppExclude("io.sentry") })
2016
)
2117
}
2218

@@ -25,34 +21,37 @@ class SentryThreadFactoryTest {
2521
@Test
2622
fun `when getCurrentThreads is called, not empty result`() {
2723
val sut = fixture.getSut()
28-
val threads = sut.getCurrentThreads(null)
24+
val threads = sut.getCurrentThreads(null, false)
2925
assertNotEquals(0, threads!!.count())
3026
}
3127

3228
@Test
3329
fun `when currentThreads is called, current thread is marked crashed`() {
3430
val sut = fixture.getSut()
35-
assertEquals(1, sut.getCurrentThreads(null)!!.filter { it.isCrashed == true }.count())
31+
assertEquals(1, sut.getCurrentThreads(null, false)!!.filter { it.isCrashed == true }.count())
3632
}
3733

3834
@Test
3935
fun `when currentThreads is called with ignoreCurrentThread, current thread is not marked crashed`() {
4036
val sut = fixture.getSut()
41-
assertEquals(0, sut.getCurrentThreads(null, true)!!.filter { it.isCrashed == true }.count())
37+
assertEquals(
38+
0,
39+
sut.getCurrentThreads(null, true, false)!!.filter { it.isCrashed == true }.count(),
40+
)
4241
}
4342

4443
@Test
4544
fun `when currentThreads is called, thread state is captured`() {
4645
val sut = fixture.getSut()
47-
assertTrue(sut.getCurrentThreads(null)!!.all { it.state != null })
46+
assertTrue(sut.getCurrentThreads(null, false)!!.all { it.state != null })
4847
}
4948

5049
@Test
5150
fun `when currentThreads is called, some thread stack frames are captured`() {
5251
val sut = fixture.getSut()
5352
assertTrue(
5453
sut
55-
.getCurrentThreads(null)!!
54+
.getCurrentThreads(null, true)!!
5655
.filter { it.stacktrace != null }
5756
.any { it.stacktrace!!.frames!!.count() > 0 }
5857
)
@@ -63,18 +62,18 @@ class SentryThreadFactoryTest {
6362
val sut = fixture.getSut()
6463
assertTrue(
6564
sut
66-
.getCurrentThreads(null)!!
65+
.getCurrentThreads(null, true)!!
6766
.filter { it.stacktrace != null }
6867
.any { it.stacktrace!!.snapshot == true }
6968
)
7069
}
7170

7271
@Test
7372
fun `when currentThreads and attachStacktrace is disabled, stack frames are not captured`() {
74-
val sut = fixture.getSut(false)
73+
val sut = fixture.getSut()
7574
assertFalse(
7675
sut
77-
.getCurrentThreads(null)!!
76+
.getCurrentThreads(null, false)!!
7877
.filter { it.stacktrace != null }
7978
.any { it.stacktrace!!.frames!!.count() > 0 }
8079
)
@@ -87,15 +86,15 @@ class SentryThreadFactoryTest {
8786
val currentThread = Thread.currentThread()
8887
stackTraces.remove(currentThread)
8988

90-
val threads = sut.getCurrentThreads(stackTraces, null, false)
89+
val threads = sut.getCurrentThreads(stackTraces, null, false, false)
9190

9291
assertNotNull(threads!!.firstOrNull { it.id == currentThread.id })
9392
}
9493

9594
@Test
9695
fun `When passing empty param to getCurrentThreads, returns null`() {
9796
val sut = fixture.getSut()
98-
val threads = sut.getCurrentThreads(mapOf(), null, false)
97+
val threads = sut.getCurrentThreads(mapOf(), null, false, false)
9998

10099
assertNull(threads)
101100
}
@@ -108,15 +107,15 @@ class SentryThreadFactoryTest {
108107
val stacktraces = emptyArray<StackTraceElement>()
109108
val threadList = mutableMapOf(thread to stacktraces)
110109

111-
val threads = sut.getCurrentThreads(threadList, threadIds, false)
110+
val threads = sut.getCurrentThreads(threadList, threadIds, false, false)
112111

113112
assertNotNull(threads!!.firstOrNull { it.isCrashed == true })
114113
}
115114

116115
@Test
117116
fun `when getCurrentThread is called, returns current thread`() {
118117
val sut = fixture.getSut()
119-
val threads = sut.currentThread
118+
val threads = sut.getCurrentThread(false)
120119
assertEquals(1, threads!!.count())
121120
}
122121
}

0 commit comments

Comments
 (0)