Skip to content

[PLAT-8912] Review session definition #1820

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

Merged
merged 18 commits into from
Oct 10, 2022

Conversation

gingerbenw
Copy link
Member

@gingerbenw gingerbenw commented Sep 23, 2022

Goal

To prevent replaceState or pushState from starting a new session. Ensuring Bugsnag sessions in the browser will start when, and only when, a page is loaded and Bugsnag is started.

Changeset

Updated wrapHistoryFn to no longer start a new session
Updated wrapHistoryFn to accept a resetEventCount parameter, enabled on pushState

Testing

Removed tests to check when a new session is started

@github-actions
Copy link

github-actions bot commented Sep 23, 2022

@bugsnag/browser bundle size diff

Minified Minfied + Gzipped
Before 43.19 kB 13.21 kB
After 43.17 kB 13.21 kB
± -19 bytes No change

code coverage diff

Ok File Lines Branches Functions Statements
🔴 /home/runner/work/bugsnag-js/bugsnag-js/packages/plugin-navigation-breadcrumbs/navigation-breadcrumbs.js 85.71%
(-0.34%)
64.29%
(+5.03%)
71.43%
(+0%)
84.31%
(+1.29%)
/home/runner/work/bugsnag-js/bugsnag-js/packages/plugin-simple-throttle/throttle.js 100%
(+0%)
100%
(+0%)
100%
(+20%)
100%
(+8.33%)

Total:

Lines Branches Functions Statements
87.27%(+0%) 75.58%(+0.06%) 86.86%(+0.09%) 86.24%(+0.03%)

Generated by 🚫 dangerJS against 5e64a6e

nickdowell and others added 5 commits September 26, 2022 08:47
This configuration in particular seems to be incredibly unstable, doing
more to reduce confidence in the test suite rather than shore up
confidence in the changes being made. Builds hang after "Starting
Selenium driver" somewhat consistently, then timing out after maximum
wait time.

Some recent examples:

* https://buildkite.com/bugsnag/bugsnag-js-browser/builds/2677#018364be-9339-4b60-ac93-8e6df37ba792
* https://buildkite.com/bugsnag/bugsnag-js-browser/builds/2684#0183667b-b10e-4a10-9390-52e25b3f725f
* https://buildkite.com/bugsnag/bugsnag-js-browser/builds/2689#01837982-63a7-49a1-a76b-95589934c1d9
* https://buildkite.com/bugsnag/bugsnag-js-browser/builds/2691#01837f1f-3fe9-4d06-bd8d-fce070950c7a

Similar issues occur in the Safari 15 build, but given its the only
representation for tests against that browser (and was at least
*somewhat* more stable), did not warrant the same response.
@gingerbenw gingerbenw marked this pull request as ready for review September 29, 2022 11:11
if (typeof client.resetEventCount === 'function') client.resetEventCount()
// if the client is operating in auto session-mode, a new route should trigger a new session
if (client._config.autoTrackSessions) client.startSession()
if (resetEventCount && typeof client.resetEventCount === 'function') client.resetEventCount()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider testing this (at integration level?) or is there already e2e tests

@gingerbenw gingerbenw changed the base branch from next to integration/v8 September 29, 2022 13:50
@gingerbenw gingerbenw merged commit f373d0f into integration/v8 Oct 10, 2022
@gingerbenw gingerbenw mentioned this pull request Aug 29, 2024
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.

6 participants