Capture and report Thread.state for Android Runtime threads
#1367
Conversation
lemnik
force-pushed
the
PLAT-7204/add-thread-state
branch
from
0ff0953 to
64a1aad
Compare
lemnik
force-pushed
the
PLAT-7204/add-thread-state
branch
from
64a1aad to
7f588ed
Compare
| @@ -98,6 +101,7 @@ internal class ThreadSerializationTest { | |||
| "main-one", | |||
| ThreadType.ANDROID, | |||
| false, | |||
| Thread.State.RUNNABLE, | |||
There was a problem hiding this comment.
It'd be good to have a test case that serializes something other than RUNNABLE
| @@ -32,6 +32,7 @@ public Thread deserialize(Map<String, Object> map) { | |||
| MapUtils.<String>getOrThrow(map, "name"), | |||
| ThreadType.valueOf(type.toUpperCase(Locale.US)), | |||
| errorReportingThread, | |||
| Thread.State.byName(MapUtils.<String>getOrThrow(map, "state")), | |||
There was a problem hiding this comment.
Will this require any updates to the React Native JavaScript layer for the state field to get passed down? It feels like it might work out-of-the-box but if not then this might result in threads being dropped from JS payloads. Just something to check when updating RN.
There was a problem hiding this comment.
The ReactNative JS layer doesn't directly know about threads, however the typescript definitions in common.d.ts will need to be changed to reflect the state property.
| @@ -18,6 +19,7 @@ class ThreadInternal internal constructor( | |||
| writer.name("id").value(id) | |||
| writer.name("name").value(name) | |||
| writer.name("type").value(type.desc) | |||
| writer.name("state").value(state.name) | |||
There was a problem hiding this comment.
Double-checking - will getName() be safe if ProGuard obfuscation is enabled?
There was a problem hiding this comment.
Progaurd doesn't obfuscate enum names by default - but that doesn't mean other tools won't. I'll sort out something more robust 👍
lemnik
force-pushed
the
PLAT-7204/add-thread-state
branch
from
f3a504b to
43654cf
Compare
Goal
Capture and report the Thread.state of all captured threads to help in diagnosing issues.
Design
To separate our implementation from any future platform changes or errors, a new Bugsnag
Thread.Stateenum was introduced, reflecting the possible thread-states currently available on Android. The new enum also includes anUNKNOWNstate which is used in cases where we either cannot capture or map the platform state.Testing
Manual testing with the Example app and a debug server. Updated existing unit tests and end-to-end tests to include checks for the new Thread.State field.