feat: Add onCrashedLastRun

[philipphofmann]

📜 Description

Add a callback onCrashedLastRun to SentryOptions that is called by the SDK passing the eventId
when Sentry is initialized and the last program execution terminated with a crash.

💡 Motivation and Context

We want to give our users a callback for crashes so they can ask their users to attach feedback on what happened. This would solve #699 and #629.

The API together with user feedback looks like this

SentrySDK.start { options in
    // ...
    
    // Only gets called for the first crash event
    options.onCrashedLastRun = { event in
        self.displayUserFeedback(event: event)
    }
}

// Not a sentry function. Only wraps call to user feedback
func displayUserFeedback(event: Event) {
		
    // Display UI and get feedback from the user. 

    // Then send feedback to Sentry
    let feedback = SentryUserFeedback(eventId: event.eventId)
    feedback.comments = "It broke"
    feedback.name = "John Smith"
    feedback.email = "[email protected]"
    SentrySDK.capture(feedback: feedback)
}

💚 How did you test it?

Unit tests and simulator.

📝 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 #808 (008008d) into master (4647554) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #808      +/-   ##
==========================================
+ Coverage   93.56%   93.60%   +0.03%     
==========================================
  Files          74       74              
  Lines        3277     3297      +20     
==========================================
+ Hits         3066     3086      +20     
  Misses        211      211              

Impacted Files	Coverage Δ
Sources/Sentry/SentryClient.m	93.40% <100.00%> (+0.58%)	⬆️
Sources/Sentry/SentryHub.m	93.10% <100.00%> (ø)
Sources/Sentry/SentryOptions.m	100.00% <100.00%> (ø)
Sources/Sentry/SentrySDK.m	67.44% <100.00%> (+0.77%)	⬆️

---

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 4647554...008008d. Read the comment docs.

[philipphofmann]

h: What happens if there are multiple crash reports? Maybe it would be better to replace it with a property SentrySDK.lastCrashEventId, which is set internally. When multiple crash reports are converted to an event, this is going to be overwritten.

[georges]

A callback is not strictly necessary. The app can simply check for SentrySDK.lastCrashEventId at startup and if something is returned will know to prompt the user and submit the feedback for that event.

[bruno-garcia]

You'd need to know the eventId somehow. SentrySDK.lastEventId wouldn't have the value after restart, at least not in its current form

[georges]

Understood (that's what trigger my initial request #699). This value needs to be exposed somehow. The fact that the sdk can now submit to the feedback api is gravy but getting the eventId is the key.

[philipphofmann]

A property won't work really, because it might take a few milliseconds until it is initialized. This happens because the SentryCrashIntegration is converting crash reports on a background thread to SentryEvents. So it wouldn't be really handy to wait for it to be initialized and you don't have a friendly way to achieve this. Instead, I would propose that the onCrashedLastRun only gets called for the first crash event. In case there are multiple crashes to send which is an edge case the others are just sent without calling the callback. What do you think about this @georges?

[georges]

in this case callback is the way to go and agree, multiple crash should be an edge case anyways. the main concern i had was if the callback fired more than once in the same session and it would keep prompting the user. if it fires only once for the latest crash, that's sufficient.

[marandaneto]

H: I think this would work for most of the cases, but it's flakey IMO.
how do we know that the event calling onCrashedLastRun is actually the one that caused the crash?

I'd suggest another approach:

SentryCrash sets fixture.crashAdapter.internalCrashedLastLaunch so it knows the event that caused this flag to be toggled.
What about caching the eventId as well that caused the app to crash and later, we filter by eventId? so we'll be sure that the onCrashedLastRun is actually called with the event that actually crashed the App.

My concern is, if the app is offline and there are multiple and distinct hard crashes, the event called onCrashedLastRun might not be the event that actually caused the last crash and the user feedback won't be accurate.

[philipphofmann]

It's very good that you think about such edge cases, but I think we can take another simpler approach.

When a crash happens, SentryCrash writes a crash-report.json to disk. The next time the app is launched, the SDK installs the SentryCrashIntegration, which calls the SentryCrashReportConverter, who converts the crash-report.json to a SentryEvent. The SentryCrashReportSink then calls [SentrySDK captureCrashEvent:event];. At that point, SentrySDK knows that it is an event with a crash. The SentryHub also has a captureCrashEvent, but the SentryClient doesn't. Therefore I suggest that the SentryHub just passes this information to the SentryClient and in callOnCrashedLastRun we can simplify the logic a lot.

Good point I'm going to change this.

[marandaneto]

thanks for doing this :)

I left a few comments and one of them concerns me a bit, looks good overall.

[bruno-garcia]

LGTM