@financial-times/o-tracking - npm Package Compare versions

Comparing version 4.6.0 to 4.6.1

CHANGELOG.md

 # Changelog
 
+## [4.6.1](https://github.com/Financial-Times/origami/compare/o-tracking-v4.6.0...o-tracking-v4.6.1) (2024-11-18)
+
+
+### Bug Fixes
+
+* correctly detect truly circular paths in JS objects, rather than objects with multiple references ([63f3c0a](https://github.com/Financial-Times/origami/commit/63f3c0a1d6e7fbd0b8734eb56df5c7ca568cb490))
+
 ## [4.6.0](https://github.com/Financial-Times/origami/compare/o-tracking-v4.5.4...o-tracking-v4.6.0) (2024-11-14)
@@ -4,0 +11,0 @@

package.json

 {
   "name": "@financial-times/o-tracking",
-  "version": "4.6.0",
+  "version": "4.6.1",
   "description": "Provides tracking for a product. Tracking requests are sent to the Spoor API.",
@@ -5,0 +5,0 @@ "keywords": [

src/javascript/core/send.js

 import {get as getSetting} from './settings.js';
-import {broadcast, is, findCircularPathsIn, containsCircularPaths, merge, addEvent, log} from '../utils.js';
+import {broadcast, is, safelyStringifyJson, merge, addEvent, log} from '../utils.js';
 import {Queue} from './queue.js';
@@ -71,12 +71,4 @@ import {get as getTransport} from './transports/index.js';
 
-	if (containsCircularPaths(request)) {
-		const errorMessage = "o-tracking does not support circular references in the analytics data.\n" +
-		"Please remove the circular references in the data.\n" +
-		"Here are the paths in the data which are circular:\n" +
-		JSON.stringify(findCircularPathsIn(request), undefined, 4);
-		throw new Error(errorMessage);
-	}
+	const stringifiedData = safelyStringifyJson(request);
 
-	const stringifiedData = JSON.stringify(request);
-
 	transport.complete(function (error) {
@@ -83,0 +75,0 @@ if (is(user_callback, 'function')) {

src/javascript/core/store.js

@@ -1,2 +0,2 @@
-import {broadcast, containsCircularPaths, decode, encode, findCircularPathsIn, is} from '../utils.js';
+import {broadcast, safelyStringifyJson, decode, encode, is} from '../utils.js';
 
@@ -175,10 +175,3 @@ /**
 	} else {
-		if (containsCircularPaths(this.data)) {
-			const errorMessage = "o-tracking does not support circular references in the analytics data.\n" +
-			"Please remove the circular references in the data.\n" +
-			"Here are the paths in the data which are circular:\n" +
-			JSON.stringify(findCircularPathsIn(this.data), undefined, 4);
-			throw new Error(errorMessage);
-		}
-		value = JSON.stringify(this.data);
+		value = safelyStringifyJson(this.data);
 	}
@@ -185,0 +178,0 @@

src/javascript/tracking.js

@@ -20,3 +20,3 @@ import {set, get, destroy as destroySetting} from './core/settings.js';
  */
-const version = '4.6.0';
+const version = '4.6.1';
 
@@ -23,0 +23,0 @@ /**

src/javascript/utils.js

@@ -244,97 +244,128 @@ /**
 
+
 /**
- * Used to find out all the paths which contain a circular reference.
+ * Identify circular references in 'object', and replace them with a string representation
+ * of the reference. Returns a succesfully serialised JSON string, and a list of circular
+ * references which were removed.
+ * 
+ * Inspired by https://github.com/sindresorhus/safe-stringify and 
+ * https://github.com/sindresorhus/decircular
  *
- * @param {*} rootObject The object we want to search within for circular references
- * @returns {string[]} Returns an array of strings, the strings are the full paths to the circular references within the rootObject
+ * @param {*} object The object we want to stringify, and search within for circular references
+ * @returns {Object: {jsonString: string, warnings: array}} The stringified object, and a warnings for each circular reference which was removed
  */
-function findCircularPathsIn(rootObject) {
-	const traversedValues = new WeakSet();
-	const circularPaths = [];
+function removeCircularReferences(object) {
 
-	function _findCircularPathsIn(currentObject, path) {
-		// If we already saw this object
-		// the rootObject contains a circular reference
-		// and we can stop looking any further into this currentObj
-		if (traversedValues.has(currentObject)) {
-			circularPaths.push(path);
-			return;
+	// WeakMaps release memory when all references are garbage-collected
+	const circularReferences = new WeakMap();
+	const paths = new WeakMap();
+
+	const warnings = [];
+
+	function getPathFragment(parent, key) {
+		if (!key) {
+			return '$';
 		}
 
-		// Only Objects and things which inherit from Objects can contain circular references
-		// I.E. string/number/boolean/template literals can not contain circular references
-		if (currentObject instanceof Object) {
-			traversedValues.add(currentObject);
+		if (Array.isArray(parent)) {
+			return `[${key}]`;
+		}
 
-			// Loop through all the values of the current object and search those for circular references
-			for (const [key, value] of Object.entries(currentObject)) {
-				// No need to recurse on every value because only things which inherit
-				// from Objects can contain circular references
-				if (value instanceof Object) {
-					const parentObjectIsAnArray = Array.isArray(currentObject);
-					if (parentObjectIsAnArray) {
-					// Store path in bracket notation when value is an array
-						_findCircularPathsIn(value, `${path}[${key}]`);
-					} else {
-					// Store path in dot-notation when value is an object
-						_findCircularPathsIn(value, `${path}.${key}`);
-					}
-				}
-			}
+		return `.${key}`;
+	}
+
+	function formatCircularReferencesWarning(references) {
+		const paths = references.map(path => '`' + path.join('') + '`');
+		return 'Circular reference between ' + paths.join(' AND ');
+	}
+
+	function replacer(key, value) {
+		// Scalars don't need to be inspected as they can't contain circular references
+		if (!(value !== null && typeof value === 'object')) {
+			return value
 		}
+
+		// Record the path from the root ($) to the current object (value)
+		// in order to print helpful circular reference warnings.
+		const path = [...paths.get(this) || [], getPathFragment(this, key)];
+		paths.set(value, path);
+
+		// If a reference to the current value is already in the list, we have
+		// a circular reference. Add the current value to the list along with its path,
+		// and return a useful error string rather than the unserialisable value.
+		if (circularReferences.has(value)) {
+			const references = [...circularReferences.get(value), path];
+			circularReferences.set(value, references);
+			const warning = formatCircularReferencesWarning(references);
+			warnings.push(warning);
+			return warning;
+		}
+
+		// This is the first time we've seen the current value in this branch 
+		// of the object. Record its path from the object root.
+		circularReferences.set(value, [path]);
+
+		// Recurse into the value to proactively find circular references
+		// before encountering a loop.
+		const newValue = Array.isArray(value) ? [] : {};
+		for (const [k, v] of Object.entries(value)) {
+			newValue[k] = replacer.call(value, k, v);
+		}
+
+		// All circular references to this object will have been identified,
+		// so remove it from the list.
+		circularReferences.delete(value);
+
+		// This branch of the object can now be safely serialised to a JSON string
+		return newValue;
 	}
 
-	_findCircularPathsIn(rootObject, "");
-	return circularPaths;
+	const jsonString = JSON.stringify(object, replacer);
+	return {jsonString, warnings};
 }
 
+
 /**
- * Used to find out whether an object contains a circular reference.
+ * Stringify an object to JSON, removing any circular references. When circular references
+ * are found, an error is thrown in a new event loop so that global error handlers can report it.
  *
- * @param {object} rootObject The object we want to search within for circular references
- * @returns {boolean} Returns true if a circular reference was found, otherwise returns false
+ * @param {*} object The object we want to stringify, and search within for circular references
+ * @returns {string} The safely stringified JSON string
  */
-function containsCircularPaths(rootObject) {
-	// Used to keep track of all the values the rootObject contains
-	const traversedValues = new WeakSet();
+function safelyStringifyJson(object) {
 
-	/**
-	 *
-	 * @param {*} currentObject The current object we want to search within for circular references
-	 * @returns {boolean|undefined} Returns true if a circular reference was found, otherwise returns undefined
-	 */
-	function _containsCircularPaths(currentObject) {
-		// If we already saw this object
-		// the rootObject contains a circular reference
-		// and we can stop looking any further
-		if (traversedValues.has(currentObject)) {
-			return true;
-		}
+	// JSON.stringify throws on two cases:
+	// - value contains a circular reference
+	// - A BigInt value is encountered
+	// Circular references are a real possibility in the way o-tracking is called (and saves a queue of 
+	// messages in a store), so we need to handle those gracefully.
+	// 
+	// However, for performance reasons, we always attempt to do a basic JSON.stringify() first. The 
+	// recursion involved in removeCircularReferences() makes it about 20x slower to stringify a basic payload. 
+	// This performance hit will be exacerbated on slow devices (e.g. old Android phones) with lots of queued offline events.
+	try {
+		return JSON.stringify(object);
 
-		// Only Objects and things which inherit from Objects can contain circular references
-		// I.E. string/number/boolean/template literals can not contain circular references
-		if (currentObject instanceof Object) {
-			traversedValues.add(currentObject);
-			// Loop through all the values of the current object and search those for circular references
-			for (const value of Object.values(currentObject)) {
-				// No need to recurse on every value because only things which inherit
-				// from Objects can contain circular references
-				if (value instanceof Object) {
-					if (_containsCircularPaths(value)) {
-						return true;
-					}
-				}
-			}
+	// NB: error is discarded - we have more work to do in order to throw a useful message
+	} catch (error) {
+	
+		const {jsonString, warnings} = removeCircularReferences(object);
+	
+		if (warnings.length) {
+			// Throw in a new event loop, as we always want to return JSON so the tracking payload is sent
+			setTimeout(() => {
+				const errorMessage = "AssertionError: o-tracking does not support circular references in the analytics data.\n" +
+				"Please remove the circular references in the data.\n" +
+				"Here are the paths in the data which are circular:\n" +
+				warnings.join('\n');
+				throw new Error(errorMessage);
+			});
 		}
+	
+		return jsonString;
 	}
 
-	// _containsCircularPaths returns true or undefined.
-	// By using Boolean we convert the undefined into false.
-	return Boolean(
-		_containsCircularPaths(
-			rootObject
-		)
-	);
-}
 
+};
+
 /**
@@ -428,5 +459,5 @@ * Find out whether two objects are deeply equal to each other.
 	filterProperties,
-	findCircularPathsIn,
-	containsCircularPaths,
+	removeCircularReferences,
+	safelyStringifyJson,
 	isDeepEqual
 };

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

157362

increased by0.44%

Lines of code

2731

increased by0.04%

No dependency changes