Merge pull request #1756 from strongloop/fix/migrate-errors

fix: report errors from automigrate/autoupdate

lib/datasource.js

@@ -136,6 +136,7 @@ function DataSource(name, settings, modelBuilder) {
   this.models = this.modelBuilder.models;
   this.definitions = this.modelBuilder.definitions;
   this.juggler = juggler;
+  this._queuedInvocations = 0;
 
   // operation metadata
   // Initialize it before calling setup as the connector might register operations
@@ -477,18 +478,22 @@ DataSource.prototype.setup = function(dsName, settings) {
       if (this.connected) {
         debug('DataSource %s is now connected to %s', this.name, this.connector.name);
         this.emit('connected');
-      } else {
+      } else if (err) {
         // The connection fails, let's report it and hope it will be recovered in the next call
-        if (err) {
-          // Reset the connecting to `false`
-          this.connecting = false;
+        // Reset the connecting to `false`
+        this.connecting = false;
+        if (this._queuedInvocations) {
+          // Another operation is already waiting for connect() result,
+          // let them handle the connection error.
+          debug('Connection fails: %s\nIt will be retried for the next request.', err);
+        } else {
           g.error('Connection fails: %s\nIt will be retried for the next request.', err);
           this.emit('error', err);
-        } else {
-          // Either lazyConnect or connector initialize() defers the connection
-          debug('DataSource %s will be connected to connector %s', this.name,
-            this.connector.name);
         }
+      } else {
+        // Either lazyConnect or connector initialize() defers the connection
+        debug('DataSource %s will be connected to connector %s', this.name,
+          this.connector.name);
       }
     }.bind(this);
 
@@ -1053,6 +1058,14 @@ DataSource.prototype.automigrate = function(models, cb) {
     }
   }
 
+  const args = [models, cb];
+  args.callee = this.automigrate;
+  const queued = this.ready(this, args);
+  if (queued) {
+    // waiting to connect
+    return cb.promise;
+  }
+
   this.connector.automigrate(models, cb);
   return cb.promise;
 };
@@ -1114,6 +1127,14 @@ DataSource.prototype.autoupdate = function(models, cb) {
     }
   }
 
+  const args = [models, cb];
+  args.callee = this.automigrate;
+  const queued = this.ready(this, args);
+  if (queued) {
+    // waiting to connect
+    return cb.promise;
+  }
+
   this.connector.autoupdate(models, cb);
   return cb.promise;
 };
@@ -2514,12 +2535,15 @@ function(obj, args) {
     return false;
   }
 
+  this._queuedInvocations++;
+
   const method = args.callee;
   // Set up a callback after the connection is established to continue the method call
 
   let onConnected = null, onError = null, timeoutHandle = null;
   onConnected = function() {
     debug('Datasource %s is now connected - executing method %s', self.name, method.name);
+    this._queuedInvocations--;
     // Remove the error handler
     self.removeListener('error', onError);
     if (timeoutHandle) {
@@ -2542,6 +2566,7 @@ function(obj, args) {
   };
   onError = function(err) {
     debug('Datasource %s fails to connect - aborting method %s', self.name, method.name);
+    this._queuedInvocations--;
     // Remove the connected listener
     self.removeListener('connected', onConnected);
     if (timeoutHandle) {
@@ -2563,6 +2588,7 @@ function(obj, args) {
   timeoutHandle = setTimeout(function() {
     debug('Datasource %s fails to connect due to timeout - aborting method %s',
       self.name, method.name);
+    this._queuedInvocations--;
     self.connecting = false;
     self.removeListener('error', onError);
     self.removeListener('connected', onConnected);
@@ -2575,7 +2601,11 @@ function(obj, args) {
 
   if (!this.connecting) {
     debug('Connecting datasource %s to connector %s', this.name, this.connector.name);
-    this.connect();
+    // When no callback is provided to `connect()`, it returns a Promise.
+    // We are not handling that promise and thus UnhandledPromiseRejection
+    // warning is triggered when the connection could not be established.
+    // We are avoiding this problem by providing a no-op callback.
+    this.connect(() => {});
   }
   return true;
 };

test/datasource.test.js

@@ -457,4 +457,80 @@ describe('DataSource', function() {
       });
     });
   });
+
+  describe('automigrate', () => {
+    it('reports connection errors (immediate connect)', async () => {
+      const dataSource = new DataSource({
+        connector: givenConnectorFailingOnConnect(),
+      });
+      dataSource.define('MyModel');
+      await dataSource.automigrate().should.be.rejectedWith(/test failure/);
+    });
+
+    it('reports connection errors (lazy connect)', () => {
+      const dataSource = new DataSource({
+        connector: givenConnectorFailingOnConnect(),
+        lazyConnect: true,
+      });
+      dataSource.define('MyModel');
+      return dataSource.automigrate().should.be.rejectedWith(/test failure/);
+    });
+
+    function givenConnectorFailingOnConnect() {
+      return givenMockConnector({
+        connect: function(cb) {
+          process.nextTick(() => cb(new Error('test failure')));
+        },
+        automigrate: function(models, cb) {
+          cb(new Error('automigrate should not have been called'));
+        },
+      });
+    }
+  });
+
+  describe('autoupdate', () => {
+    it('reports connection errors (immediate connect)', async () => {
+      const dataSource = new DataSource({
+        connector: givenConnectorFailingOnConnect(),
+      });
+      dataSource.define('MyModel');
+      await dataSource.autoupdate().should.be.rejectedWith(/test failure/);
+    });
+
+    it('reports connection errors (lazy connect)', () => {
+      const dataSource = new DataSource({
+        connector: givenConnectorFailingOnConnect(),
+        lazyConnect: true,
+      });
+      dataSource.define('MyModel');
+      return dataSource.autoupdate().should.be.rejectedWith(/test failure/);
+    });
+
+    function givenConnectorFailingOnConnect() {
+      return givenMockConnector({
+        connect: function(cb) {
+          process.nextTick(() => cb(new Error('test failure')));
+        },
+        autoupdate: function(models, cb) {
+          cb(new Error('autoupdate should not have been called'));
+        },
+      });
+    }
+  });
 });
+
+function givenMockConnector(props) {
+  const connector = {
+    name: 'loopback-connector-mock',
+    initialize: function(ds, cb) {
+      ds.connector = connector;
+      if (ds.settings.lazyConnect) {
+        cb(null, false);
+      } else {
+        connector.connect(cb);
+      }
+    },
+    ...props,
+  };
+  return connector;
+}