fix(ci): stabilize BrowserStack Test workflow

.github/workflows/cross-browser-testing.yml

@@ -4,10 +4,23 @@
 name: 'BrowserStack Test'
 on: [push, pull_request, workflow_dispatch]
 
+# Cancel superseded runs for the same branch to reduce pressure on the
+# limited pool of BrowserStack parallel sessions, which causes queueing,
+# tunnel disconnects, and browser capture timeouts. Use head_ref (PR) or
+# ref_name (push) so the pull_request and push events for one branch share
+# a group instead of running two matrices at once.
+concurrency:
+  group: browserstack-test-${{ github.head_ref || github.ref_name }}
+  cancel-in-progress: true
+
 jobs:
   browserstack-test:
     name: 'BrowserStack Test'
     runs-on: ubuntu-latest
+    # A healthy run takes < 15 minutes. Without a timeout, karma occasionally
+    # waits on half-open BrowserStack tunnel sockets until GitHub's 6 hour
+    # default cancels the job.
+    timeout-minutes: 30
     steps:
 
       - name: 'BrowserStack Env Setup'  # Invokes the setup-env action

src/cookieSyncManager.ts

@@ -156,18 +156,32 @@ export default function CookieSyncManager(
         mpid: MPID,
         cookieSyncDates: CookieSyncDates,
     ): void => {
-        const img = document.createElement('img');
-
-        mpInstance.Logger.verbose(InformationMessages.CookieSync);
-        img.onload = function() {
-            cookieSyncDates[moduleId] = new Date().getTime();
-
-            mpInstance._Persistence.saveUserCookieSyncDatesToPersistence(
-                mpid,
-                cookieSyncDates
+        try {
+            const img = document.createElement('img');
+
+            mpInstance.Logger.verbose(InformationMessages.CookieSync);
+            img.onload = function() {
+                cookieSyncDates[moduleId] = new Date().getTime();
+
+                mpInstance._Persistence.saveUserCookieSyncDatesToPersistence(
+                    mpid,
+                    cookieSyncDates
+                );
+            };
+            img.src = url;
+        } catch (error) {
+            // Pixel URLs come from server-side configuration, and some browsers
+            // (e.g. legacy EdgeHTML) throw synchronously when an invalid URL is
+            // assigned to img.src. Contain the error so a single bad pixel cannot
+            // break public API calls (identify, setConsentState) or prevent the
+            // remaining pixels from syncing.
+            mpInstance.Logger.error(
+                'Error performing cookie sync for module ID ' +
+                    moduleId +
+                    ': ' +
+                    ((error as Error).message || error)
             );
-        };
-        img.src = url;
+        }
     };
 }
 

test/cross-browser-testing/browserstack.karma.config.js

