Skip to content

Commit

Permalink
Merge pull request #12752 from rwjblue/rebase-12549
Browse files Browse the repository at this point in the history
Do not re-raise on errors handled in route error action.
  • Loading branch information
rwjblue committed Dec 27, 2015
2 parents 2d622f1 + a8dc7ca commit 91b0ebb
Show file tree
Hide file tree
Showing 3 changed files with 310 additions and 2 deletions.
36 changes: 34 additions & 2 deletions packages/ember-routing/lib/system/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@ import {
stashParamNames,
calculateCacheKey
} from 'ember-routing/utils';
import { guidFor } from 'ember-metal/utils';
import RouterState from './router_state';
import { getOwner } from 'container/owner';
import dictionary from 'ember-metal/dictionary';

/**
@module ember
Expand All @@ -33,6 +35,7 @@ function K() { return this; }

var slice = [].slice;


/**
The `Ember.Router` class manages the application state and URLs. Refer to
the [routing guide](http://emberjs.com/guides/routing/) for documentation.
Expand Down Expand Up @@ -111,6 +114,7 @@ var EmberRouter = EmberObject.extend(Evented, {
this._activeViews = {};
this._qpCache = new EmptyObject();
this._resetQueuedQueryParameterChanges();
this._handledErrors = dictionary(null);
},

/*
Expand Down Expand Up @@ -299,8 +303,7 @@ var EmberRouter = EmberObject.extend(Evented, {

_doURLTransition(routerJsMethod, url) {
var transition = this.router[routerJsMethod](url || '/');
didBeginTransition(transition, this);
return transition;
return didBeginTransition(transition, this);
},

transitionTo(...args) {
Expand Down Expand Up @@ -724,6 +727,20 @@ var EmberRouter = EmberObject.extend(Evented, {
run.cancel(this._slowTransitionTimer);
}
this._slowTransitionTimer = null;
},

// These three helper functions are used to ensure errors aren't
// re-raised if they're handled in a route's error action.
_markErrorAsHandled(errorGuid) {
this._handledErrors[errorGuid] = true;
},

_isErrorHandled(errorGuid) {
return this._handledErrors[errorGuid];
},

_clearHandledError(errorGuid) {
delete this._handledErrors[errorGuid];
}
});

Expand Down Expand Up @@ -884,6 +901,11 @@ export function triggerEvent(handlerInfos, ignoreFailure, args) {
if (handler.actions[name].apply(handler, args) === true) {
eventWasHandled = true;
} else {
// Should only hit here if a non-bubbling error action is triggered on a route.
if (name === 'error') {
var errorId = guidFor(args[0]);
handler.router._markErrorAsHandled(errorId);
}
return;
}
}
Expand Down Expand Up @@ -1047,6 +1069,16 @@ function didBeginTransition(transition, router) {
router.set('currentState', routerState);
}
router.set('targetState', routerState);

return transition.catch(function(error) {
var errorId = guidFor(error);

if (router._isErrorHandled(errorId)) {
router._clearHandledError(errorId);
} else {
throw error;
}
});
}

function resemblesURL(str) {
Expand Down
2 changes: 2 additions & 0 deletions packages/ember/tests/routing/basic_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1022,6 +1022,7 @@ QUnit.test('The Special page returning an error invokes SpecialRoute\'s error ha
actions: {
error(reason) {
equal(reason, 'Setup error', 'SpecialRoute#error received the error thrown from setup');
return true;
}
}
});
Expand Down Expand Up @@ -1059,6 +1060,7 @@ function testOverridableErrorHandler(handlersName) {
attrs[handlersName] = {
error(reason) {
equal(reason, 'Setup error', 'error was correctly passed to custom ApplicationRoute handler');
return true;
}
};

Expand Down
274 changes: 274 additions & 0 deletions packages/ember/tests/routing/substates_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,280 @@ QUnit.test('Default error event moves into nested route', function() {
equal(appController.get('currentPath'), 'grandma.error', 'Initial route fully loaded');
});

QUnit.test('Error events that aren\'t bubbled don\t throw application assertions', function() {
expect(2);

templates['grandma'] = 'GRANDMA {{outlet}}';

Router.map(function() {
this.route('grandma', function() {
this.route('mom', { resetNamespace: true }, function() {
this.route('sally');
});
});
});

App.ApplicationController = Ember.Controller.extend();

App.MomSallyRoute = Ember.Route.extend({
model() {
step(1, 'MomSallyRoute#model');

return Ember.RSVP.reject({
msg: 'did it broke?'
});
},
actions: {
error(err) {
equal(err.msg, 'did it broke?');
return false;
}
}
});

bootApplication('/grandma/mom/sally');
});

QUnit.test('Non-bubbled errors that re-throw aren\'t swallowed', function() {
expect(2);

templates['grandma'] = 'GRANDMA {{outlet}}';

Router.map(function() {
this.route('grandma', function() {
this.route('mom', { resetNamespace: true }, function() {
this.route('sally');
});
});
});

App.ApplicationController = Ember.Controller.extend();

App.MomSallyRoute = Ember.Route.extend({
model() {
step(1, 'MomSallyRoute#model');

return Ember.RSVP.reject({
msg: 'did it broke?'
});
},
actions: {
error(err) {
// returns undefined which is falsey
throw err;
}
}
});

throws(function() {
bootApplication('/grandma/mom/sally');
}, function(err) { return err.msg === 'did it broke?';});
});

QUnit.test('Handled errors that re-throw aren\'t swallowed', function() {
expect(4);

var handledError;

templates['grandma'] = 'GRANDMA {{outlet}}';

Router.map(function() {
this.route('grandma', function() {
this.route('mom', { resetNamespace: true }, function() {
this.route('sally');
this.route('this-route-throws');
});
});
});

App.ApplicationController = Ember.Controller.extend();

App.MomSallyRoute = Ember.Route.extend({
model() {
step(1, 'MomSallyRoute#model');

return Ember.RSVP.reject({
msg: 'did it broke?'
});
},
actions: {
error(err) {
step(2, 'MomSallyRoute#error');

handledError = err;

this.transitionTo('mom.this-route-throws');

// Marks error as handled
return false;
}
}
});

App.MomThisRouteThrowsRoute = Ember.Route.extend({
model() {
step(3, 'MomThisRouteThrows#model');

throw handledError;
}
});

throws(function() {
bootApplication('/grandma/mom/sally');
}, function(err) { return err.msg === 'did it broke?'; });
});

QUnit.test('Handled errors that bubble can be handled at a higher level', function() {
expect(4);

var handledError;

templates['grandma'] = 'GRANDMA {{outlet}}';

Router.map(function() {
this.route('grandma', function() {
this.route('mom', { resetNamespace: true }, function() {
this.route('sally');
});
});
});

App.ApplicationController = Ember.Controller.extend();

App.MomRoute = Ember.Route.extend({
actions: {
error(err) {
step(3, 'MomRoute#error');

equal(err, handledError, 'error handled and rebubbled is handleable at heigher route');
}
}
});

App.MomSallyRoute = Ember.Route.extend({
model() {
step(1, 'MomSallyRoute#model');

return Ember.RSVP.reject({
msg: 'did it broke?'
});
},

actions: {
error(err) {
step(2, 'MomSallyRoute#error');

handledError = err;

return true;
}
}
});

bootApplication('/grandma/mom/sally');
});

QUnit.test('errors that are bubbled are thrown at a higher level if not handled', function() {
expect(3);

var handledError;

templates['grandma'] = 'GRANDMA {{outlet}}';

Router.map(function() {
this.route('grandma', function() {
this.route('mom', { resetNamespace: true }, function() {
this.route('sally');
});
});
});

App.ApplicationController = Ember.Controller.extend();

App.MomSallyRoute = Ember.Route.extend({
model() {
step(1, 'MomSallyRoute#model');

return Ember.RSVP.reject({
msg: 'did it broke?'
});
},

actions: {
error(err) {
step(2, 'MomSallyRoute#error');

handledError = err;

return true;
}
}
});

throws(
function() {
bootApplication('/grandma/mom/sally');
},
function(err) {
return err.msg === 'did it broke?';
},
'Correct error was thrown'
);
});

QUnit.test('Handled errors that are thrown through rejection aren\'t swallowed', function() {
expect(4);

var handledError;

templates['grandma'] = 'GRANDMA {{outlet}}';

Router.map(function() {
this.route('grandma', function() {
this.route('mom', { resetNamespace: true }, function() {
this.route('sally');
this.route('this-route-throws');
});
});
});

App.ApplicationController = Ember.Controller.extend();

App.MomSallyRoute = Ember.Route.extend({
model() {
step(1, 'MomSallyRoute#model');

return Ember.RSVP.reject({
msg: 'did it broke?'
});
},
actions: {
error(err) {
step(2, 'MomSallyRoute#error');

handledError = err;

this.transitionTo('mom.this-route-throws');

// Marks error as handled
return false;
}
}
});

App.MomThisRouteThrowsRoute = Ember.Route.extend({
model() {
step(3, 'MomThisRouteThrows#model');

return Ember.RSVP.reject(handledError);
}
});

throws(function() {
bootApplication('/grandma/mom/sally');
}, function(err) { return err.msg === 'did it broke?'; });
});

QUnit.test('Setting a query param during a slow transition should work', function() {
var deferred = RSVP.defer();

Expand Down

0 comments on commit 91b0ebb

Please sign in to comment.