diff --git a/src/features/page_view_event/aggregate/index.js b/src/features/page_view_event/aggregate/index.js index 4c41e5404..edf14c566 100644 --- a/src/features/page_view_event/aggregate/index.js +++ b/src/features/page_view_event/aggregate/index.js @@ -115,7 +115,6 @@ export class Aggregate extends AggregateBase { postHarvestCleanup ({ status, responseText, xhr }) { const rumEndTime = now() - this.blocked = true // this prevents harvester from polling this feature's event buffer (DNE) on interval; in other words, harvests will skip PVE if (status >= 400 || status === 0) { warn(18, status) diff --git a/src/features/utils/aggregate-base.js b/src/features/utils/aggregate-base.js index 43f6a5d4a..d7c2711ae 100644 --- a/src/features/utils/aggregate-base.js +++ b/src/features/utils/aggregate-base.js @@ -18,8 +18,7 @@ export class AggregateBase extends FeatureBase { // This switch needs to be after doOnceForAllAggregate which may new sharedAggregator and reset mainAppKey. switch (this.featureName) { - // PVE has no need for eventBuffer, and SessionTrace + Replay have their own storage mechanisms. - case FEATURE_NAMES.pageViewEvent: + // SessionTrace + Replay have their own storage mechanisms. case FEATURE_NAMES.sessionTrace: case FEATURE_NAMES.sessionReplay: break @@ -28,6 +27,9 @@ export class AggregateBase extends FeatureBase { case FEATURE_NAMES.metrics: this.events = agentRef.sharedAggregator break + /** All other features get EventBuffer in the ESM by default. Note: PVE is included here, but event buffer will always be empty so future harvests will still not happen by interval or EOL. + This was necessary to prevent race cond. issues where the event buffer was checked before the feature could "block" itself. + Its easier to just keep an empty event buffer in place. */ default: this.events = new EventStoreManager(agentRef.mainAppKey, 1) break diff --git a/tests/components/page_view_event/aggregate.test.js b/tests/components/page_view_event/aggregate.test.js new file mode 100644 index 000000000..769357a9c --- /dev/null +++ b/tests/components/page_view_event/aggregate.test.js @@ -0,0 +1,32 @@ +import { setupAgent } from '../setup-agent' +import { Instrument as PageViewEvent } from '../../../src/features/page_view_event/instrument' + +let mainAgent, pveAggregate + +beforeAll(async () => { + mainAgent = setupAgent() + mainAgent.info.errorBeacon = undefined // this prevents Harvester from actually running its `send` method +}) +beforeEach(async () => { + const pveInstrument = new PageViewEvent(mainAgent) + await new Promise(process.nextTick) + pveAggregate = pveInstrument.featAggregate +}) + +test('PageViewEvent does not throw on Harvester driven processes', () => { + expect(pveAggregate.blocked).toEqual(false) + expect(() => pveAggregate.makeHarvestPayload(true)).not.toThrow() + + // The following both don't send anything since PVE buffer is meant to stay empty, so they return false. + expect(mainAgent.runtime.harvester.triggerHarvestFor(pveAggregate)).toEqual(false) // mimics what the harvester does on interval + expect(mainAgent.runtime.harvester.triggerHarvestFor(pveAggregate, { isFinalHarvest: true })).toEqual(false) // mimics what the harvester does on EoL + + expect(mainAgent.runtime.harvester.triggerHarvestFor(pveAggregate, { + directSend: { + targetApp: 'someApp', + payload: 'blah' + }, + needResponse: true, + sendEmptyBody: true + })).toEqual(true) // mimics the manual trigger in PVE `sendRum`; this should return true as it actually tries to "send" +}) diff --git a/tests/unit/features/utils/aggregate-base.test.js b/tests/unit/features/utils/aggregate-base.test.js index f6a6721d3..1163f32d2 100644 --- a/tests/unit/features/utils/aggregate-base.test.js +++ b/tests/unit/features/utils/aggregate-base.test.js @@ -150,16 +150,16 @@ test('does not initialized Aggregator more than once with multiple features', as expect(mainAgent.mainAppKey).toBeUndefined() new AggregateBase(mainAgent, FEATURE_NAMES.pageViewEvent) - expect(EventStoreManager).toHaveBeenCalledTimes(1) + expect(EventStoreManager).toHaveBeenCalledTimes(2) // once for runtime.sharedAgg + once for PVE.events expect(EventStoreManager).toHaveBeenCalledWith(mainAgent.mainAppKey, 2) // 2 = initialize EventAggregator expect(mainAgent.mainAppKey).toBeTruthy() expect(mainAgent.sharedAggregator).toBeTruthy() new AggregateBase(mainAgent, FEATURE_NAMES.jserrors) // this feature should be using that same aggregator as its .events - expect(EventStoreManager).toHaveBeenCalledTimes(1) + expect(EventStoreManager).toHaveBeenCalledTimes(2) new AggregateBase(mainAgent, FEATURE_NAMES.pageViewTiming) // PVT should use its own EventStoreManager - expect(EventStoreManager).toHaveBeenCalledTimes(2) + expect(EventStoreManager).toHaveBeenCalledTimes(3) expect(EventStoreManager).toHaveBeenCalledWith(mainAgent.mainAppKey, 1) // 1 = initialize EventBuffer }) @@ -175,10 +175,9 @@ test('does initialize separate Aggregators with multiple agents', async () => { new AggregateBase(mainAgent, FEATURE_NAMES.pageViewEvent) new AggregateBase(mainAgent2, FEATURE_NAMES.pageViewEvent) - expect(EventStoreManager).toHaveBeenCalledTimes(2) - expect(EventStoreManager).not.toHaveBeenCalledWith(expect.any(Object), 1) + expect(EventStoreManager).toHaveBeenCalledTimes(4) // runtime.sharedAgg + PVE.events but multiply by having 2 agents new AggregateBase(mainAgent, FEATURE_NAMES.jserrors) // still does not initialize sharedAgg again on the same agent new AggregateBase(mainAgent2, FEATURE_NAMES.jserrors) - expect(EventStoreManager).toHaveBeenCalledTimes(2) + expect(EventStoreManager).toHaveBeenCalledTimes(4) })