From b204367aa64e7a49d535219655db320557b0b943 Mon Sep 17 00:00:00 2001 From: Clark Wang Date: Wed, 10 Dec 2014 12:04:56 +0800 Subject: [PATCH 1/3] fix nested remoting function throwing error will crash app Signed-off-by: Clark Wang --- lib/model.js | 12 ++++++++++-- test/relations.integration.js | 20 ++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/model.js b/lib/model.js index 8b55c5d4..21fe044f 100644 --- a/lib/model.js +++ b/lib/model.js @@ -674,7 +674,11 @@ Model.nestRemoting = function(relationName, options, cb) { this[getterName](fkId, function(err, inst) { if (err && cb) return cb(err); if (inst instanceof relation.modelTo) { - nestedFn.apply(inst, args); + try { + nestedFn.apply(inst, args); + } catch (err) { + cb(err); + } } else if (cb) { cb(err, null); } @@ -688,7 +692,11 @@ Model.nestRemoting = function(relationName, options, cb) { this[getterName](function(err, inst) { if (err && cb) return cb(err); if (inst instanceof relation.modelTo) { - nestedFn.apply(inst, args); + try { + nestedFn.apply(inst, args); + } catch (err) { + cb(err); + } } else if (cb) { cb(err, null); } diff --git a/test/relations.integration.js b/test/relations.integration.js index 4ba459f2..21a54d9b 100644 --- a/test/relations.integration.js +++ b/test/relations.integration.js @@ -1168,6 +1168,13 @@ describe('relations - integration', function() { Page.hasMany(Note); Image.belongsTo(Book); + // fake a remote method that match the filter in Model.nestRemoting() + Page.prototype.__throw__errors = function () { + throw new Error('This should not crash the app'); + }; + + Page.remoteMethod('__throw__errors', { isStatic: false, http: { path: '/throws', verb: 'get' } }); + Book.nestRemoting('pages'); Image.nestRemoting('book'); @@ -1305,6 +1312,19 @@ describe('relations - integration', function() { }); }); }); + + it('should catch error if nested function throws', function (done) { + var test = this; + this.get('/api/books/' + test.book.id + '/pages/' + this.page.id + '/throws') + .end(function(err, res) { + expect(res.body).to.be.an('object'); + expect(res.body.error).to.be.an('object'); + expect(res.body.error.name).to.equal('Error'); + expect(res.body.error.status).to.equal(500); + expect(res.body.error.message).to.equal('This should not crash the app'); + done(); + }); + }); }); }); From 7a3e2544030122456809a61cc9046625d97fd8fa Mon Sep 17 00:00:00 2001 From: Clark Wang Date: Wed, 10 Dec 2014 19:03:48 +0800 Subject: [PATCH 2/3] test if cb exists Signed-off-by: Clark Wang --- lib/model.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/model.js b/lib/model.js index 21fe044f..90e3dbea 100644 --- a/lib/model.js +++ b/lib/model.js @@ -677,7 +677,7 @@ Model.nestRemoting = function(relationName, options, cb) { try { nestedFn.apply(inst, args); } catch (err) { - cb(err); + cb && cb(err); } } else if (cb) { cb(err, null); @@ -695,7 +695,7 @@ Model.nestRemoting = function(relationName, options, cb) { try { nestedFn.apply(inst, args); } catch (err) { - cb(err); + cb && cb(err); } } else if (cb) { cb(err, null); From 9c147f1b250ef0305ceacade12e4342d5a26a26d Mon Sep 17 00:00:00 2001 From: Clark Wang Date: Wed, 10 Dec 2014 19:43:55 +0800 Subject: [PATCH 3/3] fix jshint errors Signed-off-by: Clark Wang --- lib/model.js | 4 ++-- test/relations.integration.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/model.js b/lib/model.js index 90e3dbea..d93e4ab4 100644 --- a/lib/model.js +++ b/lib/model.js @@ -677,7 +677,7 @@ Model.nestRemoting = function(relationName, options, cb) { try { nestedFn.apply(inst, args); } catch (err) { - cb && cb(err); + if (cb) return cb(err); } } else if (cb) { cb(err, null); @@ -695,7 +695,7 @@ Model.nestRemoting = function(relationName, options, cb) { try { nestedFn.apply(inst, args); } catch (err) { - cb && cb(err); + if (cb) return cb(err); } } else if (cb) { cb(err, null); diff --git a/test/relations.integration.js b/test/relations.integration.js index 21a54d9b..f1aa72da 100644 --- a/test/relations.integration.js +++ b/test/relations.integration.js @@ -1169,7 +1169,7 @@ describe('relations - integration', function() { Image.belongsTo(Book); // fake a remote method that match the filter in Model.nestRemoting() - Page.prototype.__throw__errors = function () { + Page.prototype['__throw__errors'] = function() { throw new Error('This should not crash the app'); }; @@ -1313,7 +1313,7 @@ describe('relations - integration', function() { }); }); - it('should catch error if nested function throws', function (done) { + it('should catch error if nested function throws', function(done) { var test = this; this.get('/api/books/' + test.book.id + '/pages/' + this.page.id + '/throws') .end(function(err, res) {