app.setup() should be async

[zaro]

I have feathers app that uses sequelize for storage. For the tests, I use sqlite database, which is removed before the test starts, and is created in sequelize.js file as generated by feathers generate service. This all worked fine until the number of models increased beyond 10 ( the number is dependent on how fast the machine is). Now every time I run the full test suite the first couple of tests fail because the DB sync hasn't completed yet and it happens so that the first tests that get ran are the last tables being created.

As a simple workaround , I changed the before of app.test.js(it's the first test being executed) like so :

--- a/core/test/app.test.js
+++ b/core/test/app.test.js
@@ -18,7 +18,7 @@ const getUrl = pathname => url.format({
 describe('Feathers application tests', () => {
   before(function(done) {
     this.server = app.listen(port);
-    this.server.once('listening', () => done());
+    this.server.once('listening', () => setTimeout( ()=> done(), 1000));
   });

This fixes it for now until the number of models grows of the test is executed on a slower machine.
The real problem is that app.listen() calls app.setup(), but doesn't wait for it to complete before firing the listening event. The database is created in app.setup and can take some time.

I think app.setup() should be async and app.listen() should wait for it to complete, or else there will always be possibility for race conditions between setup() and service requiring the setup() to complete before it's functional.

[daffl]

It should indeed and will probably be addressed at the same time as #509

[ganlub]

@daffl Any plans on addressing that? It would be nice to have some insight on how this could be solved and maybe someone could do an MR about.

If there's an official way to fix this, it could also be documented 💃. I went over #509 on the past with no luck.

[daffl]

Yes. This is related to feathersjs-ecosystem/commons#72. Basically app.setup and service.setup should return a promise and be hook-able and app level setup hooks should only run once.

Even though it isn't really a big API change it'd unfortunately be a breaking change (setup used to return the app instead of a promise).

[ganlub]

@daffl I understand. I think this would justify the jump to v4, it's quite important.
Although those could be maintained and provide something like .setupAsync or so. What do you think?

[aercolino]

Last week I got bitten too: my app ran smoothly, but its tests threw errors when re-building the database in the before hook of mocha.

The problem with the generated Sequelize adapter is that it calls sequelize.sync but doesn't handle the returned promise.

Here is my workaround (without changing what app.setup returns to app.listen):

src/sequelize.js

- sequelize.sync();
+ app.set('sequelizeSync', sequelize.sync());

src/index.js

- server.on('listening', () =>
-   logger.info('Feathers application started on http://%s:%d', app.get('host'), port)
- );
+ server.on('listening', () => {
+   app.get('sequelizeSync')
+     .then(() => logger.info('Feathers application started on http://%s:%d', app.get('host'), port))
+     .catch(reason => {
+       console.log(reason);
+       process.exit(1);
+     });
+ });

[KrishnaPG]

One way to achieve this without breaking the existing setup call would be to initialize all the services normally, as being done currently, but block the method calls till the service initialization is complete.

This has the additional advantage of:

1. service being self-regulated and
2. in combination with built-in retries provides a powerful mechanism for cross-dependent services to auto-regulate amongst themselves.

How does it work: consider a simple service that loads some data over network and provides services on top of it, as below:

const myService = {
  async get(id) { return this.data[id]; },

  setup(app, path) {
    // some async/delayed work, such as third-party data retrieval, connecting to db etc.
    axios.get("<thirdparty-api").then(resp => this.data = resp.data);
  }
}

When get method is called, the resource may not be available. To tackle the issue, service maintains a serviceAvailable built-in observable that controls the get (and all other functions) availability as below:

const myService = {
  async get(id) { return this.data[id]; },

  setup(app, path) {
    this._originalFns = { get: this.get, create: this.create, ...<etc.> }
    this._serviceAvailable = false; // triggers disable methods

    axios.get("<thirdparty-api").then(resp => {
       this.data = resp.data;
       this.__serviceAvailable  = true; // triggers enable methods
     )};
  }
}
disableMethods(svc) {
     svc.get = svc.create = .... = () => new Error(`Not Yet Initialized`);
}
enableMethods(svc) {
     svc.get = svc._originalFns.get;
     svc.create = svc._originalFns.create;
     // ... so on
}

When get and other functions are called before the data is available they will receive an error. It could be a custom feathers error, something like NotInitialized etc..

When services are dependent on each other, with the help of built-in retries they could wait for the dependency service to become available before proceeding (i.e. keep retrying while NotInitialized is received)

Since the serviceAvailable is an observable, it could be set to true or false anytime to control the service availability, such as below:

  get(id) { 
     return axios.get("<some third party>").then(resp => resp.data[id])
       .catch(ex => {
          this._serviceAvailable = false; // triggers disable methods
          throw ex;
       });      
  },

or something like

const myService = {
    setup() {
         myotherservice.on("connected", () => this._serviceAvailable = true);
         myotherservice.on("disconnected", () => this._serviceAvailable = false);
    }
}

When many microservices are interacting with each other with complex dependencies, this kind of self-regulation and dependency-based regulation creates flexibility to the designer since it allows them to control the life-time and availability of a service at the granular level.

To achieve this above, Feathers, as a framework, has to:

1. provide the serviceAvailable functionality as a built in for every service and take care of enabling/disabling the service methods whenever the availability changes,
2. the existing services should be run without modifications. i.e. by default this._serviceAvailable will be set to true before calling the services setup method.