Collect and report all types of crashes together with full stack trace information #250
Conversation
matux
changed the base branch from
master
to
collect_crashes_with_stacktraces
| [super install]; | ||
|
|
||
| KSCrashMonitorType monitoring = isDebuggerAttached() | ||
| ? KSCrashMonitorTypeDebuggerSafe & ~(KSCrashMonitorTypeOptional | KSCrashMonitorTypeExperimental) |
There was a problem hiding this comment.
Prefix ~ is bitwise negation. It basically flips the integer bits:
So say some x is 5 and it's type is a 16-bit representation of an integer:
0000000000000101
~x would turn that into:
1111111111111010`
In our case, we use it to flip the bitwise-OR of (Optional, Experimental), effectively turning those bits to 0 in DebuggerSafe, and thus, removing those two monitors we don't want.
So, let's say we have 4 integers of 4 bits:
monitor_a = 1 = b0001
monitor_b = 2 = b0010
monitor_c = 4 = b0100
monitor_d = 8 = b1000
And say only a, c and d are debugger safe:
debugger_safe = monitor_a | monitor_c | monitor_d = b1101
And that we want to use debugger_safe but not monitor_d:
monitors_i_want = debugger_safe & ~monitor_d = b0101
monitor_disb1000~monitor_dflips its bits, fromb1000tob0111debugger_safeisb1101b1101 & b0111=b0101
1st bit = true && true = true (1)
2nd bit = false && true = false (0)
3rd bit = true && true = true (1)
4th bit = true && false = false (0)
Which effectively has the bits corresponding to monitor_a and monitor_c. Which are the ones we want.
Bitwise operations are simply boolean operations over bits where 1 means true and 0 false.
All in all, this is a horrible way of on/off options that I would never suggest unless you're extremely constrained by memory and performance. And even on those cases, LLVM is smart enough to do it for you.
There was a problem hiding this comment.
Ahh ok it didn't register to me that this was effectively just configuration flags. Makes sense!
| @"short_version": shortVersion ? shortVersion : @"", | ||
| @"bundle_identifier": bundleIdentifier ? bundleIdentifier : @"", | ||
| @"app_name": bundleName ? bundleName : [[NSProcessInfo processInfo] processName] | ||
| @"code_version": version ?: @"", |
There was a problem hiding this comment.
Is a ?: b in obj-c basically saying a or b if a doesn't exist? Shorthand for the ternary you removed?
There was a problem hiding this comment.
Exactly. It's called the Elvis operator, and since what's being compared is a pointer to a value, if the value doesn't exist, the pointer points to 0, and 0 is false. If the value exists, the pointer points to "something that is not 0", and that means true.
| RollbarSdkLog(@"Column: %s", column[i]); | ||
| RollbarSdkLog(@"Data: %s", data[i]); | ||
| } | ||
| // RollbarSdkLog(@"Columns: %d", columns); |
There was a problem hiding this comment.
Whats the value in keeping this around?
There was a problem hiding this comment.
I'd like for our users to decide whether Rollbar spits output to the console or not. Right now, it spits everything and makes the console unusable. I commented out these because I want to restore them later behind some on/off flag.
| @@ -294,7 +294,7 @@ - (void)queuePayload_OnlyCallOnThisThread:(nonnull NSArray *)data { | |||
| } | |||
|
|
|||
| - (void)savePayload:(nonnull RollbarPayload *)payload withConfig:(nonnull RollbarConfig *)config { | |||
|
|
|||
| //RollbarSdkLog(@"RollbarThread::savePayload: %@", payload.data.body); | |||
There was a problem hiding this comment.
Same as above. I'd rather not lose these and restore them properly later. There are a ton of places where we output and I don't wanna go into a day-long hunt and would rather just search for //RollbarSdkLog.
I'd say this is certainly not good practice, but these are special circumstances 😛.
Description of the change
This PR rewrites the entire mechanism by which we detect and collect crash reports together with stack traces:
Type of change
Related issues
Checklists
Development
Code review