Skip to content

Commit

Permalink
fix(Rejection): Silence "Error: Uncaught (in Exception)"
Browse files Browse the repository at this point in the history
Closes #2676
  • Loading branch information
christopherthielen committed Jun 27, 2016
1 parent 6becb12 commit 38432f4
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 24 deletions.
13 changes: 7 additions & 6 deletions src/common/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 || {};
Expand Down Expand Up @@ -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<any>) =>
promise.catch(e => 0) && promise;
export const silentRejection = (error: any) =>
silenceUncaughtInPromise(services.$q.reject(error));
6 changes: 4 additions & 2 deletions src/state/stateService.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -274,7 +274,7 @@ export class StateService {
return this._handleInvalidTargetState(currentPath, ref);

if (!ref.valid())
return services.$q.reject(ref.error());
return <TransitionPromise> silentRejection(ref.error());

/**
* Special handling for Ignored, Aborted, and Redirected transitions
Expand Down Expand Up @@ -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 });
};
Expand Down
5 changes: 2 additions & 3 deletions src/transition/rejectFactory.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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 */
Expand Down
6 changes: 5 additions & 1 deletion src/transition/transition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 4 additions & 9 deletions src/transition/transitionHook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,15 @@ export class TransitionHook {

private isSuperseded = () => this.options.current() !== this.options.transition;

invokeHook(rethrow = false): Promise<any> {
invokeHook(): Promise<any> {
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);
}

/**
Expand Down Expand Up @@ -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();
Expand Down
14 changes: 11 additions & 3 deletions test/ng1/transitionSpec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,18 @@ 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 || {};
var stateDefinition = matcher.find(identifier, options.relative);
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: {},
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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); });

Expand All @@ -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); });

Expand All @@ -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"); });
Expand Down
19 changes: 19 additions & 0 deletions test/testUtilsNg1.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 38432f4

Please sign in to comment.