Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX lts] Mark error as handled before transition for error routes and substates #15689

Merged
merged 1 commit into from
Oct 2, 2017
Merged

[BUGFIX lts] Mark error as handled before transition for error routes and substates #15689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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