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 app breadcrumb exception #1722

Merged
merged 2 commits into from
Apr 19, 2022

Conversation

imjoehaines
Copy link
Contributor

Goal

To avoid collecting too many breadcrumbs we debounce the moved event handler, but this can cause an exception when closing a window immediately after moving it (i.e. within the ~250ms debounce timer)

This PR protects against this in two ways:

  1. check if the window is destroyed before using it in the moved callback
  2. cancel a queued call to the moved callback when the closed event fires, as this is the point where the browser window is destroyed

The other event listeners should never be called with a destroyed browser window (other than closed) because they are not debounced and so run as soon as the event is emitted

This can happen when closing a window immediately after moving it
(i.e. within the ~250ms debounce timer). We protect against this in
two ways:

1. check if the window is destroyed before using it in the 'moved'
   callback
2. cancel a queued call to the 'moved' callback when the 'closed'
   event fires, as this is the point where the browser window is
   destroyed

The other event listeners should never be called with a destroyed
browser window (other than 'closed') because they are not debounced
and so run as soon as the event is emitted
@github-actions
Copy link

@bugsnag/browser bundle size diff

Minified Minfied + Gzipped
Before 42.31 kB 12.94 kB
After 42.31 kB 12.94 kB
± No change No change

code coverage diff

Ok File Lines Branches Functions Statements
🔴 /home/runner/work/bugsnag-js/bugsnag-js/packages/electron-test-helpers/src/BrowserWindow.ts 90.7%
(-0.73%)
75%
(-3.57%)
100%
(+0%)
91.3%
(-0.81%)

Total:

Lines Branches Functions Statements
81.93%(+0.02%) 71.6%(+0.01%) 83.21%(+0.04%) 81.02%(+0.02%)

Generated by 🚫 dangerJS against 04767f4

@imjoehaines imjoehaines merged commit f487eba into next Apr 19, 2022
@imjoehaines imjoehaines deleted the fix-electron-app-breadcrumb-crash branch April 19, 2022 09:30
@imjoehaines imjoehaines mentioned this pull request May 3, 2022
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.

2 participants