-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Crash logging: Add hybrid SDK helpers to allow better stack traces for other platforms #183
Conversation
These helpers will allow better stack traces for exceptions produced from other platforms, like React native in case of Gutenberg editor.
public func shouldSendEvent() -> Bool { | ||
#if DEBUG | ||
return UserDefaults.standard.bool(forKey: "force-crash-logging") | ||
#else | ||
return !dataProvider.userHasOptedOut | ||
#endif | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initially, I thought that beforeSend
would be called in all cases but looks like that for capturing envelopes this is skipped. For this reason, I moved out this logic from beforeSend
and expose it, this way Gutenberg can access it and determine if an event should be sent.
public func getOptionsDict() -> [String: Any] { | ||
return [ | ||
"dsn": self.dataProvider.sentryDSN, | ||
"environment": self.dataProvider.buildType, | ||
"releaseName": self.dataProvider.releaseName | ||
] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In hybrid SDKs like React Native (used in Gutenberg), we need to initialize the SDK so I exposed the required options to match the same values we use for the native SDK.
public func getSentryUserDict() -> [String: Any]? { | ||
return dataProvider.currentUser?.sentryUser.serialize() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to attachScopeToEvent
, I exposed the Sentry user for including it in the events generated in the hybrid SDKs. Initially, I thought that the user would be part of the current scope but it's not as it's assigned to the event in the beforeSend
callback.
public func attachScopeToEvent(_ eventDict: [String: Any]) -> [String: Any] { | ||
let scope = SentrySDK.currentHub().getScope().serialize() | ||
|
||
// Setup tags | ||
var tags = scope["tags"] as? [String: String] ?? [String: String]() | ||
tags["locale"] = NSLocale.current.languageCode | ||
|
||
/// Always provide a value in order to determine how often we're unable to retrieve application state | ||
tags["app.state"] = ApplicationFacade().applicationState ?? "unknown" | ||
|
||
tags["release"] = self.dataProvider.releaseName | ||
|
||
// Assign scope to event | ||
var eventWithScope = eventDict | ||
eventWithScope["tags"] = tags | ||
eventWithScope["breadcrumbs"] = scope["breadcrumbs"] | ||
eventWithScope["contexts"] = scope["context"] | ||
|
||
return eventWithScope | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The scope used in the events generated in hybrid SDKs is different from the one of the native side so I expose this function to incorporate the same data we see in native crashes.
This logic serializes the current scope and includes the same extra tags added in the beforeSend
callback, finally, a new object is created with the following data set:
- Tags
- Breadcrumbs
- Contexts
public func logEnvelope(_ envelopeDict: [String: Any]) { | ||
if JSONSerialization.isValidJSONObject(envelopeDict) { | ||
guard let headerDict = envelopeDict["header"] as? [String: Any] else { | ||
DDLogError("⛔️ Unable to send envelope to Sentry – header is not defined in the envelope.") | ||
return | ||
} | ||
guard let headerEventId = headerDict["event_id"] as? String else { | ||
DDLogError("⛔️ Unable to send envelope to Sentry – event id is not defined in the envelope header.") | ||
return | ||
} | ||
guard let payloadDict = envelopeDict["payload"] as? [String: Any] else { | ||
DDLogError("⛔️ Unable to send envelope to Sentry – payload is not defined in the envelope.") | ||
return | ||
} | ||
guard let eventLevel = payloadDict["level"] as? String else { | ||
DDLogError("⛔️ Unable to send envelope to Sentry – level is not defined in the envelope payload.") | ||
return | ||
} | ||
|
||
// Define the envelope header | ||
let sdkInfo = SentrySdkInfo.init(dict: headerDict) | ||
let eventId = SentryId.init(uuidString: headerEventId) | ||
let envelopeHeader = SentryEnvelopeHeader.init(id: eventId, andSdkInfo: sdkInfo) | ||
|
||
guard let envelopeItemData = try? JSONSerialization.data(withJSONObject: payloadDict) else { | ||
DDLogError("⛔️ Unable to send envelope to Sentry – payload could not be serialized.") | ||
return | ||
} | ||
|
||
let itemType = payloadDict["type"] as? String ?? "event" | ||
let envelopeItemHeader = SentryEnvelopeItemHeader.init(type: itemType, length: UInt(bitPattern: envelopeItemData.count)) | ||
let envelopeItem = SentryEnvelopeItem.init(header: envelopeItemHeader, data: envelopeItemData) | ||
let envelope = SentryEnvelope.init(header: envelopeHeader, singleItem: envelopeItem) | ||
|
||
#if DEBUG | ||
SentrySDK.currentHub().getClient()?.capture(envelope: envelope) | ||
#else | ||
if eventLevel == "fatal" { | ||
// Storing to disk happens asynchronously with captureEnvelope | ||
SentrySDK.currentHub().getClient()?.store(envelope) | ||
} else { | ||
SentrySDK.currentHub().getClient()?.capture(envelope: envelope) | ||
} | ||
#endif | ||
} else { | ||
DDLogError("⛔️ Unable to send envelope to Sentry – envelope is not a valid JSON object.") | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hybrid SDKs send the events as envelopes, it's basically an object that contains all the data required to ingest an event into Sentry.
This implementation is based on the original version of the Sentry React native SDK (version 2.4.2
). I'd like to note that the code reference is from version 2.4.2
because is the last version that uses Sentry iOS SDK version 6.x
that should match the version we use for crash logging (currently 6.x
from the podspec).
👋 @jkmassel I'd appreciate if you could take a look at this implementation that enables the Sentry integration with React native, let me know if you're available, thank you very much for your help 🙇 ! Related issue: wordpress-mobile/gutenberg-mobile#1187. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested via WordPress/gutenberg#32768 (review)
The code changes LGTM too 🎉
Surpassed by #278. |
This PR adds helpers that will allow us to improve the stack traces that we get when an exception is caused by hybrid SDKs, like React native for the Gutenberg editor.
Related PRs:
WordPress-iOS
PR: wordpress-mobile/WordPress-iOS#16700gutenberg-mobile
PR: wordpress-mobile/gutenberg-mobile#3629gutenberg
PR: WordPress/gutenberg#32768