Skip to content

Commit

Permalink
feat(Transition): Normalize all transition errors to a Rejection.
Browse files Browse the repository at this point in the history 
BREAKING CHANGE: All Transition errors are now wrapped in a Rejection object.

#### Before:

Previously, if a transition hook returned a rejected promise:
```js
.onStart({}, () => Promise.reject('reject transition'));
```

In `onError` or `transtion.promise.catch()`, the raw rejection was returned:
```js
.onError({}, (trans, err) => err === 'reject transition')
```

#### Now:

Now, the error is wrapped in a Rejection object.

- The detail (thrown error or rejected value) is still available as `.detail`.

```js
.onError({}, (trans, err) =>
    err instanceof Rejection && err.detail === 'reject transition')
```

- The Rejection object indicates the `.type` of transition rejection (ABORTED, ERROR, SUPERSEDED and/or redirection).
```js
.onError({}, (trans, err) => {
  err.type === RejectType.ABORTED === 3
});
```

#### Motivation:

Errors *thrown from* a hook and rejection values *returned from* a hook can now be processed in the same way.

#### BC Likelihood

How likely is this to affect me?

Medium: apps which have onError handlers for rejected values

#### BC Severity

How severe is this change?

Low: Find all error handlers (or .catch/.then chains) that do not understand Rejection. Add `err.detail` processing.
  • Loading branch information
@christopherthielen
christopherthielen committed Feb 23, 2017
1 parent 05013a6 commit a7464bb
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 62 deletions.
2 changes: 1 addition & 1 deletion src/transition/transitionEventType.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,6 @@ export class TransitionEventType {
public reverseSort: boolean = false,
public getResultHandler: GetResultHandler = TransitionHook.HANDLE_RESULT,
public getErrorHandler: GetErrorHandler = TransitionHook.REJECT_ERROR,
public rejectIfSuperseded: boolean = true,
public synchronous: boolean = false,
) { }
}
124 changes: 73 additions & 51 deletions src/transition/transitionHook.ts
View file
Original file line number Diff line number Diff line change
@@ -1,22 +1,21 @@
/**
* @coreapi
* @module transition
*/ /** for typedoc */
import {TransitionHookOptions, HookResult} from "./interface";
import {defaults, noop, identity} from "../common/common";
import {fnToString, maxLength} from "../common/strings";
import {isPromise} from "../common/predicates";
import {val, is, parse} from "../common/hof";
import {trace} from "../common/trace";
import {services} from "../common/coreservices";

import {Rejection} from "./rejectFactory";
import {TargetState} from "../state/targetState";
import {Transition} from "./transition";
import {State} from "../state/stateObject";
import {TransitionEventType} from "./transitionEventType";
import {StateService} from "../state/stateService"; // has or is using
import {RegisteredHook} from "./hookRegistry"; // has or is using
*/
/** for typedoc */
import { TransitionHookOptions, HookResult } from './interface';
import { defaults, noop, silentRejection } from '../common/common';
import { fnToString, maxLength } from '../common/strings';
import { isPromise } from '../common/predicates';
import { is, parse } from '../common/hof';
import { trace } from '../common/trace';
import { services } from '../common/coreservices';
import { Rejection } from './rejectFactory';
import { TargetState } from '../state/targetState';
import { Transition } from './transition';
import { State } from '../state/stateObject';
import { TransitionEventType } from './transitionEventType';
import { RegisteredHook } from './hookRegistry'; // has or is using

