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

Electron minidumps support #1581

Merged
merged 34 commits into from
Nov 17, 2021
Merged

Electron minidumps support #1581

merged 34 commits into from
Nov 17, 2021

Conversation

imjoehaines
Copy link
Contributor

Merge of the integration branch into next for the upcoming release

lemnik and others added 30 commits July 13, 2021 17:16
* 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`
* 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
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'
@imjoehaines imjoehaines requested a review from lemnik November 11, 2021 16:36
@github-actions
Copy link

@bugsnag/browser bundle size diff

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%)

Generated by 🚫 dangerJS against bb7a427

@tomlongridge tomlongridge merged commit a5d51de into next Nov 17, 2021
@tomlongridge tomlongridge deleted the integration/minidumps branch November 17, 2021 09:45
@djskinner djskinner mentioned this pull request Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants