chore: ensure data from any previous 'queue-bug' event is included in newer report

Author: georgecrawford

libraries/o-tracking/src/javascript/core/send.js

@@ -137,18 +137,31 @@ function run(callback = function () { /* empty */}) {
 	// Investigate queue lengths bug
 	// https://jira.ft.com/browse/DTP-330
 	const all_events = queue.all();
+	let queue_length = all_events.length;
 
-	if (all_events.length > 200) {
+	if (queue_length > 200) {
 		const counts = {};
 
-		all_events.forEach(function (event) {
-			const label = [event.category, event.action].join(':');
-
-			if (!Object.prototype.hasOwnProperty.call(counts, label)) {
-				counts[label] = 0;
+		all_events.forEach(function ({
+			category,
+			action,
+			context: {counts: event_counts = {}} = {},
+		}) {
+			// If a previously-queued event was a 'queue-bug' summary,
+			// extract the information and add to the current context
+			if (category === 'o-tracking' && action === 'queue-bug') {
+				for (const [label, count] of Object.entries(event_counts)) {
+					counts[label] = (counts[label] || 0) + count;
+					queue_length += count;
+				}
+
+				// Data from this old 'queue-bug' event has been exported,
+				// so the event can now be forgotten
+				queue_length -= 1;
+			} else {
+				const label = [category, action].join(':');
+				counts[label] = (counts[label] || 0) + 1;
 			}
-
-			counts[label] += 1;
 		});
 
 		queue.replace([]);
@@ -158,10 +171,10 @@ function run(callback = function () { /* empty */}) {
 			action: 'queue-bug',
 			context: {
 				url: document.URL,
-				queue_length: all_events.length,
+				queue_length,
 				counts: counts,
-				storage: queue.storage.storage._type
-			}
+				storage: queue.storage.storage._type,
+			},
 		});
 	}
 
@@ -210,9 +223,4 @@ function init() {
 	return queue;
 }
 
-export {
-	init,
-	add,
-	run,
-	addAndRun
-};
+export {init, addAndRun};

libraries/o-tracking/test/core.test.js

@@ -5,7 +5,7 @@ import sinon from 'sinon/pkg/sinon-esm.js';
 import {set, destroy} from '../src/javascript/core/settings.js';
 import {Queue} from '../src/javascript/core/queue.js';
 import {session, init} from '../src/javascript/core/session.js';
-import {init as initSend, run} from '../src/javascript/core/send.js';
+import {init as initSend} from '../src/javascript/core/send.js';
 import core from '../src/javascript/core.js';
 import { errorNextSend } from './setup.js';
 
@@ -100,7 +100,7 @@ describe('Core', function () {
 			proclaim.equal(callback.called, 1, 'Callback called once.');
 
 			// Try again
-			run();
+			initSend();
 
 			proclaim.ok(callback.calledOnce, 'Callback should only be called once as next send could be on a different page.');
 		});

libraries/o-tracking/test/core/send.test.js

@@ -4,12 +4,12 @@ import proclaim from 'proclaim';
 import sinon from 'sinon/pkg/sinon-esm.js';
 import {
 	init,
-	add,
 	addAndRun
 } from '../../src/javascript/core/send.js';
 import {Queue} from "../../src/javascript/core/queue.js";
 import {unmockTransport, mockTransport} from '../setup.js';
 import {set, destroy} from '../../src/javascript/core/settings.js';
+import {mock} from '../../src/javascript/core/transports/index.js';
 
 const request = {
 	id: '1.199.83760034665465.1432907605043.-56cf00f',
@@ -51,13 +51,6 @@ describe('Core.Send', function () {
 		});
 	});
 
-	it('should add a request', function () {
-		proclaim.doesNotThrow(function () {
-			add(request);
-		});
-	});
-
-
 	describe('fallback transports', function () {
 		before(function () {
 			unmockTransport();
@@ -252,13 +245,18 @@ describe('Core.Send', function () {
 	});
 
 	describe('queue bug', function () {
+		let queue;
 		let transport;
+		let transportError = false;
 
 		beforeEach(function () {
+			set('config', {queue: true});
+			queue = init();
+
 			transport = {
 				send: sinon.spy(),
 				complete: function (callback) {
-					callback();
+					callback(transportError);
 				},
 			};
 
@@ -267,28 +265,30 @@ describe('Core.Send', function () {
 			};
 		});
 
-		it('should cope with the huge queue bug', function (done) {
-			let queue = new Queue('requests');
+		afterEach(function () {
+			queue.storage.destroy();
+		});
 
-			queue.replace([]);
+		it('should summarise large numbers of offline events to avoid the huge queue bug', function () {
+			// Simulate offline
+			transportError = true;
 
 			for (let i = 0; i < 200; i++) {
-				queue.add({
+				addAndRun({
 					category: 'page',
 					action: 'view',
 				});
 			}
 
-			queue.add({
+			// Go online, and send the events with a clean sinon.spy history
+			transportError = false;
+			transport.send.resetHistory();
+
+			addAndRun({
 				category: 'button',
 				action: 'click',
 			});
 
-			queue.save();
-
-			// Run the queue
-			init();
-
 			// Only one event should be sent
 			proclaim.equal(transport.send.callCount, 1);
 
@@ -300,6 +300,59 @@ describe('Core.Send', function () {
 				'page:view': 200,
 				'button:click': 1,
 			});
+
+			proclaim.equal(queue.all().length, 0);
+		});
+
+		it('should summarise all offline events rather than discard previous summaries', function () {
+			// Simulate offline
+			transportError = true;
+
+			// queue up 200 events
+			for (let i = 0; i < 200; i++) {
+				addAndRun({
+					category: 'page',
+					action: 'view',
+				});
+			}
+
+			// try - and fail - to send the events
+			addAndRun({
+				category: 'button',
+				action: 'click',
+			});
+
+			// queue up 200 more events
+			for (let i = 0; i < 200; i++) {
+				addAndRun({
+					category: 'page',
+					action: 'view',
+				});
+			}
+
+			// Go online, and send the events with a clean sinon.spy history
+			transportError = false;
+			transport.send.resetHistory();
+
+
+			addAndRun({
+				category: 'button',
+				action: 'click',
+			});
+
+			// Two send events should have been attempted
+			proclaim.equal(transport.send.callCount, 2);
+
+			const payload = JSON.parse(transport.send.secondCall.args[1]);
+			proclaim.equal(payload.category, 'o-tracking');
+			proclaim.equal(payload.action, 'queue-bug');
+			proclaim.equal(payload.context.queue_length, 401);
+			proclaim.deepEqual(payload.context.counts, {
+				'page:view': 400,
+				'button:click': 2,
+			});
+
+			proclaim.equal(queue.all().length, 0);
 		});
 	});
 });