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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
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
13 changes: 1 addition & 12 deletions .buildkite/browser-pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -244,18 +244,7 @@ steps:
concurrency: 5
concurrency_group: 'browserstack'

- label: ':android: Android 5.0 Browser tests'
timeout_in_minutes: 20
plugins:
docker-compose#v3.9.0:
pull: browser-maze-runner-v6
run: browser-maze-runner-v6
use-aliases: true
command:
- --farm=bs
- --browser=android_s6
concurrency: 5
concurrency_group: 'browserstack'
# Skipping Android 5 due to test environment stability issues

- label: ':android: Android 6.0 Browser tests'
timeout_in_minutes: 20
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/test-electron.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ jobs:
runs-on: ${{ matrix.os }}
strategy:
matrix:
electron: [ '^11.2.0' ]
electron: [ '^12.0.0', '^20.0.0' ]
node-version: [12, 14]
os: [ ubuntu-20.04, windows-2019, macos-10.15 ]
os: [ ubuntu-20.04, windows-2019, macos-11 ]
exclude:
- os: macos-10.15
node-version: 14
Expand Down
9 changes: 8 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
# Changelog

## TBD

### Changed

- (react-native) Update bugsnag-cocoa from v6.23.1 to [v6.24.0](https://github.com/bugsnag/bugsnag-cocoa/blob/master/CHANGELOG.md#6240-2022-10-05)
- (plugin-navigation-breadcrumbs) calling `pushState` or `replaceState` no longer triggers a new session when `autoTrackSessions` is enabled [#1820](https://github.com/bugsnag/bugsnag-js/pull/1820)

## v7.18.0 (2022-09-22)

### Changed

- (react-native) Update bugsnag-cocoa from v6.22.3 to [v6.23.0](https://github.com/bugsnag/bugsnag-cocoa/blob/master/CHANGELOG.md#6230-2022-09-14)
- (react-native) Update bugsnag-cocoa from v6.22.3 to [v6.23.1](https://github.com/bugsnag/bugsnag-cocoa/blob/master/CHANGELOG.md#6231-2022-09-21)
- Added `getFeatureFlags()` to error events [#1815](https://github.com/bugsnag/bugsnag-js/pull/1815)

## v7.17.4 (2022-09-08)
Expand Down
41 changes: 41 additions & 0 deletions packages/browser/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,4 +172,45 @@ describe('browser notifier', () => {
done()
})
})

describe('navigation breadcrumbs', () => {
it('resets events on pushState', () => {
const Bugsnag = getBugsnag()
const client = Bugsnag.createClient('API_KEY')
const resetEventCount = jest.spyOn(client, 'resetEventCount')

window.history.pushState('', '', 'new-url')
expect(resetEventCount).toHaveBeenCalled()

resetEventCount.mockReset()
resetEventCount.mockRestore()
})

it('does not reset events on replaceState', () => {
const Bugsnag = getBugsnag()
const client = Bugsnag.createClient('API_KEY')
const resetEventCount = jest.spyOn(client, 'resetEventCount')

window.history.replaceState('', '', 'new-url')
expect(resetEventCount).not.toHaveBeenCalled()

resetEventCount.mockReset()
resetEventCount.mockRestore()
})

it('does not start unnecessary sessions', () => {
const Bugsnag = getBugsnag()
const client = Bugsnag.createClient('API_KEY')
const startSession = jest.spyOn(client, 'startSession')

window.history.replaceState('', '', 'new-url')
expect(startSession).not.toHaveBeenCalled()

window.history.pushState('', '', 'new-url')
expect(startSession).not.toHaveBeenCalled()

startSession.mockReset()
startSession.mockRestore()
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,8 @@ module.exports = (win = window) => {
}, true)

// the only way to know about replaceState/pushState is to wrap them… >_<

if (win.history.pushState) wrapHistoryFn(client, win.history, 'pushState', win, true)
if (win.history.replaceState) wrapHistoryFn(client, win.history, 'replaceState', win)
if (win.history.pushState) wrapHistoryFn(client, win.history, 'pushState', win)
}
}

Expand Down Expand Up @@ -63,14 +62,12 @@ const stateChangeToMetadata = (win, state, title, url) => {
return { title, state, prevState: getCurrentState(win), to: url || currentPath, from: currentPath }
}

const wrapHistoryFn = (client, target, fn, win) => {
const wrapHistoryFn = (client, target, fn, win, resetEventCount = false) => {
const orig = target[fn]
target[fn] = (state, title, url) => {
client.leaveBreadcrumb(`History ${fn}`, stateChangeToMetadata(win, state, title, url), 'navigation')
// if throttle plugin is in use, reset the event sent count
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

// Internet Explorer will convert `undefined` to a string when passed, causing an unintended redirect
// to '/undefined'. therefore we only pass the url if it's not undefined.
orig.apply(target, [state, title].concat(url !== undefined ? url : []))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,23 +51,6 @@ describe('plugin: navigation breadcrumbs', () => {
expect(c._breadcrumbs.length).toBe(0)
})

it('should start a new session if autoTrackSessions=true', () => {
const { winHandlers, docHandlers, window } = getMockWindow()
const c = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', plugins: [plugin(window)] })
c._sessionDelegate = {
startSession: jest.fn(),
pauseSession: noop,
resumeSession: id
}
winHandlers.load.forEach((h) => h.call(window))
docHandlers.DOMContentLoaded.forEach((h) => h.call(window.document))
window.history.replaceState({}, 'bar', 'network-breadcrumb-test.html')
expect(c._sessionDelegate.startSession).toHaveBeenCalledWith(c, expect.objectContaining({
id: expect.any(String),
startedAt: expect.any(Date)
}))
})

it('should not start a new session if autoTrackSessions=false', () => {
const { winHandlers, docHandlers, window } = getMockWindow()
const c = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', autoTrackSessions: false, plugins: [plugin(window)] })
Expand Down
2 changes: 1 addition & 1 deletion packages/react-native/ios/vendor/bugsnag-cocoa
Submodule bugsnag-cocoa updated 50 files
+2 −2 .jazzy.yaml
+2 −2 Bugsnag.podspec.json
+23 −109 Bugsnag.xcodeproj/project.pbxproj
+26 −3 Bugsnag/BSGCrashSentry.m
+18 −28 Bugsnag/Breadcrumbs/BSGNotificationBreadcrumbs.m
+4 −1 Bugsnag/BugsnagInternals.h
+2 −2 Bugsnag/BugsnagSessionTracker.m
+5 −5 Bugsnag/Client/BugsnagClient.m
+67 −72 Bugsnag/Configuration/BSGConfigurationBuilder.m
+2 −0 Bugsnag/Configuration/BugsnagConfiguration.m
+2 −0 Bugsnag/Delivery/BSGEventUploader.h
+6 −0 Bugsnag/Delivery/BSGEventUploader.m
+0 −2 Bugsnag/Helpers/BSGAppKit.h
+13 −4 Bugsnag/Helpers/BSGDefines.h
+21 −0 Bugsnag/Helpers/BSGFeatureFlagStore.h
+141 −25 Bugsnag/Helpers/BSGFeatureFlagStore.m
+0 −9 Bugsnag/Helpers/BSGKeys.h
+1 −0 Bugsnag/Helpers/BSGTelemetry.m
+0 −2 Bugsnag/Helpers/BSGUIKit.h
+5 −5 Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.m
+3 −1 Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry.c
+3 −0 Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry.h
+8 −0 Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_NSException.m
+3 −1 Bugsnag/KSCrash/Source/KSCrash/Recording/Tools/BSG_KSMachHeaders.c
+1 −1 Bugsnag/Payload/BugsnagEvent.m
+1 −1 Bugsnag/Payload/BugsnagNotifier.m
+27 −0 Bugsnag/include/Bugsnag/BugsnagConfiguration.h
+3 −3 BugsnagNetworkRequestPlugin.podspec.json
+3 −1 BugsnagNetworkRequestPlugin/BugsnagNetworkRequestPlugin.xcodeproj/project.pbxproj
+24 −0 CHANGELOG.md
+1 −1 Framework/Info.plist
+45 −0 Tests/BugsnagTests/BSGConfigurationBuilderTests.m
+67 −3 Tests/BugsnagTests/BSGFeatureFlagStoreTests.m
+2 −2 Tests/BugsnagTests/BugsnagSessionTrackerTest.m
+1 −1 Tests/BugsnagTests/Info.plist
+18 −14 Tests/KSCrashTests/BSG_KSMachHeadersTests.m
+1 −1 Tests/TestHost-iOS/Info.plist
+1 −1 VERSION
+15 −0 features/delivery.feature
+12 −12 features/fixtures/ios/iOSTestApp.xcodeproj/project.pbxproj
+0 −7 features/fixtures/ios/iOSTestApp/iOSTestApp-Bridging-Header.h
+16 −8 features/fixtures/macos/macOSTestApp.xcodeproj/project.pbxproj
+0 −7 features/fixtures/macos/macOSTestApp/macOSTestApp-Bridging-Header.h
+0 −14 features/fixtures/shared/scenarios/AttachCustomStacktraceHook.h
+24 −0 features/fixtures/shared/scenarios/AttemptDeliveryOnCrashScenario.swift
+0 −21 features/fixtures/shared/scenarios/BugsnagHooks.h
+0 −8 features/fixtures/shared/scenarios/CustomPluginNotifierDescriptionScenario.m
+2 −4 features/fixtures/shared/scenarios/Scenario.h
+0 −7 features/fixtures/watchos/watchOSTestApp WatchKit Extension/Swift-Objc-Bridging-Header.h
+12 −4 features/fixtures/watchos/watchOSTestApp.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,22 @@ if [ "$REACT_NATIVE_VERSION" = "rn0.60" ]; then
npm install react-native-reanimated@^1.13 --registry=$REGISTRY_URL
npm install react-native-safe-area-context@^3.1 --registry=$REGISTRY_URL
npm install react-native-screens@^2.18 --registry=$REGISTRY_URL
else
elif [ "$REACT_NATIVE_VERSION" = "rn0.66" ] || [ "$REACT_NATIVE_VERSION" = "rn0.67" ] || [ "$REACT_NATIVE_VERSION" = "rn0.68-hermes" ]; then
npm install @react-native-community/masked-view@^0.1 --registry=$REGISTRY_URL
npm install @react-navigation/native@^6.0 --registry=$REGISTRY_URL
npm install @react-navigation/stack@^6.0 --registry=$REGISTRY_URL
npm install react-native-gesture-handler@^2.2 --registry=$REGISTRY_URL
# gesture-handler locked to avoid Kotlin version conflicts, see "Important changes" at:
# https://github.com/software-mansion/react-native-gesture-handler/releases/tag/2.7.0
npm install [email protected] --registry=$REGISTRY_URL
npm install react-native-reanimated@^1.13 --registry=$REGISTRY_URL
npm install [email protected] --registry=$REGISTRY_URL
npm install [email protected] --registry=$REGISTRY_URL
else
npm install @react-native-community/masked-view@^0.1 --registry=$REGISTRY_URL
npm install @react-navigation/native@^6.0 --registry=$REGISTRY_URL
npm install @react-navigation/stack@^6.0 --registry=$REGISTRY_URL
npm install react-native-gesture-handler@^2.2 --registry=$REGISTRY_URL
npm install react-native-reanimated@^1.13 --registry=$REGISTRY_URL
npm install [email protected] --registry=$REGISTRY_URL
npm install [email protected] --registry=$REGISTRY_URL
fi