@@ -70,7 +70,23 @@ module.exports = function(config) {
     config.set({
       browserStack: {
         username: process.env.BS_USERNAME,
-        accessKey: process.env.BS_ACCESS_KEY
+        accessKey: process.env.BS_ACCESS_KEY,
+        // In CI, the BrowserStackLocal tunnel is started by the
+        // browserstack/github-actions setup-local step with a unique
+        // local-identifier. Sessions must be pinned to that identifier;
+        // otherwise BrowserStack routes their localhost traffic through an
+        // arbitrary active tunnel for the account, so concurrent workflow
+        // runs hijack each other's tunnels (browsers that never capture,
+        // "ghost" browsers from other runs, and mid-run transport errors).
+        // Locally (no BROWSERSTACK_LOCAL_IDENTIFIER), karma still starts and
+        // owns its own tunnel as before.
+        ...(process.env.BROWSERSTACK_LOCAL_IDENTIFIER
+            ? {
+                  startTunnel: false,
+                  'browserstack.localIdentifier':
+                      process.env.BROWSERSTACK_LOCAL_IDENTIFIER,
+              }
+            : {}),
       },
       autoWatch: false,
       customLaunchers,
@@ -85,6 +101,12 @@ module.exports = function(config) {
       browserConsoleLogOptions,
       client: {
         captureConsole,
+        mocha: {
+          // Increase from the 2 second default: tests on loaded BrowserStack
+          // VMs run slower than locally and time out spuriously. Must exceed
+          // several waitForCondition polls (3s each) in a single spec.
+          timeout: 20000,
+        },
       },
       junitReporter: {
         outputDir: 'reports/',
@@ -95,6 +117,12 @@ module.exports = function(config) {
       // 2000 ms (default) timeout 
       browserDisconnectTimeout: 50000,
       browserDisconnectTolerance: 5,
+      // BrowserStack can queue sessions when parallel slots are busy, so allow
+      // browsers longer than the 60s default to connect before killing the run
+      captureTimeout: 300000,
+      // Tolerate mid-run stalls on slow BrowserStack VMs without treating
+      // them as disconnects (default is 30s of silence)
+      browserNoActivityTimeout: 120000,
       concurrency: 5,
     });
 };

test/jest/cookieSyncManager.spec.ts

@@ -897,6 +897,56 @@ describe('CookieSyncManager', () => {
 
             expect(loggerSpy).toHaveBeenCalledWith('Performing cookie sync');
         });
+
+        it('should log an error instead of throwing when the tracking pixel cannot be created', () => {
+            // Legacy EdgeHTML throws synchronously when an invalid URL is
+            // assigned to img.src. performCookieSync must contain the error so
+            // public API calls (identify, setConsentState) do not throw.
+            const mockImage = {
+                onload: jest.fn(),
+            };
+            Object.defineProperty(mockImage, 'src', {
+                set() {
+                    throw new Error('Invalid argument.');
+                },
+            });
+            jest.spyOn(document, 'createElement').mockReturnValue(
+                mockImage as unknown as HTMLImageElement
+            );
+
+            const errorSpy = jest.fn();
+            const saveSpy = jest.fn();
+
+            const mockMPInstance = ({
+                _Persistence: {
+                    saveUserCookieSyncDatesToPersistence: saveSpy,
+                },
+                Logger: {
+                    verbose: jest.fn(),
+                    error: errorSpy,
+                },
+            } as unknown) as IMParticleWebSDKInstance;
+
+            const cookieSyncManager = new CookieSyncManager(mockMPInstance);
+
+            const cookieSyncDates: CookieSyncDates = {};
+            expect(() =>
+                cookieSyncManager.performCookieSync(
+                    'https://test.com%3Fredirect%3Dhttps%3A%2F%2Fredirect.com',
+                    42,
+                    '1234',
+                    cookieSyncDates,
+                )
+            ).not.toThrow();
+
+            expect(errorSpy).toHaveBeenCalledWith(
+                'Error performing cookie sync for module ID 42: Invalid argument.'
+            );
+
+            // A failed pixel must not record a sync date
+            expect(cookieSyncDates[42]).toBeUndefined();
+            expect(saveSpy).not.toHaveBeenCalled();
+        });
     });
 
     describe('#isLastSyncDateExpired', () => {

test/src/config/utils.js

@@ -604,7 +604,9 @@ var pluses = /\+/g,
     },
     waitForCondition = function async(
         conditionFn,
-        timeout = 200,
+        // 200ms is enough on local machines, but loaded BrowserStack VMs
+        // regularly need longer for mocked async work to settle
+        timeout = 3000,
         interval = 10
     ) {
         return new Promise((resolve, reject) => {

test/src/tests-core-sdk.js

@@ -975,6 +975,12 @@ describe('core SDK', function() {
         // fetching the config is async and we need to wait for it to finish
         mParticle.getInstance()._Store.isInitialized.should.equal(true);
 
+        // The identify call made during init (mocked as a 400 above) must
+        // finish before another identity request is made, otherwise the SDK
+        // rejects the new request as already in flight and this test flakes
+        // on slow browsers (Firefox 51 on BrowserStack).
+        await waitForCondition(hasIdentityCallInflightReturned);
+
         // have to manually call identify although it was called as part of init because we can only mock the server response once
         fetchMockSuccess(urls.identify, {
             mpid: 'MPID1',

test/src/tests-runtimeToBatchEventsDTO.ts

@@ -459,56 +459,70 @@ describe('Old model to batch model conversion', () => {
         expect(event.data.custom_event_type).to.equal('media')
     });
 
-    it('Set width and height to 0 when window is defined but screen is not defined', () => {
-        const originalScreen = window.screen;
-        delete window.screen;
-
-        const sdkEvent: SDKEvent = {
-            EventName: "Pause Event",
-            EventCategory: 8,
-            ExpandedEventCount: 0,
-            EventDataType: 4,
-            EventAttributes: {
-                content_duration: '120000',
-                content_id: "1234567",
-                content_title: "My sweet sweet media",
-                content_type: "Video",
-                media_session_id: "07be2e14-7e05-4053-bcb5-94950365822d",
-                playhead_position: '7023.335999999999',
-                stream_type: "OnDemand",
-            },
-            ConsentState: null,
-            CurrencyCode: null,
-            CustomFlags: {},
-            DataPlan: {},
-            Debug: true,
-            DeviceId: "0edd580e-d887-44e4-89ae-cd65aa0ee933",
-            Location: null,
-            MPID: "-8433569646818451201",
-            OptOut: null,
-            SDKVersion: "2.11.15",
-            SourceMessageId: 'testSMID',
-            SessionId: "64102C03-592F-440D-8BCC-1D27AAA6B188",
-            SessionStartDate: 1603211322698,
-            Timestamp: 1603212299414,
-            ActiveTimeOnSite: 10,
-            UserAttributes: {},
-            UserIdentities: [],
-            IsFirstRun: true,
+    it('Set width and height to 0 when window is defined but screen is not defined', function(this: Mocha.Context) {
+        // On EdgeHTML and IE, window.screen is a non-configurable host object.
+        // Deleting it throws mid-test and leaves window.screen access broken for
+        // the remainder of the run, which silently breaks every subsequent batch
+        // upload (convertEvents reads window.screen). This test covers a pure JS
+        // branch of convertEvents rather than browser behavior, so skipping it on
+        // EdgeHTML/IE does not lose browser-specific coverage.
+        if (/(?:Edge|Trident)\//.test(window.navigator.userAgent)) {
+            this.skip();
         }
 
-        const batch = Converter.convertEvents(
-            '-8433569646818451201',
-            [sdkEvent],
-            window.mParticle.getInstance()
-        );
+        const originalScreen = window.screen;
 
-        expect(batch).to.be.ok;
-        expect(batch.device_info.screen_height).to.equal(0);
-        expect(batch.device_info.screen_width).to.equal(0);
+        try {
+            delete window.screen;
+
+            const sdkEvent: SDKEvent = {
+                EventName: "Pause Event",
+                EventCategory: 8,
+                ExpandedEventCount: 0,
+                EventDataType: 4,
+                EventAttributes: {
+                    content_duration: '120000',
+                    content_id: "1234567",
+                    content_title: "My sweet sweet media",
+                    content_type: "Video",
+                    media_session_id: "07be2e14-7e05-4053-bcb5-94950365822d",
+                    playhead_position: '7023.335999999999',
+                    stream_type: "OnDemand",
+                },
+                ConsentState: null,
+                CurrencyCode: null,
+                CustomFlags: {},
+                DataPlan: {},
+                Debug: true,
+                DeviceId: "0edd580e-d887-44e4-89ae-cd65aa0ee933",
+                Location: null,
+                MPID: "-8433569646818451201",
+                OptOut: null,
+                SDKVersion: "2.11.15",
+                SourceMessageId: 'testSMID',
+                SessionId: "64102C03-592F-440D-8BCC-1D27AAA6B188",
+                SessionStartDate: 1603211322698,
+                Timestamp: 1603212299414,
+                ActiveTimeOnSite: 10,
+                UserAttributes: {},
+                UserIdentities: [],
+                IsFirstRun: true,
+            }
+
+            const batch = Converter.convertEvents(
+                '-8433569646818451201',
+                [sdkEvent],
+                window.mParticle.getInstance()
+            );
 
-        // set screen back on
-        window.screen = originalScreen;
+            expect(batch).to.be.ok;
+            expect(batch.device_info.screen_height).to.equal(0);
+            expect(batch.device_info.screen_width).to.equal(0);
+        } finally {
+            // set screen back on even if an assertion fails so that later
+            // suites are unaffected
+            window.screen = originalScreen;
+        }
     });
 
     it('propagates PageUrl to page_url on the converted event', () => {