Skip to content
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

Fix Electron onSendError callbacks not being called #1999

Merged
merged 6 commits into from
Aug 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@
- (react-native) Update bugsnag-cocoa from v6.25.2 to [v6.26.2](https://github.com/bugsnag/bugsnag-cocoa/blob/master/CHANGELOG.md#6262-2023-04-20)
- (delivery-xml-http-request) Ensure delivery errors are passed to the post report callback [#1938](https://github.com/bugsnag/bugsnag-js/pull/1938)

### Fixed

- (electron) Fix `onSendError` callbacks not being called [#1999](https://github.com/bugsnag/bugsnag-js/pull/1999)

## 7.20.1 (2023-02-08)

### Changes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ module.exports = (app, net, filestore, NativeClient) => ({

const { sendMinidump } = sendMinidumpFactory(net, client)
const queue = new MinidumpQueue(filestore)
const loop = new MinidumpDeliveryLoop(sendMinidump, client._config.onSend, queue, client._logger)
const loop = new MinidumpDeliveryLoop(sendMinidump, client._config.onSendError, queue, client._logger)

app.whenReady().then(() => {
const stateManagerPlugin = client.getPlugin('clientStateManager')
Expand Down
138 changes: 138 additions & 0 deletions packages/plugin-electron-deliver-minidumps/event-serialisation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
const Event = require('@bugsnag/core/event')
const Session = require('@bugsnag/core/session')
const Breadcrumb = require('@bugsnag/core/breadcrumb')

const supportedProperties = [
'app',
'breadcrumbs',
'context',
'device',
'featureFlags',
'groupingHash',
'metaData',
'request',
'session',
'severity',
'unhandled',
'user'
]

function hasValueForProperty (object, name) {
if (!Object.prototype.hasOwnProperty.call(object, name)) {
return false
}

const value = object[name]

if (typeof value === 'undefined' || value === null) {
return false
}

if (Array.isArray(value) && value.length === 0) {
return false
}

if (typeof value === 'object' && Object.keys(value).length === 0) {
return false
}

return true
}

function serialiseEvent (event) {
const json = event.toJSON()
const serialisedEvent = {}

for (let i = 0; i < supportedProperties.length; ++i) {
const property = supportedProperties[i]

if (!hasValueForProperty(json, property)) {
continue
}

// breadcrumbs and session information need to be encoded further
if (property === 'breadcrumbs') {
serialisedEvent.breadcrumbs = json.breadcrumbs.map(breadcrumb => breadcrumb.toJSON())
} else if (property === 'session') {
serialisedEvent.session = json.session.toJSON()
} else if (property === 'metaData') {
serialisedEvent.metadata = json[property]
} else {
serialisedEvent[property] = json[property]
}
}

// set the severityReason if the severity was changed
// 'severity' is not set by default so if it's present then the user must have
// set it in a callback
if (serialisedEvent.severity) {
serialisedEvent.severityReason = { type: 'userCallbackSetSeverity' }
}

return serialisedEvent
}

function deserialiseEvent (json, minidumpPath) {
if (!json || typeof json !== 'object') {
return
}

const event = new Event('Native Crash', 'Event created for a native crash', [], {})

if (hasValueForProperty(json, 'app')) {
event.app = json.app
}

if (hasValueForProperty(json, 'breadcrumbs')) {
event.breadcrumbs = json.breadcrumbs.map(
breadcrumb => new Breadcrumb(
breadcrumb.name,
breadcrumb.metaData,
breadcrumb.type,
new Date(breadcrumb.timestamp)
)
)
}

if (hasValueForProperty(json, 'context')) {
event.context = json.context
}

if (hasValueForProperty(json, 'device')) {
event.device = json.device
}

if (hasValueForProperty(json, 'featureFlags')) {
for (let i = 0; i < json.featureFlags.length; ++i) {
const flag = json.featureFlags[i]

event.addFeatureFlag(flag.featureFlag, flag.variant)
}
}

if (hasValueForProperty(json, 'metadata')) {
event._metadata = json.metadata
}

if (hasValueForProperty(json, 'session')) {
const session = new Session()
session.id = json.session.id
session.startedAt = new Date(json.session.startedAt)
session._handled = json.session.events.handled
session._unhandled = json.session.events.unhandled

event._session = session
}

if (hasValueForProperty(json, 'user')) {
event._user = json.user
}

// this doesn't exist on the Event class, but could be helpful in onSendError
// callbacks as it allows the user to find the related minidump
event.minidumpPath = minidumpPath

return event
}

module.exports = { serialiseEvent, deserialiseEvent }
40 changes: 28 additions & 12 deletions packages/plugin-electron-deliver-minidumps/minidump-loop.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
const { readFile } = require('fs').promises
const runSyncCallbacks = require('@bugsnag/core/lib/sync-callback-runner')
const { serialiseEvent, deserialiseEvent } = require('./event-serialisation')

module.exports = class MinidumpDeliveryLoop {
constructor (sendMinidump, onSend = () => true, minidumpQueue, logger) {
constructor (sendMinidump, onSendError, minidumpQueue, logger) {
this._sendMinidump = sendMinidump
this._onSend = onSend
this._minidumpQueue = minidumpQueue
this._logger = logger
this._running = false

// onSendError can be a function or an array of functions
this._onSendError = typeof onSendError === 'function'
? [onSendError]
: onSendError
}

_onerror (err, minidump) {
Expand All @@ -31,24 +37,34 @@ module.exports = class MinidumpDeliveryLoop {
}

async _deliverMinidump (minidump) {
const event = await this._readEvent(minidump.eventPath)
const shouldSendMinidump = event && await this._onSend(event)
let shouldSendMinidump = true
let eventJson = await this._readEvent(minidump.eventPath)

if (shouldSendMinidump === false) {
this._minidumpQueue.remove(minidump)
this._scheduleSelf()
} else {
if (eventJson && this._onSendError.length > 0) {
const event = deserialiseEvent(eventJson, minidump.minidumpPath)
const ignore = runSyncCallbacks(this._onSendError, event, 'onSendError', this._logger)

// i.e. we SHOULD send the minidump if we SHOULD NOT ignore the event
shouldSendMinidump = !ignore

// reserialise the event for sending in the form payload
eventJson = serialiseEvent(event)
}

if (shouldSendMinidump) {
try {
await this._sendMinidump(minidump.minidumpPath, event)
await this._sendMinidump(minidump.minidumpPath, eventJson)

// if we had a successful delivery - remove the minidump from the queue, and schedule the next
// if we had a successful delivery - remove the minidump from the queue
this._minidumpQueue.remove(minidump)
} catch (e) {
this._onerror(e, minidump)
} finally {
this._scheduleSelf()
}
} else {
this._minidumpQueue.remove(minidump)
}

this._scheduleSelf()
}

async _deliverNextMinidump () {
Expand Down
1 change: 1 addition & 0 deletions packages/plugin-electron-deliver-minidumps/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
},
"files": [
"deliver-minidumps.js",
"event-serialisation.js",
"minidump-loop.js",
"minidump-queue.js",
"send-minidump.js"
Expand Down
Loading
Loading