From bc09c31c428d3138bbe1d058df73fb1cc1d479cc Mon Sep 17 00:00:00 2001 From: Abdulrahman Almkheibar Date: Mon, 16 Oct 2017 16:02:42 +0200 Subject: [PATCH 1/2] feat(errorHandlingConfig): add an option so that the parameters of URL error is truncated In Angular.js the error in loading a module will lead to a failur in loading the dependent modules. In turn, this leads to a chain of errors in loading those modules. For some reason, angularjs is copying the error stack and passes it to the next error. This stack is encoded in URL error,so we end up sometimes with so long Error URL when we have may dependent modules. This option disables encoding the parameters in URL error: angular.errorHandlingConfig({isUrlParameter:false}); closes #14744 --- src/minErr.js | 16 +++++++++--- test/minErrSpec.js | 61 ++++++++++++++++++++++++++++++++++------------ 2 files changed, 57 insertions(+), 20 deletions(-) diff --git a/src/minErr.js b/src/minErr.js index e20040319222..d636d171984c 100644 --- a/src/minErr.js +++ b/src/minErr.js @@ -7,7 +7,8 @@ */ var minErrConfig = { - objectMaxDepth: 5 + objectMaxDepth: 5, + isUrlParameters:true }; /** @@ -30,12 +31,17 @@ var minErrConfig = { * * `objectMaxDepth` **{Number}** - The max depth for stringifying objects. Setting to a * non-positive or non-numeric value, removes the max depth limit. * Default: 5 + * * `isUrlParameters` **{Boolean}** - Specifies wether the generated url error will contain the paramters or not. + * Default: true */ function errorHandlingConfig(config) { if (isObject(config)) { if (isDefined(config.objectMaxDepth)) { minErrConfig.objectMaxDepth = isValidObjectMaxDepth(config.objectMaxDepth) ? config.objectMaxDepth : NaN; } + if (isDefined(config.isUrlParameters) && isBoolean(config.isUrlParameters)) { + minErrConfig.isUrlParameters = config.isUrlParameters; + } } else { return minErrConfig; } @@ -50,6 +56,7 @@ function isValidObjectMaxDepth(maxDepth) { return isNumber(maxDepth) && maxDepth > 0; } + /** * @description * @@ -103,9 +110,10 @@ function minErr(module, ErrorConstructor) { message += '\nhttp://errors.angularjs.org/"NG_VERSION_FULL"/' + (module ? module + '/' : '') + code; - - for (i = 0, paramPrefix = '?'; i < templateArgs.length; i++, paramPrefix = '&') { - message += paramPrefix + 'p' + i + '=' + encodeURIComponent(templateArgs[i]); + if (minErrConfig.isUrlParameters) { + for (i = 0, paramPrefix = '?'; i < templateArgs.length; i++, paramPrefix = '&') { + message += paramPrefix + 'p' + i + '=' + encodeURIComponent(templateArgs[i]); + } } return new ErrorConstructor(message); diff --git a/test/minErrSpec.js b/test/minErrSpec.js index aae001cba415..201356c5605f 100644 --- a/test/minErrSpec.js +++ b/test/minErrSpec.js @@ -2,32 +2,53 @@ describe('errors', function() { var originalObjectMaxDepthInErrorMessage = minErrConfig.objectMaxDepth; - + var originalIsUrlParameters = minErrConfig.isUrlParameters; afterEach(function() { minErrConfig.objectMaxDepth = originalObjectMaxDepthInErrorMessage; + minErrConfig.isUrlParameters = originalIsUrlParameters; }); describe('errorHandlingConfig', function() { - it('should get default objectMaxDepth', function() { - expect(errorHandlingConfig().objectMaxDepth).toBe(5); + describe('objectMaxDepth',function() { + it('should get default objectMaxDepth', function() { + expect(errorHandlingConfig().objectMaxDepth).toBe(5); + }); + + it('should set objectMaxDepth', function() { + errorHandlingConfig({objectMaxDepth: 3}); + expect(errorHandlingConfig().objectMaxDepth).toBe(3); + }); + + it('should not change objectMaxDepth when undefined is supplied', function() { + errorHandlingConfig({objectMaxDepth: undefined}); + expect(errorHandlingConfig().objectMaxDepth).toBe(originalObjectMaxDepthInErrorMessage); + }); + + they('should set objectMaxDepth to NaN when $prop is supplied', + [NaN, null, true, false, -1, 0], function(maxDepth) { + errorHandlingConfig({objectMaxDepth: maxDepth}); + expect(errorHandlingConfig().objectMaxDepth).toBeNaN(); + } + ); }); - it('should set objectMaxDepth', function() { - errorHandlingConfig({objectMaxDepth: 3}); - expect(errorHandlingConfig().objectMaxDepth).toBe(3); - }); - it('should not change objectMaxDepth when undefined is supplied', function() { - errorHandlingConfig({objectMaxDepth: undefined}); - expect(errorHandlingConfig().objectMaxDepth).toBe(originalObjectMaxDepthInErrorMessage); + describe('isUrlParameters',function() { + it('should get default isUrlParameters', function() { + expect(errorHandlingConfig().isUrlParameters).toBe(true); + }); + it('should set isUrlParameters', function() { + errorHandlingConfig({isUrlParameters:false}); + expect(errorHandlingConfig().isUrlParameters).toBe(false); + errorHandlingConfig({isUrlParameters:true}); + expect(errorHandlingConfig().isUrlParameters).toBe(true); + }); + it('should not change its value when non-boolean is supplied', function() { + errorHandlingConfig({isUrlParameters:123}); + expect(errorHandlingConfig().isUrlParameters).toBe(originalIsUrlParameters); + }); }); - they('should set objectMaxDepth to NaN when $prop is supplied', - [NaN, null, true, false, -1, 0], function(maxDepth) { - errorHandlingConfig({objectMaxDepth: maxDepth}); - expect(errorHandlingConfig().objectMaxDepth).toBeNaN(); - } - ); }); describe('minErr', function() { @@ -164,5 +185,13 @@ describe('errors', function() { expect(testError('acode', 'aproblem', 'a', 'b', 'value with space').message) .toMatch(/^[\s\S]*\?p0=a&p1=b&p2=value%20with%20space$/); }); + + it('should not generate URL query parameters when isUrlParameters is false', function() { + + errorHandlingConfig({isUrlParameters:false}); + expect(testError('acode', 'aproblem', 'a', 'b', 'c').message) + .not.toContain('?p0=a&p1=b&p2=c'); + }); + }); }); From b06176d3705c7779e720b2b314cd278fb9872915 Mon Sep 17 00:00:00 2001 From: Abdulrahman Almkheibar Date: Wed, 25 Oct 2017 14:01:31 +0200 Subject: [PATCH 2/2] feat(loadModules) : options to customize raising errors When some error occurs while loading a module, an error will raise containing the original error stack. Sometimes the final error contains a lot of stacks which make it difficult to read. closes #14744 --- src/.eslintrc.json | 3 ++- src/Angular.js | 2 +- src/auto/injector.js | 39 ++++++++++++++++++++-------------- src/minErr.js | 29 ++++++++++++++++++-------- test/.eslintrc.json | 2 +- test/auto/injectorSpec.js | 34 ++++++++++++++++++++++++++++++ test/minErrSpec.js | 44 +++++++++++++++++++++++++++++++++------ 7 files changed, 119 insertions(+), 34 deletions(-) diff --git a/src/.eslintrc.json b/src/.eslintrc.json index 64e8b866e6c8..a04eb9e58aa8 100644 --- a/src/.eslintrc.json +++ b/src/.eslintrc.json @@ -15,8 +15,9 @@ "splice": false, "push": false, "toString": false, - "minErrConfig": false, + "errConfigObj": false, "errorHandlingConfig": false, + "loadModulesErrorConfig":false, "isValidObjectMaxDepth": false, "ngMinErr": false, "_angular": false, diff --git a/src/Angular.js b/src/Angular.js index 7c424897ff18..8bd90eeaf172 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -10,7 +10,7 @@ splice, push, toString, - minErrConfig, + errConfigObj, errorHandlingConfig, isValidObjectMaxDepth, ngMinErr, diff --git a/src/auto/injector.js b/src/auto/injector.js index f41d1e8c8e9a..21dea72ee111 100644 --- a/src/auto/injector.js +++ b/src/auto/injector.js @@ -830,8 +830,7 @@ function createInjector(modulesToLoad, strictDi) { provider[invokeArgs[1]].apply(provider, invokeArgs[2]); } } - - try { + function tryBlock() { if (isString(module)) { moduleFn = angularModule(module); instanceInjector.modules[module] = moduleFn; @@ -845,20 +844,28 @@ function createInjector(modulesToLoad, strictDi) { } else { assertArgFn(module, 'module'); } - } catch (e) { - if (isArray(module)) { - module = module[module.length - 1]; - } - if (e.message && e.stack && e.stack.indexOf(e.message) === -1) { - // Safari & FF's stack traces don't contain error.message content - // unlike those of Chrome and IE - // So if stack doesn't contain message, we create a new string that contains both. - // Since error.stack is read-only in Safari, I'm overriding e and not e.stack here. - // eslint-disable-next-line no-ex-assign - e = e.message + '\n' + e.stack; - } - throw $injectorMinErr('modulerr', 'Failed to instantiate module {0} due to:\n{1}', - module, e.stack || e.message || e); + } + + if (!errorHandlingConfig().isModuleError) { + tryBlock(); + } else { + try { + tryBlock(); + } catch (e) { + if (isArray(module)) { + module = module[module.length - 1]; + } + if (errorHandlingConfig().isModuleStack && e.message && e.stack && e.stack.indexOf(e.message) === -1) { + // Safari & FF's stack traces don't contain error.message content + // unlike those of Chrome and IE + // So if stack doesn't contain message, we create a new string that contains both. + // Since error.stack is read-only in Safari, I'm overriding e and not e.stack here. + // eslint-disable-next-line no-ex-assign + e = e.message + '\n' + e.stack; + } + throw $injectorMinErr('modulerr', 'Failed to instantiate module {0} due to:\n{1}', + module,errorHandlingConfig().isModuleStack ? (e.stack || e.message || e) : e.message); + } } }); return runBlocks; diff --git a/src/minErr.js b/src/minErr.js index d636d171984c..2d7133791959 100644 --- a/src/minErr.js +++ b/src/minErr.js @@ -1,16 +1,17 @@ 'use strict'; /* exported - minErrConfig, + errConfigObj, errorHandlingConfig, isValidObjectMaxDepth */ -var minErrConfig = { +var errConfigObj = { objectMaxDepth: 5, - isUrlParameters:true + isUrlParameters:true, + isModuleStack:true, + isModuleError:true }; - /** * @ngdoc function * @name angular.errorHandlingConfig @@ -33,17 +34,27 @@ var minErrConfig = { * Default: 5 * * `isUrlParameters` **{Boolean}** - Specifies wether the generated url error will contain the paramters or not. * Default: true + * * `isModuleStack` **{Boolean}** - Specifies wether the generated error for a module has a stack + * Default: true + * * `isModuleError` **{Boolean}** - Specifies wether the error will be rethrown for each module + * Default: true */ function errorHandlingConfig(config) { if (isObject(config)) { if (isDefined(config.objectMaxDepth)) { - minErrConfig.objectMaxDepth = isValidObjectMaxDepth(config.objectMaxDepth) ? config.objectMaxDepth : NaN; + errConfigObj.objectMaxDepth = isValidObjectMaxDepth(config.objectMaxDepth) ? config.objectMaxDepth : NaN; } if (isDefined(config.isUrlParameters) && isBoolean(config.isUrlParameters)) { - minErrConfig.isUrlParameters = config.isUrlParameters; + errConfigObj.isUrlParameters = config.isUrlParameters; + } + if (isDefined(config.isModuleStack) && isBoolean(config.isModuleStack)) { + errConfigObj.isModuleStack = config.isModuleStack; + } + if (isDefined(config.isModuleError) && isBoolean(config.isModuleError)) { + errConfigObj.isModuleError = config.isModuleError; } } else { - return minErrConfig; + return errConfigObj; } } @@ -94,7 +105,7 @@ function minErr(module, ErrorConstructor) { template = arguments[1], message = '[' + (module ? module + ':' : '') + code + '] ', templateArgs = sliceArgs(arguments, 2).map(function(arg) { - return toDebugString(arg, minErrConfig.objectMaxDepth); + return toDebugString(arg, errConfigObj.objectMaxDepth); }), paramPrefix, i; @@ -110,7 +121,7 @@ function minErr(module, ErrorConstructor) { message += '\nhttp://errors.angularjs.org/"NG_VERSION_FULL"/' + (module ? module + '/' : '') + code; - if (minErrConfig.isUrlParameters) { + if (errConfigObj.isUrlParameters) { for (i = 0, paramPrefix = '?'; i < templateArgs.length; i++, paramPrefix = '&') { message += paramPrefix + 'p' + i + '=' + encodeURIComponent(templateArgs[i]); } diff --git a/test/.eslintrc.json b/test/.eslintrc.json index 6401cb26f590..facd3ca166dc 100644 --- a/test/.eslintrc.json +++ b/test/.eslintrc.json @@ -25,7 +25,7 @@ /* angular.js */ "angular": false, - "minErrConfig": false, + "errConfigObj": false, "errorHandlingConfig": false, "msie": false, "jqLite": false, diff --git a/test/auto/injectorSpec.js b/test/auto/injectorSpec.js index 7f5254e201e6..fba755fede19 100644 --- a/test/auto/injectorSpec.js +++ b/test/auto/injectorSpec.js @@ -940,6 +940,13 @@ describe('injector', function() { describe('error handling', function() { + var obj = angular.copy(errorHandlingConfig()); + + afterEach(function() { + errorHandlingConfig({isModuleStack:obj.isModuleStack}); + errorHandlingConfig({isModuleError:obj.isModuleError}); + }); + it('should handle wrong argument type', function() { expect(function() { createInjector([ @@ -957,6 +964,33 @@ describe('injector', function() { }).toThrowMinErr('$injector', 'modulerr', /Failed to instantiate module .+ due to:\n.*MyError/); }); + it('should just rethrow the exception if ´isModuleError´ is false', function() { + expect(function() { + errorHandlingConfig({isModuleError:false}); + createInjector([function() { + throw new Error('MyError'); + }], {}); + }).toThrow(new Error('MyError')); + }); + + it('should not generate the stack for the failed module if ´isModuleStack´ is false', function() { + errorHandlingConfig({isModuleStack:false}); + expect(function() { + createInjector([function() { + throw new Error('MyError'); + }], {}); + + }).not.toThrowMinErr('$injector', 'modulerr', /(\w+@|at\s+\w+).+:\d+:\d+/); + }); + + it('should generate the stack if ´isModuleStack´ option is true', function() { + errorHandlingConfig({isModuleStack:true}); + expect(function() { + createInjector([function() { + throw new Error('MyError'); + }], {}); + }).toThrowMinErr('$injector', 'modulerr', /(\w+@|at\s+\w+).+:\d+:\d+/); + }); it('should decorate the missing service error with module name', function() { angular.module('TestModule', [], function(xyzzy) {}); diff --git a/test/minErrSpec.js b/test/minErrSpec.js index 201356c5605f..4b505c8b9a5a 100644 --- a/test/minErrSpec.js +++ b/test/minErrSpec.js @@ -1,11 +1,13 @@ 'use strict'; describe('errors', function() { - var originalObjectMaxDepthInErrorMessage = minErrConfig.objectMaxDepth; - var originalIsUrlParameters = minErrConfig.isUrlParameters; + var originalObj = angular.copy(errConfigObj); + afterEach(function() { - minErrConfig.objectMaxDepth = originalObjectMaxDepthInErrorMessage; - minErrConfig.isUrlParameters = originalIsUrlParameters; + errConfigObj.objectMaxDepth = originalObj.objectMaxDepth; + errConfigObj.isUrlParameters = originalObj.isUrlParameters; + errConfigObj.isModuleError = originalObj.isModuleError; + errConfigObj.isModuleStack = originalObj.isModuleStack; }); describe('errorHandlingConfig', function() { @@ -21,7 +23,7 @@ describe('errors', function() { it('should not change objectMaxDepth when undefined is supplied', function() { errorHandlingConfig({objectMaxDepth: undefined}); - expect(errorHandlingConfig().objectMaxDepth).toBe(originalObjectMaxDepthInErrorMessage); + expect(errorHandlingConfig().objectMaxDepth).toBe(originalObj.objectMaxDepth); }); they('should set objectMaxDepth to NaN when $prop is supplied', @@ -45,7 +47,37 @@ describe('errors', function() { }); it('should not change its value when non-boolean is supplied', function() { errorHandlingConfig({isUrlParameters:123}); - expect(errorHandlingConfig().isUrlParameters).toBe(originalIsUrlParameters); + expect(errorHandlingConfig().isUrlParameters).toBe(originalObj.isUrlParameters); + }); + }); + describe('isModuleStack',function() { + it('should get default isModuleStack', function() { + expect(errorHandlingConfig().isModuleStack).toBe(true); + }); + it('should set isModuleStack', function() { + errorHandlingConfig({isModuleStack:false}); + expect(errorHandlingConfig().isModuleStack).toBe(false); + errorHandlingConfig({isModuleStack:true}); + expect(errorHandlingConfig().isModuleStack).toBe(true); + }); + it('should not change its value when non-boolean is supplied', function() { + errorHandlingConfig({isModuleStack:123}); + expect(errorHandlingConfig().isModuleStack).toBe(originalObj.isModuleStack); + }); + }); + describe('isModuleError',function() { + it('should get default isModuleError', function() { + expect(errorHandlingConfig().isModuleError).toBe(true); + }); + it('should set isModuleError', function() { + errorHandlingConfig({isModuleError:false}); + expect(errorHandlingConfig().isModuleError).toBe(false); + errorHandlingConfig({isModuleError:true}); + expect(errorHandlingConfig().isModuleError).toBe(true); + }); + it('should not change its value when non-boolean is supplied', function() { + errorHandlingConfig({isModuleError:123}); + expect(errorHandlingConfig().isModuleError).toBe(originalObj.isModuleError); }); });