From 15ff3e63995e636ed07ae4a2ce335c1b464b9700 Mon Sep 17 00:00:00 2001 From: mohamed amr Date: Thu, 26 Jan 2017 19:08:06 +0200 Subject: [PATCH] feat(minErr): set max depth for angular error JSON stringify Closes #15402 Closes #15433 --- src/.eslintrc.json | 3 ++ src/Angular.js | 72 +++++++++++++++++++++++++++++++++++++------ src/AngularPublic.js | 1 + src/minErr.js | 23 +++++++------- src/stringify.js | 12 ++++++-- test/.eslintrc.json | 3 ++ test/AngularSpec.js | 24 +++++++++++++++ test/minErrSpec.js | 38 +++++++++++++++++++++++ test/stringifySpec.js | 37 ++++++++++++++++++++++ 9 files changed, 189 insertions(+), 24 deletions(-) diff --git a/src/.eslintrc.json b/src/.eslintrc.json index dc505098cfed..1b3b837bf457 100644 --- a/src/.eslintrc.json +++ b/src/.eslintrc.json @@ -15,6 +15,9 @@ "splice": false, "push": false, "toString": false, + "minErrConfig": false, + "errorHandlingConfig": false, + "isValidObjectMaxDepth": false, "ngMinErr": false, "_angular": false, "angularModule": false, diff --git a/src/Angular.js b/src/Angular.js index d4088ad35227..cb7875a043bd 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -10,6 +10,9 @@ splice, push, toString, + minErrConfig, + errorHandlingConfig, + isValidObjectMaxDepth, ngMinErr, angularModule, uid, @@ -125,6 +128,50 @@ var VALIDITY_STATE_PROPERTY = 'validity'; var hasOwnProperty = Object.prototype.hasOwnProperty; +var minErrConfig = { + objectMaxDepth: 5 +}; + + /** + * @ngdoc function + * @name angular.errorHandlingConfig + * @module ng + * @kind function + * + * @description + * Configure several aspects of error handling in AngularJS if used as a setter or return the + * current configuration if used as a getter. The following options are supported: + * + * - **objectMaxDepth**: The maximum depth to which objects are traversed when stringified for error messages. + * + * Omitted or undefined options will leave the corresponding configuration values unchanged. + * + * @param {Object=} config - The configuration object. May only contain the options that need to be + * updated. Supported keys: + * + * * `objectMaxDepth` **{Number}** - The max depth for stringifying objects. Setting to a + * non-positive or non-numeric value, removes the max depth limit. + * Default: 5 + */ +function errorHandlingConfig(config) { + if (isObject(config)) { + if (isDefined(config.objectMaxDepth)) { + minErrConfig.objectMaxDepth = isValidObjectMaxDepth(config.objectMaxDepth) ? config.objectMaxDepth : NaN; + } + } else { + return minErrConfig; + } +} + +/** + * @private + * @param {Number} maxDepth + * @return {boolean} + */ +function isValidObjectMaxDepth(maxDepth) { + return isNumber(maxDepth) && maxDepth > 0; +} + /** * @ngdoc function * @name angular.lowercase @@ -847,9 +894,10 @@ function arrayRemove(array, value) { */ -function copy(source, destination) { +function copy(source, destination, maxDepth) { var stackSource = []; var stackDest = []; + maxDepth = isValidObjectMaxDepth(maxDepth) ? maxDepth : NaN; if (destination) { if (isTypedArray(destination) || isArrayBuffer(destination)) { @@ -872,35 +920,39 @@ function copy(source, destination) { stackSource.push(source); stackDest.push(destination); - return copyRecurse(source, destination); + return copyRecurse(source, destination, maxDepth); } - return copyElement(source); + return copyElement(source, maxDepth); - function copyRecurse(source, destination) { + function copyRecurse(source, destination, maxDepth) { + maxDepth--; + if (maxDepth < 0) { + return '...'; + } var h = destination.$$hashKey; var key; if (isArray(source)) { for (var i = 0, ii = source.length; i < ii; i++) { - destination.push(copyElement(source[i])); + destination.push(copyElement(source[i], maxDepth)); } } else if (isBlankObject(source)) { // createMap() fast path --- Safe to avoid hasOwnProperty check because prototype chain is empty for (key in source) { - destination[key] = copyElement(source[key]); + destination[key] = copyElement(source[key], maxDepth); } } else if (source && typeof source.hasOwnProperty === 'function') { // Slow path, which must rely on hasOwnProperty for (key in source) { if (source.hasOwnProperty(key)) { - destination[key] = copyElement(source[key]); + destination[key] = copyElement(source[key], maxDepth); } } } else { // Slowest path --- hasOwnProperty can't be called as a method for (key in source) { if (hasOwnProperty.call(source, key)) { - destination[key] = copyElement(source[key]); + destination[key] = copyElement(source[key], maxDepth); } } } @@ -908,7 +960,7 @@ function copy(source, destination) { return destination; } - function copyElement(source) { + function copyElement(source, maxDepth) { // Simple values if (!isObject(source)) { return source; @@ -937,7 +989,7 @@ function copy(source, destination) { stackDest.push(destination); return needsRecurse - ? copyRecurse(source, destination) + ? copyRecurse(source, destination, maxDepth) : destination; } diff --git a/src/AngularPublic.js b/src/AngularPublic.js index b1b5f332ce8a..c77eb5b583ff 100644 --- a/src/AngularPublic.js +++ b/src/AngularPublic.js @@ -126,6 +126,7 @@ var version = { function publishExternalAPI(angular) { extend(angular, { + 'errorHandlingConfig': errorHandlingConfig, 'bootstrap': bootstrap, 'copy': copy, 'extend': extend, diff --git a/src/minErr.js b/src/minErr.js index 876c83003fa7..1332b1d190d2 100644 --- a/src/minErr.js +++ b/src/minErr.js @@ -35,18 +35,20 @@ function minErr(module, ErrorConstructor) { return function() { var SKIP_INDEXES = 2; - var templateArgs = arguments, - code = templateArgs[0], + var code = arguments[0], message = '[' + (module ? module + ':' : '') + code + '] ', - template = templateArgs[1], - paramPrefix, i; + template = arguments[1], + paramPrefix, i, + templateArgs = sliceArgs(arguments, SKIP_INDEXES).map(function(arg) { + return toDebugString(arg, minErrConfig.objectMaxDepth); + }); + message += template.replace(/\{\d+\}/g, function(match) { - var index = +match.slice(1, -1), - shiftedIndex = index + SKIP_INDEXES; + var index = +match.slice(1, -1); - if (shiftedIndex < templateArgs.length) { - return toDebugString(templateArgs[shiftedIndex]); + if (index < templateArgs.length) { + return templateArgs[index]; } return match; @@ -55,9 +57,8 @@ function minErr(module, ErrorConstructor) { message += '\nhttp://errors.angularjs.org/"NG_VERSION_FULL"/' + (module ? module + '/' : '') + code; - for (i = SKIP_INDEXES, paramPrefix = '?'; i < templateArgs.length; i++, paramPrefix = '&') { - message += paramPrefix + 'p' + (i - SKIP_INDEXES) + '=' + - encodeURIComponent(toDebugString(templateArgs[i])); + for (i = 0, paramPrefix = '?'; i < templateArgs.length; i++, paramPrefix = '&') { + message += paramPrefix + 'p' + i + '=' + encodeURIComponent(templateArgs[i]); } return new ErrorConstructor(message); diff --git a/src/stringify.js b/src/stringify.js index ce07dffa4092..c33aabb48c88 100644 --- a/src/stringify.js +++ b/src/stringify.js @@ -2,9 +2,15 @@ /* global toDebugString: true */ -function serializeObject(obj) { +function serializeObject(obj, maxDepth) { var seen = []; + // There is no direct way to stringify object until reaching a specific depth + // and a very deep object can cause a performance issue, so we copy the object + // based on this specific depth and then stringify it. + if (isValidObjectMaxDepth(maxDepth)) { + obj = copy(obj, null, maxDepth); + } return JSON.stringify(obj, function(key, val) { val = toJsonReplacer(key, val); if (isObject(val)) { @@ -17,13 +23,13 @@ function serializeObject(obj) { }); } -function toDebugString(obj) { +function toDebugString(obj, maxDepth) { if (typeof obj === 'function') { return obj.toString().replace(/ \{[\s\S]*$/, ''); } else if (isUndefined(obj)) { return 'undefined'; } else if (typeof obj !== 'string') { - return serializeObject(obj); + return serializeObject(obj, maxDepth); } return obj; } diff --git a/test/.eslintrc.json b/test/.eslintrc.json index 62f21300db30..96a109483038 100644 --- a/test/.eslintrc.json +++ b/test/.eslintrc.json @@ -25,6 +25,8 @@ /* angular.js */ "angular": false, + "minErrConfig": false, + "errorHandlingConfig": false, "msie": false, "jqLite": false, "jQuery": false, @@ -37,6 +39,7 @@ "nodeName_": false, "uid": false, "toDebugString": false, + "serializeObject": false, "lowercase": false, "uppercase": false, diff --git a/test/AngularSpec.js b/test/AngularSpec.js index 9e9bbfea6a98..9d334633709b 100644 --- a/test/AngularSpec.js +++ b/test/AngularSpec.js @@ -602,6 +602,30 @@ describe('angular', function() { expect(copy(new Number(NaN)).valueOf()).toBeNaN(); /* eslint-enable */ }); + + it('should copy source until reaching a given max depth', function() { + var source = {a1: 1, b1: {b2: {b3: 1}}, c1: [1, {c2: 1}], d1: {d2: 1}}; + var dest; + + dest = copy(source, {}, 1); + expect(dest).toEqual({a1:1, b1:'...', c1:'...', d1:'...'}); + + dest = copy(source, {}, 2); + expect(dest).toEqual({a1:1, b1:{b2:'...'}, c1:[1,'...'], d1:{d2:1}}); + + dest = copy(source, {}, 3); + expect(dest).toEqual({a1: 1, b1: {b2: {b3: 1}}, c1: [1, {c2: 1}], d1: {d2: 1}}); + + dest = copy(source, {}, 4); + expect(dest).toEqual({a1: 1, b1: {b2: {b3: 1}}, c1: [1, {c2: 1}], d1: {d2: 1}}); + }); + + they('should copy source and ignore max depth when maxDepth = $prop', [NaN, null, undefined, true, false, -1, 0], + function(maxDepth) { + var source = {a1: 1, b1: {b2: {b3: 1}}, c1: [1, {c2: 1}], d1: {d2: 1}}; + var dest = copy(source, {}, maxDepth); + expect(dest).toEqual({a1: 1, b1: {b2: {b3: 1}}, c1: [1, {c2: 1}], d1: {d2: 1}}); + }); }); describe('extend', function() { diff --git a/test/minErrSpec.js b/test/minErrSpec.js index 675c6e40fede..59cbc2414e0b 100644 --- a/test/minErrSpec.js +++ b/test/minErrSpec.js @@ -9,6 +9,11 @@ describe('minErr', function() { var emptyTestError = minErr(), testError = minErr('test'); + var originalObjectMaxDepthInErrorMessage = minErrConfig.objectMaxDepth; + afterEach(function() { + minErrConfig.objectMaxDepth = originalObjectMaxDepthInErrorMessage; + }); + it('should return an Error factory', function() { var myError = testError('test', 'Oops'); expect(myError instanceof Error).toBe(true); @@ -68,6 +73,39 @@ describe('minErr', function() { expect(myError.message).toMatch(/a is {"b":{"a":"..."}}/); }); + it('should handle arguments that are objects with max depth', function() { + var a = {b: {c: {d: {e: {f: {g: 1}}}}}}; + + var myError = testError('26', 'a when objectMaxDepth is default=5 is {0}', a); + expect(myError.message).toMatch(/a when objectMaxDepth is default=5 is {"b":{"c":{"d":{"e":{"f":"..."}}}}}/); + expect(errorHandlingConfig().objectMaxDepth).toBe(5); + + errorHandlingConfig({objectMaxDepth: 1}); + myError = testError('26', 'a when objectMaxDepth is set to 1 is {0}', a); + expect(myError.message).toMatch(/a when objectMaxDepth is set to 1 is {"b":"..."}/); + expect(errorHandlingConfig().objectMaxDepth).toBe(1); + + errorHandlingConfig({objectMaxDepth: 2}); + myError = testError('26', 'a when objectMaxDepth is set to 2 is {0}', a); + expect(myError.message).toMatch(/a when objectMaxDepth is set to 2 is {"b":{"c":"..."}}/); + expect(errorHandlingConfig().objectMaxDepth).toBe(2); + + errorHandlingConfig({objectMaxDepth: undefined}); + myError = testError('26', 'a when objectMaxDepth is set to undefined is {0}', a); + expect(myError.message).toMatch(/a when objectMaxDepth is set to undefined is {"b":{"c":"..."}}/); + expect(errorHandlingConfig().objectMaxDepth).toBe(2); + }); + + they('should handle arguments that are objects and ignore max depth when objectMaxDepth = $prop', + [NaN, null, true, false, -1, 0], function(maxDepth) { + var a = {b: {c: {d: 1}}}; + + errorHandlingConfig({objectMaxDepth: maxDepth}); + var myError = testError('26', 'a is {0}', a); + expect(myError.message).toMatch(/a is {"b":{"c":{"d":1}}}/); + expect(errorHandlingConfig().objectMaxDepth).toBeNaN(); + }); + it('should preserve interpolation markers when fewer arguments than needed are provided', function() { // this way we can easily see if we are passing fewer args than needed diff --git a/test/stringifySpec.js b/test/stringifySpec.js index d971acf7737b..33361002a3bf 100644 --- a/test/stringifySpec.js +++ b/test/stringifySpec.js @@ -12,4 +12,41 @@ describe('toDebugString', function() { expect(toDebugString(a)).toEqual('{"a":"..."}'); expect(toDebugString([a,a])).toEqual('[{"a":"..."},"..."]'); }); + + it('should convert its argument that are objects to string based on maxDepth', function() { + var a = {b: {c: {d: 1}}}; + expect(toDebugString(a, 1)).toEqual('{"b":"..."}'); + expect(toDebugString(a, 2)).toEqual('{"b":{"c":"..."}}'); + expect(toDebugString(a, 3)).toEqual('{"b":{"c":{"d":1}}}'); + }); + + they('should convert its argument that object to string and ignore max depth when maxDepth = $prop', + [NaN, null, undefined, true, false, -1, 0], function(maxDepth) { + var a = {b: {c: {d: 1}}}; + expect(toDebugString(a, maxDepth)).toEqual('{"b":{"c":{"d":1}}}'); + }); +}); + +describe('serializeObject', function() { + it('should convert its argument to a string', function() { + expect(serializeObject({a:{b:'c'}})).toEqual('{"a":{"b":"c"}}'); + + var a = { }; + a.a = a; + expect(serializeObject(a)).toEqual('{"a":"..."}'); + expect(serializeObject([a,a])).toEqual('[{"a":"..."},"..."]'); + }); + + it('should convert its argument that are objects to string based on maxDepth', function() { + var a = {b: {c: {d: 1}}}; + expect(serializeObject(a, 1)).toEqual('{"b":"..."}'); + expect(serializeObject(a, 2)).toEqual('{"b":{"c":"..."}}'); + expect(serializeObject(a, 3)).toEqual('{"b":{"c":{"d":1}}}'); + }); + + they('should convert its argument that object to string and ignore max depth when maxDepth = $prop', + [NaN, null, undefined, true, false, -1, 0], function(maxDepth) { + var a = {b: {c: {d: 1}}}; + expect(serializeObject(a, maxDepth)).toEqual('{"b":{"c":{"d":1}}}'); + }); });