Thread aware live-report capture #158
Conversation
add generateLiveReport with thread to CrashReporting protocol defer mach_port_deallocate to avoid Mach-port leaks add helper currentThreadIdentifier() for thread id logging and testing generate reports with faulting thread Add thread forwarding and leak spec tests
| @@ -118,6 +118,7 @@ extension BacktraceReporter { | |||
| faultMessage: String? = nil) throws -> BacktraceReport { | |||
| attributesProvider.set(faultMessage: faultMessage) | |||
| let resource = try reporter.generateLiveReport(exception: exception, | |||
| thread: mach_thread_self(), | |||
There was a problem hiding this comment.
Should we always use the main thread id instead?
There was a problem hiding this comment.
No, we want to capture the specific currently executing thread, the main thread would be missing context and/or reporting wrong informations if the error occurs on a background thread for example.
There was a problem hiding this comment.
- callers who want main thread can still pass it explicitly.
| @@ -54,6 +54,25 @@ extension BacktraceCrashReporter: CrashReporting { | |||
| let reportData = try reporter.generateLiveReport(with: exception) | |||
There was a problem hiding this comment.
I think it would be better if you will stick to one implementation for everything. The generateLiveReport (no thread argument) can call generateLiveReport with a thread argument by using current thread. You don't need to have a duplicated logic. If you start extending it at some point, you would need to double your code
There was a problem hiding this comment.
sure thing, motivation was offering both methods without introducing a breaking change.
|
|
||
| // Returns the thread ID | ||
| // can be used for logging/testing | ||
| static func currentThreadIdentifier() -> thread_t { |
There was a problem hiding this comment.
shouldn't this be a helper? We already had a lot of code in Common. This class should be as clean as possible since this is a business logic of our SDK
There was a problem hiding this comment.
Common is for example app, removing this method since it's useless after consolidating generateLiveReport methods.
Consolidates the 'generateLiveReport' implementations Remove 'currentThreadIdentifier' helper as it is no longer required by the simplified public interface
Why
No public APIs changes, Handled-exception reports now show the actual faulting thread in Backtrace, eliminating false “Thread 0/1 (CRASHED)” stacks.
ref: BT-5583