Ensure tests cover non-error timber tags #4843
Conversation
markushi
requested review from
adinauer,
lcian,
romtsn and
stefanosiano
as code owners
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| fun `Tree captures an event with TimberTag tag for debug events`() { | ||
| val sut = fixture.getSut(minEventLevel = SentryLevel.INFO) | ||
| Timber.plant(sut) | ||
| // only available thru static class | ||
| Timber.tag("infoTag").i("message") | ||
| verify(fixture.scopes).captureEvent(check { assertEquals("infoTag", it.getTag("TimberTag")) }) | ||
| } | ||
|
|
||
| @Test | ||
| fun `Tree captures an event without TimberTag tag`() { | ||
| val sut = fixture.getSut() |
There was a problem hiding this comment.
Bug: SentryTimberTree.log() fails to capture events when Timber's chained API is used, only setting pendingTag.
Severity: HIGH | Confidence: 1.00
🔍 Detailed Analysis
When Timber's chained API, such as Timber.tag("tag").i("message"), is used, the SentryTimberTree.log(priority: Int, tag: String?, message: String, t: Throwable?) method is invoked. This method only sets the pendingTag in a ThreadLocal but does not proceed with the actual logging by calling super.log() or logWithSentry(). Consequently, no event is captured by Sentry for these chained logging calls, leading to missing log data.
💡 Suggested Fix
Modify SentryTimberTree.log() to invoke the actual logging logic (e.g., logWithSentry()) after setting the pendingTag when Timber's chained API is used.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location:
sentry-android-timber/src/test/java/io/sentry/android/timber/SentryTimberTreeTest.kt#L139-L153
Potential issue: When Timber's chained API, such as `Timber.tag("tag").i("message")`, is
used, the `SentryTimberTree.log(priority: Int, tag: String?, message: String, t:
Throwable?)` method is invoked. This method only sets the `pendingTag` in a ThreadLocal
but does not proceed with the actual logging by calling `super.log()` or
`logWithSentry()`. Consequently, no event is captured by Sentry for these chained
logging calls, leading to missing log data.
Did we get this right? 👍 / 👎 to inform future reviews.
|
Closing in favor of #4844 |
📜 Description
Add another test to verify that #4484 should be working as expected. Also ran it locally against the Timber 5.0.1 and still can not reproduce the issue.
#skip-changelog