let defaultOptions: TransitionHookOptions = {
current: noop,
Expand All @@ -33,35 +32,52 @@ export type ErrorHandler = (error) => Promise<any>;

/** @hidden */
export class TransitionHook {
type: TransitionEventType;
constructor(private transition: Transition,
private stateContext: State,
private registeredHook: RegisteredHook,
private options: TransitionHookOptions) {
this.options = defaults(options, defaultOptions);
this.type = registeredHook.eventType;
}

stateService = () => this.transition.router.stateService;
/**
* These GetResultHandler(s) are used by [[invokeHook]] below
* Each HookType chooses a GetResultHandler (See: [[TransitionService._defineCoreEvents]])
*/
static HANDLE_RESULT: GetResultHandler = (hook: TransitionHook) => (result: HookResult) =>
hook.handleHookResult(result);

static HANDLE_RESULT: GetResultHandler = (hook: TransitionHook) =>
(result: HookResult) =>
hook.handleHookResult(result);
/**
* If the result is a promise rejection, log it.
* Otherwise, ignore the result.
*/
static LOG_REJECTED_RESULT: GetResultHandler = (hook: TransitionHook) => (result: HookResult) => {
isPromise(result) && result.catch(err =>
hook.logError(Rejection.normalize(err)));
return undefined;
};

static IGNORE_RESULT: GetResultHandler = (hook: TransitionHook) =>
(result: HookResult) => undefined;
/**
* These GetErrorHandler(s) are used by [[invokeHook]] below
* Each HookType chooses a GetErrorHandler (See: [[TransitionService._defineCoreEvents]])
*/
static LOG_ERROR: GetErrorHandler = (hook: TransitionHook) => (error) =>
hook.logError(error);

static LOG_ERROR: GetErrorHandler = (hook: TransitionHook) =>
(error) =>
(hook.stateService().defaultErrorHandler()(error), undefined);
static REJECT_ERROR: GetErrorHandler = (hook: TransitionHook) => (error) =>
silentRejection(error);

static REJECT_ERROR: GetErrorHandler = (hook: TransitionHook) =>
(error) =>
Rejection.errored(error).toPromise();
static THROW_ERROR: GetErrorHandler = (hook: TransitionHook) => (error) => {
throw error;
};

static THROW_ERROR: GetErrorHandler = (hook: TransitionHook) =>
undefined;
private isSuperseded = () =>
!this.type.synchronous && this.options.current() !== this.options.transition;

private rejectIfSuperseded = () =>
this.registeredHook.eventType.rejectIfSuperseded && this.options.current() !== this.options.transition;
logError(err): any {
this.transition.router.stateService.defaultErrorHandler()(err);
}

invokeHook(): Promise<HookResult> {
let hook = this.registeredHook;
Expand All @@ -71,30 +87,36 @@ export class TransitionHook {
trace.traceHookInvocation(this, this.transition, options);

// A new transition started before this hook (from a previous transition) could be run.
if (this.rejectIfSuperseded()) {
if (this.isSuperseded()) {
return Rejection.superseded(options.current()).toPromise();
}

let cb = hook.callback;
let bind = this.options.bind;
let trans = this.transition;
let state = this.stateContext;
const invokeCallback = () =>
hook.callback.call(this.options.bind, this.transition, this.stateContext);

let errorHandler = hook.eventType.getErrorHandler(this);
let resultHandler = hook.eventType.getResultHandler(this);
resultHandler = resultHandler || identity;
const normalizeErr = err =>
Rejection.normalize(err).toPromise();

if (!errorHandler) {
return resultHandler(cb.call(bind, trans, state));
}
const handleError = err =>
hook.eventType.getErrorHandler(this)(err);

const handleResult = result =>
hook.eventType.getResultHandler(this)(result);

try {
return resultHandler(cb.call(bind, trans, state));
} catch (error) {
return errorHandler(error);
if (this.type.synchronous) {
try {
return handleResult(invokeCallback());
} catch (err) {
return handleError(Rejection.normalize(err));
}
}

return services.$q.when()
.then(invokeCallback)
.catch(normalizeErr)
.then(handleResult, handleError);
}

/**
* This method handles the return value of a Transition Hook.
*
Expand All @@ -107,15 +129,15 @@ export class TransitionHook {
handleHookResult(result: HookResult): Promise<HookResult> {
// This transition is no longer current.
// Another transition started while this hook was still running.
if (this.rejectIfSuperseded()) {
if (this.isSuperseded()) {
// Abort this transition
return Rejection.superseded(this.options.current()).toPromise();
}

// Hook returned a promise
if (isPromise(result)) {
// Wait for the promise, then reprocess the settled promise value
return result.then(this.handleHookResult.bind(this));
// Wait for the promise, then reprocess with the resulting value
return result.then(val => this.handleHookResult(val));
}

trace.traceHookResult(result, this.transition, this.options);
Expand Down
14 changes: 8 additions & 6 deletions src/transition/transitionService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,19 +235,21 @@ export class TransitionService implements IHookRegistry, Disposable {
const Phase = TransitionHookPhase;
const TH = TransitionHook;
const paths = this._criteriaPaths;
const NORMAL_SORT = false, REVERSE_SORT = true;
const ASYNCHRONOUS = false, SYNCHRONOUS = true;

this._defineEvent("onCreate", Phase.CREATE, 0, paths.to, false, TH.IGNORE_RESULT, TH.THROW_ERROR, false);
this._defineEvent("onCreate", Phase.CREATE, 0, paths.to, NORMAL_SORT, TH.LOG_REJECTED_RESULT, TH.THROW_ERROR, SYNCHRONOUS);

this._defineEvent("onBefore", Phase.BEFORE, 0, paths.to);

this._defineEvent("onStart", Phase.RUN, 0, paths.to);
this._defineEvent("onExit", Phase.RUN, 100, paths.exiting, true);
this._defineEvent("onExit", Phase.RUN, 100, paths.exiting, REVERSE_SORT);
this._defineEvent("onRetain", Phase.RUN, 200, paths.retained);
this._defineEvent("onEnter", Phase.RUN, 300, paths.entering);
this._defineEvent("onFinish", Phase.RUN, 400, paths.to);

this._defineEvent("onSuccess", Phase.SUCCESS, 0, paths.to, false, TH.IGNORE_RESULT, TH.LOG_ERROR, false);
this._defineEvent("onError", Phase.ERROR, 0, paths.to, false, TH.IGNORE_RESULT, TH.LOG_ERROR, false);
this._defineEvent("onSuccess", Phase.SUCCESS, 0, paths.to, NORMAL_SORT, TH.LOG_REJECTED_RESULT, TH.LOG_ERROR, SYNCHRONOUS);
this._defineEvent("onError", Phase.ERROR, 0, paths.to, NORMAL_SORT, TH.LOG_REJECTED_RESULT, TH.LOG_ERROR, SYNCHRONOUS);
}

/** @hidden */
Expand All @@ -269,9 +271,9 @@ export class TransitionService implements IHookRegistry, Disposable {
reverseSort: boolean = false,
getResultHandler: GetResultHandler = TransitionHook.HANDLE_RESULT,
getErrorHandler: GetErrorHandler = TransitionHook.REJECT_ERROR,
rejectIfSuperseded: boolean = true)
synchronous: boolean = false)
{
let eventType = new TransitionEventType(name, hookPhase, hookOrder, criteriaMatchPath, reverseSort, getResultHandler, getErrorHandler, rejectIfSuperseded);
let eventType = new TransitionEventType(name, hookPhase, hookOrder, criteriaMatchPath, reverseSort, getResultHandler, getErrorHandler, synchronous);

this._eventTypes.push(eventType);
makeEvent(this, this, eventType);
Expand Down
8 changes: 4 additions & 4 deletions test/lazyLoadSpec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ describe('future state', function () {
expect($state.get('A')).toBe(futureStateDef);

$state.go('A').catch(() => {
expect(errors).toEqual(['nope']);
expect(errors.map(x => x.detail)).toEqual(['nope']);
expect($state.get('A')).toBe(futureStateDef);
done();
});
Expand All @@ -399,18 +399,18 @@ describe('future state', function () {
expect($state.get('A')).toBe(futureStateDef);

$state.go('A').catch(() => {
expect(errors).toEqual(['nope']);
expect(errors.map(x => x.detail)).toEqual(['nope']);
expect($state.get('A')).toBe(futureStateDef);
expect(count).toBe(1);

$state.go('A').catch(() => {
expect(errors).toEqual(['nope', 'nope']);
expect(errors.map(x => x.detail)).toEqual(['nope', 'nope']);
expect($state.get('A')).toBe(futureStateDef);
expect(count).toBe(2);

// this time it should lazy load
$state.go('A').then(() => {
expect(errors).toEqual(['nope', 'nope']);
expect(errors.map(x => x.detail)).toEqual(['nope', 'nope']);
expect($state.get('A')).toBeTruthy();
expect($state.get('A')).not.toBe(futureStateDef);
expect(count).toBe(3);
Expand Down

0 comments on commit a7464bb

Please sign in to comment.