Skip to content

Conversation

@philipphofmann
Copy link
Member

@philipphofmann philipphofmann commented Jan 31, 2023

📜 Description

When closing the SDK directly after starting it, it could happen that the ANRTracker didn't start its watchdog thread yet. Canceling the ANRTrackers thread required the instance of the watchdog thread and didn't work correctly. This is fixed now by using simple state management in the ANRTracker. While this is an edge case, it can help with flaky tests, as the test logs showed signs of the ANRTracker running in the background.

💡 Motivation and Context

This came up while checking the raw test logs of failing tests on the main branch, see https://github.com/getsentry/sentry-cocoa/actions/runs/4045915250/jobs/6958149321. The logs contain several statements, although the ANRTracker shouldn't be running.

Test Case '-[SentryTests.SentryLogTests testLevelNone_PrintsEverythingExceptNone]' started.
2023-01-30 16:35:42.447867+0000 xctest[10355:27467] [Sentry] [fatal] 0
2023-01-30 16:35:42.448038+0000 xctes2023-01-30 16:35:42.457839+0000 xctest[10355:29827] [Sentry] [warning] [SentryANRTracker:97] ANR detected.
t[10355:27467] [Sentry] [error] 1
2023-01-30 16:35:42.462238+0000 xctest[10355:27467] [Sentry] [warning] 2
2023-01-30 16:35:42.462394+0000 xctest[10355:27467] [Sentry] [info] 3
2023-01-30 16:35:42.462848+0000 xctest[10355:27467] [Sentry] [debug] 4
2023-01-30 16:35:42.467022+0000 xctest[10355:27467] [Sentry] [debug] [SentryFileManager:659] SentryFileManager.cachePath: /Users/runner/Library/Developer/CoreSimulator/Devices/76C9311A-80D8-48DB-8FD8-A408B736C8F1/data/Library/Caches
2023-01-30 16:35:42.467370+0000 xctest[10355:27467] [Sentry] [debug] [SentryFileManager:242] Deleting /Users/runner/Library/Developer/CoreSimulator/Devices/76C9311A-80D8-48DB-8FD8-A408B736C8F1/data/Library/Caches/io.sentry/b810aaca937def33af3796f237e6793ec907f1e4/events
2023-01-30 16:35:42.468337+0000 xctest[10355:27467] [Sentry] [debug] [SentryHttpTransport:251] sendAllCachedEnvelopes start.
2023-01-30 16:35:42.468602+0000 xctest[10355:27467] [Sentry] [debug] [SentryHttpTransport:263] No envelopes left to send.
2023-01-30 16:35:42.469163+0000 xctest[10355:27467] [Sentry] [debug] [SentryHttpTransport:342] Finished sending.
2023-01-30 16:35:42.471268+0000 xctest[10355:32063] [Sentry] [debug] [SentryFileManager:97] Dispatched deletion of old envelopes from <SentryFileManager: 0x6000006b7980>
2023-01-30 16:35:42.502122+0000 xctest[10355:32063] [Sentry] [debug] [SentryFileManager:423] Writing to file: /Users/runner/Library/Developer/CoreSimulator/Devices/76C9311A-80D8-48DB-8FD8-A408B736C8F1/data/Library/Caches/io.sentry/b810aaca937def33af3796f237e6793ec907f1e4/envelopes/1675096542.502065-00000-2F1BFA76-6E3E-4E8B-9F4C-3E9D860217DA.json
/Users/runner/work/sentry-cocoa/sentry-cocoa/Tests/SentryTests/Helper/SentryLogTests.swift:65: error: -[SentryTests.SentryLogTests testLevelNone_PrintsEverythingExceptNone] : XCTAssertEqual failed: ("["[Sentry] [fatal] 0", "[Sentry] [error] 1", "[Sentry] [warning] 2", "[Sentry] [info] 3", "[Sentry] [debug] 4"]") is not equal to ("["[Sentry] [fatal] 0", "[Sentry] [warning] [SentryANRTracker:97] ANR detected.", "[Sentry] [error] 1", "[Sentry] [warning] 2", "[Sentry] [info] 3", "[Sentry] [debug] 4"]")
Test Case '-[SentryTests.SentryLogTests testLevelNone_PrintsEverythingExceptNone]' failed (0.059 seconds).

💚 How did you test it?

Unit tests.

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • I updated the docs if needed
  • Review from the native team if needed
  • No breaking changes

🔮 Next steps

When closing the SDK directly after starting it, it could happen that the ANRTracker
didn't start its watchdog thread yet. Canceling the ANRTrackers thread required the
instance of the watchdog thread and didn't work correctly. This is fixed now by using
simple state management in the ANRTracker. While this is an edge case, it can help
with flaky tests, as the test logs showed signs of the ANRTracker running in the
background.
@codecov
Copy link

codecov bot commented Feb 2, 2023

Codecov Report

Merging #2671 (ed684f1) into main (56deb55) will increase coverage by 0.22%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2671      +/-   ##
==========================================
+ Coverage   80.37%   80.59%   +0.22%     
==========================================
  Files         247      247              
  Lines       22942    22982      +40     
  Branches    10136    10161      +25     
==========================================
+ Hits        18440    18523      +83     
+ Misses       4035     3992      -43     
  Partials      467      467              
Impacted Files Coverage Δ
Sources/Sentry/SentryANRTracker.m 100.00% <100.00%> (ø)
Sources/Sentry/SentryThreadWrapper.m 100.00% <100.00%> (ø)
...ryCrash/Recording/Tools/SentryCrashDynamicLinker.c 79.77% <0.00%> (-0.08%) ⬇️
...yCrash/Recording/Tools/SentryCrashUUIDConversion.c 100.00% <0.00%> (ø)
Sources/Sentry/SentryCrashIntegration.m 98.66% <0.00%> (+0.03%) ⬆️
Sources/Sentry/SentrySerialization.m 87.97% <0.00%> (+0.12%) ⬆️
Sources/Sentry/SentryMachLogging.cpp 3.01% <0.00%> (+0.50%) ⬆️
Sources/Sentry/SentryThreadInspector.m 98.00% <0.00%> (+2.00%) ⬆️
Sources/SentryCrash/Recording/Tools/SentryHook.c 86.02% <0.00%> (+2.20%) ⬆️
Sources/Sentry/SentryNetworkTracker.m 95.39% <0.00%> (+4.98%) ⬆️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 56deb55...ed684f1. Read the comment docs.

Copy link
Contributor

@brustolin brustolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@philipphofmann philipphofmann merged commit 075a044 into main Feb 6, 2023
@philipphofmann philipphofmann deleted the fix/anr-tracker-cleanup branch February 6, 2023 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants