RUMM-1883 Report binary image with no UUID #724
Conversation
maxep
force-pushed
the
maxep/RUMM-1883/report-invalid-images
branch
from
a63ce49
to
d635cdf
Compare
| @@ -150,7 +150,7 @@ internal struct DDCrashReportExporter { | |||
|
|
|||
| return DDCrashReport.BinaryImage( | |||
| libraryName: image.imageName, | |||
| uuid: image.uuid, | |||
| uuid: image.uuid ?? unavailable, | |||
There was a problem hiding this comment.
It is still relevant to report binary images with no UUID, like we do for missing architecture. Without uuid nor arch, symbolication won't be possible, but this will give more info to the crash report and help us identify missing data.
There was a problem hiding this comment.
Double checking - this will be possible to differentiate one ??? from another ??? by exploring the content of binary_images[], right?
There was a problem hiding this comment.
Yes. We shouldn't even get ??? in stack frames for known library from PLCR as it already checks for name nullability (as long as the name is UTF8).
And we will now be able to identify binary_images with no uuid, they were filtered out before.
| return nil | ||
| } | ||
|
|
||
| self.uuid = imageUUID | ||
| self.uuid = imageInfo.imageUUID |
There was a problem hiding this comment.
I think it's safe, but wanted to double check if this won't crash if imageUUID is nil? In PLCR it is defined as implicitly unwrapped String:
/**
* 128-bit object UUID (matches Mach-O DWARF dSYM files). May be nil if unavailable.
*/
@property(nonatomic, readonly, strong) NSString *imageUUID;Hence, previous code was checking imageInfo.hasImageUUID before evaluating it.
/**
* YES if this image has an associated UUID.
*/
@property(nonatomic, readonly) BOOL hasImageUUID;The problem with implicit unwrapping is common to many PLCR types/fields and is solved by our CrashReport (swift). This test should already cover it:
but for the sake of sanity, let's double check if it does.
There was a problem hiding this comment.
Without NS_ASSUME_NONNULL_* region or _Nonnull objc annotation, the property is implicitly unwrapped optional, so this won't crash as uuid is now optional. The previous check was not necessary since the guard let was already evaluating nullability.
But I can checks in test tho 👍
There was a problem hiding this comment.
It's actually already covered in test :)
What and why?
Stack frames in crash report could be reported as
???if PLCR could not find the image or if the image's UUID is missing. To identify the root cause, we should keep reporting binary images without UUID.How?
StackFrame.libraryNameeven for binary image with no UUIDuuidinBinaryImageInfoReview checklist