fix: report errors from automigrate/autoupdate

Defer automigrate/autoupdate until we are connected, so that connection
errors can be reported back to callers.

Fix postInit handler to not report connection error to console.log
and via dataSource "error" event in case there is already an operation
queued. When this happens, we want the error to be handled by the
queued operation and reported to its caller.

Signed-off-by: Miroslav Bajtoš <[email protected]>

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('automigrate 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;
+}