Skip to content

Commit

Permalink
Merge pull request #15689 from kratiahuja/fastboot-error-template
Browse files Browse the repository at this point in the history 
[BUGFIX lts] Mark error as handled before transition for error routes and substates
  • Loading branch information
@rwjblue
rwjblue authored Oct 2, 2017
2 parents 9e24c7c + 6e32f4e commit ed381d7
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 87 deletions.
1 change: 1 addition & 0 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"pretest": "ember build",
"test": "node bin/run-tests.js",
"test:blueprints": "node node-tests/nodetest-runner.js",
"test:node": "node bin/run-node-tests.js",
"test:sauce": "node bin/run-sauce-tests.js",
"test:testem": "testem -f testem.dist.json",
"link:glimmer": "node bin/yarn-link-glimmer.js"
Expand Down
8 changes: 4 additions & 4 deletions packages/ember-glimmer/tests/integration/application/engine-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ moduleFor('Application test: engine rendering', class extends ApplicationTest {
return this.visit('/').then(() => {
this.assertText('Application');
return this.transitionTo('blog.post');
}).catch(() => {
}).then(() => {
return errorEntered.promise;
}).then(() => {
this.assertText('ApplicationError! Oh, noes!');
Expand Down Expand Up @@ -456,7 +456,7 @@ moduleFor('Application test: engine rendering', class extends ApplicationTest {
return this.visit('/').then(() => {
this.assertText('Application');
return this.transitionTo('blog.post');
}).catch(() => {
}).then(() => {
return errorEntered.promise;
}).then(() => {
this.assertText('ApplicationEngineError! Oh, noes!');
Expand Down Expand Up @@ -486,7 +486,7 @@ moduleFor('Application test: engine rendering', class extends ApplicationTest {
return this.visit('/').then(() => {
this.assertText('Application');
return this.transitionTo('blog.post');
}).catch(() => {
}).then(() => {
return errorEntered.promise;
}).then(() => {
this.assertText('ApplicationEngineError! Oh, noes!');
Expand Down Expand Up @@ -516,7 +516,7 @@ moduleFor('Application test: engine rendering', class extends ApplicationTest {
return this.visit('/').then(() => {
this.assertText('Application');
return this.transitionTo('blog.post.comments');
}).catch(() => {
}).then(() => {
return errorEntered.promise;
}).then(() => {
this.assertText('ApplicationEngineError! Oh, noes!');
Expand Down
4 changes: 4 additions & 0 deletions packages/ember-routing/lib/system/router.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1126,6 +1126,8 @@ let defaultActionHandlers = {
// Check for the existence of an 'error' route.
let errorRouteName = findRouteStateName(route, 'error');
if (errorRouteName) {
let errorId = guidFor(error);
router._markErrorAsHandled(errorId);
router.intermediateTransitionTo(errorRouteName, error);
return false;
}
Expand All @@ -1134,6 +1136,8 @@ let defaultActionHandlers = {
// Check for an 'error' substate route
let errorSubstateName = findRouteSubstateName(route, 'error');
if (errorSubstateName) {
var errorId = guidFor(error);
router._markErrorAsHandled(errorId);
router.intermediateTransitionTo(errorSubstateName, error);
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/ember/tests/routing/basic_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2714,7 +2714,7 @@ QUnit.test('Errors in transition show error template if available', function() {
}
});

throws(() => bootApplication(), /More context objects were passed/);
bootApplication();

equal(jQuery('#error').length, 1, 'Error template was rendered.');
});
Expand Down
123 changes: 59 additions & 64 deletions packages/ember/tests/routing/substates_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,15 +352,15 @@ moduleFor('Loading/Error Substates', class extends ApplicationTestCase {
}));

return this.visit('/').then(() => {
assert.throws(() => {
this.visit('/foo/bar');
}, (err) => err.msg === "did it broke?", 'it broke');
let text = this.$('#app').text();
assert.equal(
text,
'FOOBAR ERROR: did it broke?',
`foo.bar_error was entered (as opposed to something like foo/foo/bar_error)`
);
return this.visit('/foo/bar').then(() => {

let text = this.$('#app').text();
assert.equal(
text,
'FOOBAR ERROR: did it broke?',
`foo.bar_error was entered (as opposed to something like foo/foo/bar_error)`
);
});
});
}
['@test Prioritized loading substate entry works with auto-generated index routes'](assert) {
Expand Down Expand Up @@ -424,16 +424,16 @@ moduleFor('Loading/Error Substates', class extends ApplicationTestCase {
}));

return this.visit('/').then(() => {
assert.throws(() => {
this.visit('/foo');
}, (err) => err.msg === 'did it broke?', 'it broke');
let text = this.$('#app').text();

assert.equal(
text,
'FOO ERROR: did it broke?',
'foo.index_error was entered'
);

return this.visit('/foo').then(() => {
let text = this.$('#app').text();
assert.equal(
text,
'FOO ERROR: did it broke?',
'foo.index_error was entered'
);
});
});
}
});
Expand Down Expand Up @@ -461,28 +461,26 @@ moduleFor('Loading/Error Substates - globals mode app', class extends AutobootAp
['@test Rejected promises returned from ApplicationRoute transition into top-level application_error'](assert) {
let reject = true;

assert.throws(() => {
this.runTask(() => {
this.createApplication();
this.addTemplate('index', '<div id="app">INDEX</div>');
this.add('route:application', Route.extend({
init() {
this._super(...arguments);
},
model() {
if (reject) {
return RSVP.reject({ msg: 'BAD NEWS BEARS' });
} else {
return {};
}
this.runTask(() => {
this.createApplication();
this.addTemplate('index', '<div id="app">INDEX</div>');
this.add('route:application', Route.extend({
init() {
this._super(...arguments);
},
model() {
if (reject) {
return RSVP.reject({ msg: 'BAD NEWS BEARS' });
} else {
return {};
}
}));
}
}));

this.addTemplate('application_error', `
<p id="toplevel-error">TOPLEVEL ERROR: {{model.msg}}</p>
`);
})
}, (err) => err.msg === 'BAD NEWS BEARS', 'it went poorly');
this.addTemplate('application_error', `
<p id="toplevel-error">TOPLEVEL ERROR: {{model.msg}}</p>
`);
});

let text = this.$('#toplevel-error').text();
assert.equal(
Expand Down Expand Up @@ -645,16 +643,15 @@ moduleFor('Loading/Error Substates - nested routes', class extends ApplicationTe
}
}));

assert.throws(() => {
this.visit('/grandma/mom/sally');
}, (err) => err.msg === "did it broke?", 'it broke.');

step(3, 'App finished loading');

let text = this.$('#app').text();

assert.equal(text, 'GRANDMA ERROR: did it broke?', 'error bubbles');
assert.equal(this.currentPath, 'grandma.error', 'Initial route fully loaded');

return this.visit('/grandma/mom/sally').then(() => {
step(3, 'App finished loading');

let text = this.$('#app').text();

assert.equal(text, 'GRANDMA ERROR: did it broke?', 'error bubbles');
assert.equal(this.currentPath, 'grandma.error', 'Initial route fully loaded');
});
}

[`@test Non-bubbled errors that re-throw aren't swallowed`](assert) {
Expand Down Expand Up @@ -783,21 +780,19 @@ moduleFor('Loading/Error Substates - nested routes', class extends ApplicationTe
}
}));

assert.throws(() => {
this.visit('/grandma/mom/sally');
}, (err) => err.msg === 'did it broke?', 'it broke');

step(3, 'Application finished booting');

assert.equal(
this.$('#app').text(),
'GRANDMA MOM ERROR: did it broke?',
'the more specifically named mome error substate was entered over the other error route'
);

assert.equal(this.currentPath, 'grandma.mom_error',
'Initial route fully loaded'
);
return this.visit('/grandma/mom/sally').then(() => {
step(3, 'Application finished booting');

assert.equal(
this.$('#app').text(),
'GRANDMA MOM ERROR: did it broke?',
'the more specifically named mome error substate was entered over the other error route'
);

assert.equal(this.currentPath, 'grandma.mom_error',
'Initial route fully loaded'
);
});
}

['@test Slow promises waterfall on startup'](assert) {
Expand Down
62 changes: 44 additions & 18 deletions tests/node/visit-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,24 +170,50 @@ QUnit.test('FastBoot: route error', function(assert) {
var App = this.createApplication();

return RSVP.all([
fastbootVisit(App, '/a').then(
function(instance) {
assert.ok(false, 'It should not render');
instance.destroy();
},
function(error) {
assert.equal(error.message, 'Error from A');
}
),
fastbootVisit(App, '/b').then(
function(instance) {
assert.ok(false, 'It should not render');
instance.destroy();
},
function(error) {
assert.equal(error.message, 'Error from B');
}
)
fastbootVisit(App, '/a')
.then(
function(instance) {
assert.ok(false, 'It should not render');
instance.destroy();
},
function(error) {
assert.equal(error.message, 'Error from A');
}
),
fastbootVisit(App, '/b').then(
function(instance) {
assert.ok(false, 'It should not render');
instance.destroy();
},
function(error) {
assert.equal(error.message, 'Error from B');
}
)
]);
});

QUnit.test('FastBoot: route error template', function(assert) {
this.routes(function() {
this.route('a');
});

this.template('error', '<p>Error template rendered!</p>');
this.template('a', '<h1>Hello from A</h1>');

this.route('a', {
model: function() {
throw new Error('Error from A');
}
});

var App = this.createApplication();

return RSVP.all([
fastbootVisit(App, '/a')
.then(
assertFastbootResult(assert, { url: '/a', body: '<p>Error template rendered!</p>' }),
handleError(assert)
),
]);
});

Expand Down

0 comments on commit ed381d7

Please sign in to comment.