fix: Make metadata tabs always mutable #430
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses an issue whereby a tab within metadata can be an
NSDictionary
rather than anNSMutableDictionary
, and an exception can be generated in abeforeSend
callback, preventing the report being sent. The dictionaries in metadata can be immutable because within the constructor of a report we don't use the property setter or call through toBSGSanitizeDict
which will ensure the dictionary is mutable. See https://github.com/bugsnag/bugsnag-cocoa/blob/master/Source/BugsnagCrashReport.m#L280 for an example.The fix I have gone for here is to make a
mutableCopy
of the tab within theaddAttribute:withValue:toTabWithName
function. We could also add some calls toBSGSanitizeDict
inside the constructor when initially setting themetadata
value, which will ensure the dictionary is mutable, but I don't know the context of why that is not already there. Maybe @kattrali had a reason to avoid the setter when constructing the class. We could add the santize call also if we think that is a good idea for a belt and braces approach.I have also added a try-catch to the execution of each
beforeSend
callback to ensure that if they throw an exception, the result is just that thebeforeSend
don't execute, rather than preventing delivery of the reports.It is worth pointing out that I believe
addMetadata:toTabWithName:
was not affected by this issue and so is unchanged.The tests have also been paired back to be the bare minimum to reproduce the issue and verify the code change fixes it. This PR is an alternative to #429.