fix: Sanitize UserInfo of NSError and NSException

[koenpunt]

📜 Description

This PR add sanatizing to the error's user info dictionary.

💡 Motivation and Context

Certain errors would cause the event to drop the sdk and context properties, because the error's user info contained values that couldn't be serialized as JSON.

💚 How did you test it?

Changed the library in my project locally, and saw the error being reported in Sentry without the "A value set to the context or sdk is not serializable. Dropping context and sdk." breadcrumb, and with the context and sdk properties present.

I'm not sure where I should introduce a test for it. Preferably I would like to add a test that handles an error with problematic user info, and fails without my changes. But since the formatting of the event happens in SentryClient (covered by SentryClientTests), and the serialization in SentrySerialization (covered by SentrySerializationTests), there currently is no way to do that.

📝 Checklist

• I reviewed the submitted code
• I added tests to verify the changes
• I updated the CHANGELOG
• I updated the docs if needed
• No breaking changes

🔮 Next steps

[codecov-io]

Codecov Report

Merging #770 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #770      +/-   ##
==========================================
- Coverage   92.83%   92.83%   -0.01%     
==========================================
  Files          73       73              
  Lines        3212     3211       -1     
==========================================
- Hits         2982     2981       -1     
  Misses        230      230              

Impacted Files	Coverage Δ
Sources/Sentry/SentryClient.m	92.35% <100.00%> (ø)
Sources/Sentry/SentryNSURLRequest.m	65.15% <0.00%> (-0.53%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c1f1d0...606d9a7. Read the comment docs.

[bruno-garcia]

@koenpunt Thanks for contributing!
Could you please add some tests to validate this? Also, CI doesn't seem happy

[koenpunt]

Could you please add some tests to validate this?

As I mentioned in the description; I'm not really sure where I should add a test.

[philipphofmann]

As I mentioned in the description; I'm not really sure where I should add a test.

Thanks for opening up this PR. You can add a test in SentryClientTests. You just need to pass an NSError to for example client.captureError and set non-serializable values, like a Date, to the userInfo of the error. Then you check if the date in the userInfo is serialized correctly into the context of the event.

You could add something like this below testCaptureErrorWithUserInfo

func testCaptureErrorWithDateInUserInfo() {
    let expectedValue = (fixture.currentDateProvider.date() as NSDate).sentry_toIso8601String()
    let error = NSError(domain: "domain", code: 0, userInfo: ["key": fixture.currentDateProvider.date()])
    let eventId = fixture.getSut().capture(error: error, scope: fixture.scope)

    XCTAssertNotNil(eventId)
    assertLastSentEvent { actual in
        XCTAssertEqual(expectedValue, actual.context!["user info"]!["key"] as? String)
    }
}

But you need to add let currentDateProvider = TestCurrentDateProvider() to the fixture.

[koenpunt]

@philipphofmann thanks, I'll try that!

[koenpunt]

@philipphofmann I used an URL instead, because that's what was causing the issue for me. Should be good to go!

[koenpunt]

The build is failing due to the branch name(?):

  Error: Error: error: pathspec 'patch-1' did not match any file(s) known to git

[codecov-commenter]

Codecov Report

Merging #770 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #770      +/-   ##
==========================================
- Coverage   92.83%   92.83%   -0.01%     
==========================================
  Files          73       73              
  Lines        3212     3211       -1     
==========================================
- Hits         2982     2981       -1     
  Misses        230      230              

Impacted Files	Coverage Δ
Sources/Sentry/SentryClient.m	92.35% <100.00%> (ø)
Sources/Sentry/SentryNSURLRequest.m	65.15% <0.00%> (-0.53%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c1f1d0...3afeedf. Read the comment docs.

[koenpunt]

Oh I get it, it tried to commit formatting changes. Now ran clang-format locally and updated the PR.

[philipphofmann]

Awesome thanks. The entry in the changelog is in the wrong section, but never worry I'm going to fix it in another PR :).