From 38432f49b295fd1efc721d81f7359a8df78380a8 Mon Sep 17 00:00:00 2001 From: Chris Thielen Date: Mon, 27 Jun 2016 09:16:45 -0500 Subject: [PATCH] fix(Rejection): Silence "Error: Uncaught (in Exception)" Closes #2676 --- src/common/common.ts | 13 +++++++------ src/state/stateService.ts | 6 ++++-- src/transition/rejectFactory.ts | 5 ++--- src/transition/transition.ts | 6 +++++- src/transition/transitionHook.ts | 13 ++++--------- test/ng1/transitionSpec.ts | 14 +++++++++++--- test/testUtilsNg1.js | 19 +++++++++++++++++++ 7 files changed, 52 insertions(+), 24 deletions(-) diff --git a/src/common/common.ts b/src/common/common.ts index 9159b428c..41eaa1f79 100644 --- a/src/common/common.ts +++ b/src/common/common.ts @@ -6,6 +6,7 @@ import {isFunction, isString, isArray, isRegExp, isDate} from "./predicates"; import { all, any, not, prop, curry } from "./hof"; +import {services} from "./coreservices"; let w: any = typeof window === 'undefined' ? {} : window; let angular = w.angular || {}; @@ -538,9 +539,9 @@ function _arraysEq(a1, a2) { if (a1.length !== a2.length) return false; return arrayTuples(a1, a2).reduce((b, t) => b && _equals(t[0], t[1]), true); } -// -//const _addToGroup = (result, keyFn) => (item) => -// (result[keyFn(item)] = result[keyFn(item)] || []).push(item) && result; -//const groupBy = (array, keyFn) => array.reduce((memo, item) => _addToGroup(memo, keyFn), {}); -// -// + +// issue #2676 +export const silenceUncaughtInPromise = (promise: Promise) => + promise.catch(e => 0) && promise; +export const silentRejection = (error: any) => + silenceUncaughtInPromise(services.$q.reject(error)); diff --git a/src/state/stateService.ts b/src/state/stateService.ts index cba36cc11..6e090c595 100644 --- a/src/state/stateService.ts +++ b/src/state/stateService.ts @@ -1,5 +1,5 @@ /** @module state */ /** */ -import {extend, defaults } from "../common/common"; +import {extend, defaults, silentRejection, silenceUncaughtInPromise} from "../common/common"; import {isDefined, isObject, isString} from "../common/predicates"; import {Queue} from "../common/queue"; import {services} from "../common/coreservices"; @@ -274,7 +274,7 @@ export class StateService { return this._handleInvalidTargetState(currentPath, ref); if (!ref.valid()) - return services.$q.reject(ref.error()); + return silentRejection(ref.error()); /** * Special handling for Ignored, Aborted, and Redirected transitions @@ -307,6 +307,8 @@ export class StateService { let transition = this.router.transitionService.create(currentPath, ref); let transitionToPromise = transition.run().catch(rejectedTransitionHandler(transition)); + silenceUncaughtInPromise(transitionToPromise); // issue #2676 + // Return a promise for the transition, which also has the transition object on it. return extend(transitionToPromise, { transition }); }; diff --git a/src/transition/rejectFactory.ts b/src/transition/rejectFactory.ts index 5c21d0afb..ecd60b95f 100644 --- a/src/transition/rejectFactory.ts +++ b/src/transition/rejectFactory.ts @@ -1,7 +1,6 @@ /** @module transition */ /** for typedoc */ "use strict"; -import {extend} from "../common/common"; -import {services} from "../common/coreservices"; +import {extend, silentRejection} from "../common/common"; import {stringify} from "../common/strings"; export enum RejectType { @@ -27,7 +26,7 @@ export class Rejection { } toPromise() { - return extend(services.$q.reject(this), { _transitionRejection: this }); + return extend(silentRejection(this), { _transitionRejection: this }); } /** Returns true if the obj is a rejected promise created from the `asPromise` factory */ diff --git a/src/transition/transition.ts b/src/transition/transition.ts index 03107d020..6b6a714d3 100644 --- a/src/transition/transition.ts +++ b/src/transition/transition.ts @@ -487,9 +487,13 @@ export class Transition implements IHookRegistry { trace.traceTransitionStart(this); + // Chain the next hook off the previous + const appendHookToChain = (prev, nextHook) => + prev.then(() => nextHook.invokeHook()); + // Run the hooks, then resolve or reject the overall deferred in the .then() handler hookBuilder.asyncHooks() - .reduce((_chain, step) => _chain.then(step.invokeHook.bind(step)), syncResult) + .reduce(appendHookToChain, syncResult) .then(transitionSuccess, transitionError); return this.promise; diff --git a/src/transition/transitionHook.ts b/src/transition/transitionHook.ts index 2ba42d7a7..5a71244d1 100644 --- a/src/transition/transitionHook.ts +++ b/src/transition/transitionHook.ts @@ -34,20 +34,15 @@ export class TransitionHook { private isSuperseded = () => this.options.current() !== this.options.transition; - invokeHook(rethrow = false): Promise { + invokeHook(): Promise { let { options, hookFn, resolveContext } = this; trace.traceHookInvocation(this, options); if (options.rejectIfSuperseded && this.isSuperseded()) { return Rejection.superseded(options.current()).toPromise(); } - try { - var hookResult = hookFn.call(options.bind, this.transition, resolveContext.injector(), this.stateContext); - return this.handleHookResult(hookResult); - } catch (error) { - if (rethrow) throw error; - return services.$q.reject(error); - } + let hookResult = hookFn.call(options.bind, this.transition, resolveContext.injector(), this.stateContext); + return this.handleHookResult(hookResult); } /** @@ -96,7 +91,7 @@ export class TransitionHook { let results = []; for (let i = 0; i < hooks.length; i++) { try { - results.push(hooks[i].invokeHook(true)); + results.push(hooks[i].invokeHook()); } catch (exception) { if (!swallowExceptions) { return Rejection.aborted(exception).toPromise(); diff --git a/test/ng1/transitionSpec.ts b/test/ng1/transitionSpec.ts index b831a3bf6..765dff4dc 100644 --- a/test/ng1/transitionSpec.ts +++ b/test/ng1/transitionSpec.ts @@ -14,7 +14,7 @@ import {Transition} from "../../src/transition/transition"; describe('transition', function () { - var transitionProvider, matcher, pathFactory, statesMap, queue; + var $exceptionHandler, transitionProvider, matcher, pathFactory, statesMap, queue; var targetState = function(identifier, params = {}, options?) { options = options || {}; @@ -22,8 +22,10 @@ describe('transition', function () { return new TargetState(identifier, stateDefinition, params, options); }; - beforeEach(module('ui.router', function ($transitionsProvider, $urlMatcherFactoryProvider) { + beforeEach(module('ui.router', function ($transitionsProvider, $urlMatcherFactoryProvider, $exceptionHandlerProvider) { + decorateExceptionHandler($exceptionHandlerProvider); transitionProvider = $transitionsProvider; + var stateTree = { first: {}, second: {}, @@ -71,7 +73,8 @@ describe('transition', function () { var makeTransition; - beforeEach(inject(function ($transitions, $state) { + beforeEach(inject(function ($transitions, $state, _$exceptionHandler_) { + $exceptionHandler = _$exceptionHandler_; matcher = new StateMatcher(statesMap); queue.flush($state); makeTransition = function makeTransition(from, to, options) { @@ -113,6 +116,7 @@ describe('transition', function () { it('$transition$.promise should reject on error', inject(function($transitions, $q) { var result = new PromiseResult(); + $exceptionHandler.disabled = true; transitionProvider.onStart({ from: "*", to: "third" }, function($transition$) { result.setPromise($transition$.promise); @@ -126,6 +130,7 @@ describe('transition', function () { it('$transition$.promise should reject on error in synchronous hooks', inject(function($transitions, $q) { var result = new PromiseResult(); + $exceptionHandler.disabled = true; transitionProvider.onBefore({ from: "*", to: "third" }, function($transition$) { result.setPromise($transition$.promise); @@ -298,6 +303,7 @@ describe('transition', function () { })); it('should be called even if other .onSuccess() callbacks fail (throw errors, etc)', inject(function($transitions, $q) { + $exceptionHandler.disabled = true; transitionProvider.onSuccess({ from: "*", to: "*" }, function() { throw new Error("oops!"); }); transitionProvider.onSuccess({ from: "*", to: "*" }, function(trans) { states.push(trans.to().name); }); @@ -318,6 +324,7 @@ describe('transition', function () { })); it('should be called if any part of the transition fails.', inject(function($transitions, $q) { + $exceptionHandler.disabled = true; transitionProvider.onEnter({ from: "A", entering: "C" }, function() { throw new Error("oops!"); }); transitionProvider.onError({ }, function(trans) { states.push(trans.to().name); }); @@ -327,6 +334,7 @@ describe('transition', function () { })); it('should be called for only handlers matching the transition.', inject(function($transitions, $q) { + $exceptionHandler.disabled = true; transitionProvider.onEnter({ from: "A", entering: "C" }, function() { throw new Error("oops!"); }); transitionProvider.onError({ from: "*", to: "*" }, function() { hooks.push("splatsplat"); }); transitionProvider.onError({ from: "A", to: "C" }, function() { hooks.push("AC"); }); diff --git a/test/testUtilsNg1.js b/test/testUtilsNg1.js index 5983b199b..428f59f77 100644 --- a/test/testUtilsNg1.js +++ b/test/testUtilsNg1.js @@ -116,6 +116,25 @@ function html5Compat(html5mode) { return (angular.isObject(html5mode) && html5mode.hasOwnProperty("enabled") ? html5mode.enabled : html5mode); } +/** + * The ng1 $exceptionHandler from angular-mocks will re-throw any exceptions thrown in a Promise. + * This chunk of code decorates the handler, allowing a test to disable that behavior. + * Inject $exceptionHandler and set `$exceptionHandler.disabled = true` + */ +function decorateExceptionHandler($exceptionHandlerProvider) { + var $get = $exceptionHandlerProvider.$get; + + $exceptionHandlerProvider.$get = function() { + var realHandler = $get.apply($exceptionHandlerProvider, arguments); + function passThrough(e) { + if (!passThrough['disabled']) { + realHandler.apply(null, arguments); + } + } + return passThrough; + }; +} + // Utils for test from core angular var noop = angular.noop,