-
Notifications
You must be signed in to change notification settings - Fork 254
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
Electron minidumps support #1581
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* feat(minidump): deliver pending minidumps to configured endipoint * fix: moved sendMinidump to the plugin-electron-deliver-minidumps * fix: added the state-persistence plugin as standard to the electron client for smooth native-crash support * fix: filestore should look in 'pending' for minidump files * fix(electron): minidump-loop retries delivery if possible * fix(electron): native crash reporting can be disabled in config * test(electron): deliver-minidump plugin is not installed when disabled in config
* feat(minidump): deliver pending minidumps to configured endipoint * fix: moved sendMinidump to the plugin-electron-deliver-minidumps * fix: added the state-persistence plugin as standard to the electron client for smooth native-crash support * fix(electron): minidump-loop retries delivery if possible * fix(electron): minidump delivery loop only runs when the network is available * refactor(electron): extract network-status into it's own module * refactor(minidump-loop): network availability watching moved into minidump-loop * chore(electron): improved the dependency structure of the electron-network-status package so that it shouldn't result in duplicate versions being installed * fix(minidump-delivery): corrected the network-aware test behaviour
* fix(minidump): walk the crash-dir tree and find all minidump files regardless of the crash-dir layout * test(minidump): array-to-array comparison for the expected list of minidump files from `FileStore.listMinidumps`
…red minidump endpoint (#1476)
* feat(minidump): deliver pending minidumps to configured endipoint * fix: moved sendMinidump to the plugin-electron-deliver-minidumps * fix: added the state-persistence plugin as standard to the electron client for smooth native-crash support * fix: filestore should look in 'pending' for minidump files * test(electron): integration test for minidump delivery * fix(minidumps): support non mac/win32 platforms crash report directories * fix(minidump): added support for minidumps where the bugsnag_crash_id appears in dumped multipart-forms * build: remove unneeded dependency from test app The app should now receive this dep from the @bugsnag/electron package * fix(electron): detect crashes with a vectored handler on win32 The crash detection mechanism would largely fail to fire due to conflicts between the electron crash handler (breakpad), libuv custom handlers, and the bugsnag handler, which drew the short straw here. Refactored the crash handler to use vectored exception handling on windows and made the "signal_handler" into a generic "crash_handler" interface to accomodate both approaches. * test(minidump): remove `/minidump` suffix from minidump endpoint in world.js * test(minidump): uploaded minidump events are now checked against an expected structure * chore(filestore): code-clarity improvements to the multipart-form decoding in minidump-io Co-authored-by: Delisa Mason <[email protected]>
* fix(minidump): allow `apiKey` to be set by event callbacks * fix(minidump): don't validate event-specific `apiKey`s
…k becomes available (#1494)
7.12.0-rc.0 — Electron minidumps support
* feat(electron): lastRunInfo exposes the exit state of the last application run * test(electron): end2end testing for lastRunInfo * fix(electron): free lastRunInfo memory on NativeClient.uninstall * fix(electron): lastRunInfo is now `null` if not available instead of having a default value
* test(electron): end-to-end tests can await upload counts to ensure sessions are uploaded before crashing the test fixture * test(electron): Electron test fixture can be started offline to block any requests that are sent before the web content is available to the automator * refactor(test/electron): put network steps together * test(electron): verify request counts between actions Refactor the tests to separate the server reachability cases from the network connectivity cases, so now there are one of each scenario: * server and app connected from the start, single crash * no network connection, single crash, connection restored during app lifetime * no network connection, multiple crashes enqueued, send next connected launch * no server available, multiple crashes enqueued, send next connected launch * fix: mark network errors as retryable Unlike failure to write the request body or other logic errors, if the request fails to send, enqueue to be persisted and retried after other enqueued requests. * fix(delivery-electron): ensure error callback not invoked after response Co-authored-by: Delisa Mason <[email protected]> Co-authored-by: Delisa <[email protected]>
* Linux does not detect child process crashes automatically, and does not have enough in-process context to put them in the right place * Windows does not emit child process crash events, except for processes spawned by electron itself * macOS occasionally detects the crash dump too slowly for the test to pass, though it is consistent in local testing
Using app.on('ready') is problematic if Bugsnag is started in a user's app.on('ready') callback — Electron only fires this event once, so our event handler would never be called This is not the intended way to start Bugsnag, because it means all errors that happen before the app is ready will not be reported, but we should do better than silently doing nothing Using the promise-based API will work both when the app is ready and when it's not yet ready
Use `app.whenReady` to start minidump delivery
The minidumps endpoint has to remain optional to avoid a breaking change in the config schema. However, if the endpoint is missing we attempt to start delivery and this eventually triggers an error as the URL is 'undefined'
Don't send minidumps if the endpoint isn't configured
|
Minified | Minfied + Gzipped | |
---|---|---|
Before | 41.54 kB |
12.79 kB |
After | 41.54 kB |
12.79 kB |
± | +5 bytes |
+3 bytes |
code coverage diff
Ok | File | Lines | Branches | Functions | Statements |
---|---|---|---|---|---|
✅ | /home/runner/work/bugsnag-js/bugsnag-js/packages/delivery-electron/delivery.js | 97.53% (+0.13%) |
86.21% (+0%) |
78.26% (+0.99%) |
94.44% (+0.25%) |
🔴 | /home/runner/work/bugsnag-js/bugsnag-js/packages/electron-filestore/filestore.js | 79.49% (-13.37%) |
52.38% (-20.35%) |
84.62% (-6.68%) |
79.49% (-13.37%) |
✅ | /home/runner/work/bugsnag-js/bugsnag-js/packages/electron-network-status/network-status.js | 100% (+100%) |
94.12% (+94.12%) |
100% (+100%) |
100% (+100%) |
🔴 | /home/runner/work/bugsnag-js/bugsnag-js/packages/electron/src/notifier.js | 46.88% (-6.69%) |
31.25% (-4.46%) |
20% (-13.33%) |
44.44% (-5.56%) |
🔴 | /home/runner/work/bugsnag-js/bugsnag-js/packages/plugin-electron-app/app.js | 98.41% (-1.59%) |
86.67% (-1.79%) |
100% (+0%) |
98.51% (-1.49%) |
✅ | /home/runner/work/bugsnag-js/bugsnag-js/packages/plugin-electron-deliver-minidumps/deliver-minidumps.js | 50% (+50%) |
43.75% (+43.75%) |
33.33% (+33.33%) |
50.77% (+50.77%) |
✅ | /home/runner/work/bugsnag-js/bugsnag-js/packages/plugin-electron-deliver-minidumps/minidump-loop.js | 88.37% (+88.37%) |
79.17% (+79.17%) |
91.67% (+91.67%) |
88.89% (+88.89%) |
✅ | /home/runner/work/bugsnag-js/bugsnag-js/packages/plugin-electron-deliver-minidumps/minidump-queue.js | 94.74% (+94.74%) |
100% (+100%) |
100% (+100%) |
95% (+95%) |
✅ | /home/runner/work/bugsnag-js/bugsnag-js/packages/plugin-electron-deliver-minidumps/send-minidump.js | 89.19% (+89.19%) |
100% (+100%) |
100% (+100%) |
89.47% (+89.47%) |
✅ | /home/runner/work/bugsnag-js/bugsnag-js/packages/plugin-electron-session/session.js | 100% (+0%) |
90% (+2.5%) |
90.91% (+7.58%) |
100% (+0%) |
Total:
Lines | Branches | Functions | Statements |
---|---|---|---|
82.05%(-0.4%) | 71.83%(+0.03%) | 83.26%(-0.31%) | 81.1%(-0.34%) |
lemnik
approved these changes
Nov 11, 2021
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Merge of the integration branch into
next
for the upcoming release