Merge pull request #1790 from mitsos1os/return-promise-on-error

Convert error thrown by a property setter to a rejected promise
loopbackio · Nov 29, 2019 · 770f11b · 770f11b
2 parents 89a964e + b30fbf8
commit 770f11b
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 2 deletions.
diff --git a/lib/dao.js b/lib/dao.js
@@ -334,7 +334,10 @@ DataAccessObject.create = function(data, options, cb) {
     this.applyProperties(enforced, obj);
     obj.setAttributes(enforced);
   } catch (err) {
-    return cb(err);
+    process.nextTick(function() {
+      cb(err);
+    });
+    return cb.promise;
   }
 
   Model = this.lookupModel(data); // data-specific
@@ -2606,7 +2609,10 @@ DataAccessObject.replaceById = function(id, data, options, cb) {
     this.applyProperties(enforced, inst);
     inst.setAttributes(enforced);
   } catch (err) {
-    return cb(err);
+    process.nextTick(function() {
+      cb(err);
+    });
+    return cb.promise;
   }
 
   Model = this.lookupModel(data); // data-specific

diff --git a/test/manipulation.test.js b/test/manipulation.test.js
@@ -17,6 +17,11 @@ const ValidationError = require('..').ValidationError;
 
 const UUID_REGEXP = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
 
+const throwingSetter = (value) => {
+  if (!value) return; // no-op
+  throw new Error('Intentional error triggered from a property setter');
+};
+
 describe('manipulation', function() {
   before(function(done) {
     db = getSchema();
@@ -28,8 +33,11 @@ describe('manipulation', function() {
       age: {type: Number, index: true},
       dob: Date,
       createdAt: {type: Date, default: Date},
+      throwingSetter: {type: String, default: null},
     }, {forceId: true, strict: true});
 
+    Person.setter.throwingSetter = throwingSetter;
+
     db.automigrate(['Person'], done);
   });
 
@@ -111,6 +119,11 @@ describe('manipulation', function() {
         .catch(done);
     });
 
+    it('should return rejected promise when model initialization failed', async () => {
+      await Person.create({name: 'Sad Fail', age: 25, throwingSetter: 'something'}).should
+        .be.rejectedWith('Intentional error triggered from a property setter');
+    });
+
     it('should instantiate an object', function(done) {
       const p = new Person({name: 'Anatoliy'});
       p.name.should.equal('Anatoliy');
@@ -1563,7 +1576,9 @@ describe('manipulation', function() {
       Post = db.define('Post', {
         title: {type: String, length: 255},
         content: {type: String},
+        throwingSetter: {type: String, default: null},
       }, {forceId: true});
+      Post.setter.throwingSetter = throwingSetter;
       db.automigrate('Post', done);
     });
 
@@ -1598,11 +1613,19 @@ describe('manipulation', function() {
         id: created.id,
         title: 'Draft',
         content: 'a content',
+        throwingSetter: null,
       });
 
       // Verify that no warnings were triggered
       Object.keys(Post._warned).should.be.empty();
     });
+
+    it('should return rejected promise when model initialization failed', async () => {
+      const firstNotFailedPost = await Post.create({title: 'Sad Post'}); // no property with failing setter
+      await Post.replaceById(firstNotFailedPost.id, {
+        title: 'Sad Post', throwingSetter: 'somethingElse',
+      }).should.be.rejectedWith('Intentional error triggered from a property setter');
+    });
   });
 
   describe('findOrCreate', function() {