0

I have the following code which registers a series of socketio namespaces. PArts of the logic depend on database calls (via sequelize), hence I need to use promises. I want the complete promise to resolve when all the constructor called logic is complete. My issue though is that the complete promise resolves before the emitInitialPackage() function has resolved.

export class demoClass {
    public complete: Promise<boolean>;
    server: any;

    constructor(io: any) {
        this.server = io;
        this.complete = Promise.resolve(db.Line.findAll()).then(lines => {
                // do some mapping to generate routes and cells
                this.registerEndPoints(routes, [], cells);
            }).catch(err => console.log(err))
    }



registerEndPoints(routes: Array<string>, nsps: Array<any>, cells: Array<string>) {
        for (let i = 0; i < routes.length; i++) {
            nsps.push(this.server.of('/api/testNamespace/' + routes[i]));
            let that = this;
            const nsp = nsps[i];
            nsp.on('connection', function (socket) {
                that.emitInitialPackage(nsps[i], routes[i], cells[i]);
            });
        }
    }

    emitInitialPackage(nsp: any, name: string, cell: any) {
        return db.Line.find({/* Some sequelize params*/}).then(results => {
            nsp.emit('value', results);
        }).catch(err => console.log(err));
    }
}

How can I ensure emitInitialPackage has completed before complete resolves?

Cœur
  • 37,241
  • 25
  • 195
  • 267
George Edwards
  • 8,979
  • 20
  • 78
  • 161
  • 1
    Can't use loops with async code like that... Need to loop asynchronously – elclanrs Feb 27 '17 at 16:52
  • @elclanrs OK, so what would an asynchronous loop look like? – George Edwards Feb 27 '17 at 16:53
  • there's a bit to much unclear here? what does `this.server.of()` do/return? where does `cells` come from in the constructor, and what is `nsp` in `registerEndPoints`? why are you waiting for nsp to connect, and then dismiss the socket returned? Why do you do this: `Promise.resolve(db.Line.findAll())`? Either findAll() retuns a promise, or it is sync or it expects a callback-function. In none of these cases you gain anything by wrapping that in a promise. – Thomas Feb 27 '17 at 17:31
  • @Thomas `this.server.of` returns a socketio namespace. As mentioned in the commented line above, cells and routes are mapped from lines. I'll update the code sample to correct this, but `nsp` is a member of `nsps`. Not really sure what you mean by waiting for nsp to connect? `findAll` returns a bluebird promise, so I use Promise.resolve to convert it to an es6 promsie – George Edwards Feb 27 '17 at 17:36
  • @elclanrs, could you explain why one could not do that? Sure, it's painful to watch, but technically there's nothing wrong with that loop. `Need to loop asynchronously` No you don't. At least with what I understand by "async looping" it would only slow down the whole loop without any benefit. – Thomas Feb 27 '17 at 17:36
  • For loops and async code: http://stackoverflow.com/q/13343340/215552 – Heretic Monkey Feb 27 '17 at 18:02
  • 1
    You need to update `registerEndPoints` to return a promise, that completes when all of the `emitInitialPackage` calls complete. Then return that promise as part of the complete call. This allows the promises to flow (or really `map` through) – David Driscoll Feb 27 '17 at 18:51

2 Answers2

1

To wait for completion of all operations in registerEndPoints, this method should return Promise that can be chained after db.Line.findAll() operation:

export class demoClass {
    public complete: Promise<boolean>;
    server: any;

    constructor(io: any) {
        this.server = io;
        this.complete = Promise.resolve(db.Line.findAll()).then(lines => {
            // return promise created by registerEndPoints method
            return this.registerEndPoints(routes, [], cells);
        }).catch(err => console.log(err))
    }   

    registerEndPoints(routes: Array<string>, nsps: Array<any>, cells: Array<string>) {
        const endPointPromises = routes.map((route, index) => {
          // for each endpoint create Promise that gets resolved 
          // only when all work for endpoint is done
          return new Promise((resolve) => {
              nsps.push(this.server.of('/api/testNamespace/' + route));
              const nsp = nsps[index];
              nsp.on('connection', (socket) => {
                // resolve promise when emitInitialPackage did its part of the work
                this.emitInitialPackage(nsps[index], route, cells[index]).then(resolve);
              });          
          });
        });

        return Promise.all(endPointPromises);
    }

    emitInitialPackage(nsp: any, name: string, cell: any) {
        return db.Line.find({/* Some sequelize params*/}).then(results => {
            nsp.emit('value', results);
        }).catch(err => console.log(err));
    }
}
Stubb0rn
  • 1,269
  • 12
  • 17
0

There are several issues to solve:

  • Return the promise returned by this.registerEndPoints, so this.complete will only resolve when that promise resolves;
  • The [] argument passed for the nsps parameter serves no purpose as argument, so you might as well skip that parameter all together;
  • The .on('connection', ...) function should be wrapped to return a promise;
  • The for loop should create these promises, and then pass them to Promise.all for the final result. map can be used for this;
  • It is not a nice structure where you have three arrays (routes, cells, and nsps) that have related data at equal indexes. A better structure is where you have one array of objects, where each object has three properties: (route, cell, and nsp);
  • bluebird is promises/A+ compliant, so there should be no need to convert a bluebird promise to a native JS promise.

Here is some untested code:

constructor(io: any) {
    this.server = io;
    // *** bluebird is promises/A+ compliant, no need to convert it:
    this.complete = db.Line.findAll().then(lines => {
        // do some mapping to generate routes and cells
        // *** return the promise!
        // *** removed [] argument: not needed
        return this.registerEndPoints(routes, cells);
    }).catch(err => console.log(err))
}

// *** Remove the nsps parameter
registerEndPoints(routes: Array<string>, cells: Array<string>) {
    // *** Create a promise-version of the `.on('connection', ...)` method
    function nspConnect(nsp) {
        return new Promise( resolve => nsp.on('connection', resolve) );
    }
    let that = this;
    // *** Combine each route, cell, and nsp in one object, and put in array:
    const data = routes.map( (route, i) => ({
        nsp: that.server.of('/api/testNamespace/' + route),
        route,
        cell: cells[i]
    }) );
    // *** Map the array of objects to promises
    const proms = data.map( ({nsp, route, cell}) =>
        nspConnect(nsp).then(that.emitInitialPackage.bind(that, nsp, route, cell)) );
    // *** Return a promise that resolves when all these have resolved
    return Promise.all(proms); 
}

To have some debugging possibilities, you could expand the nspConnect function to:

function nspConnect(nsp) {
    return new Promise( resolve => { 
        console.log('creating promise'); 
        return nsp.on('connection', socket => { 
            console.log('resolving'); 
            resolve();
        }); 
    });
}
trincot
  • 317,000
  • 35
  • 244
  • 286
  • Thank you for this! Could you explain the line in `nspConnect(nsp)` what is `resolve`? – George Edwards Feb 27 '17 at 20:37
  • `nspConnect` is a function that returns a promise. That promise resolves when the callback to `nsp.on` is called. The `Promise` constructor always passes two functions as arguments to the callback provided to it: `resolve` and `reject`. They are handles given by the promise internals to either resolve the promise or reject it. I ignored the second. I provide the `resolve` function as callback to `nsp.on`, so that this `resolve` is called when the connection is made. As this `resolve` function gets called, it will resolve the `new Promise`. – trincot Feb 27 '17 at 21:29
  • but why pass `resolve` in as an argument for `nsp.on(...` ? – George Edwards Feb 27 '17 at 22:20
  • Because the `on` function will invoke the second argument (callback) when it has done its thing, and when it does, I want to have `resolve` executed (because then it is time for the promise to resolve), so that is why I pass it to `on`. – trincot Feb 27 '17 at 22:23
  • OK, what would `emitInitialPackage` with this solution? – George Edwards Feb 27 '17 at 22:25
  • Sorry, I don't understand the question? – trincot Feb 27 '17 at 22:31
  • Well you've bound a new context and the nsp to it. So how are you seeing this being used? – George Edwards Feb 27 '17 at 22:54
  • `bind` creates a new function that will get the specified context and arguments once it gets called. This is nice to pass as callback function. It is equivalent to passing an inline anonymous function: `then( _ => that.emitInitialPackage(nsp, route, cell))`. NB: I just made a correction in the `data.map` callback: I had `prom` as an argument, which should not have been there. It was a left-over of a previous version I had typed. – trincot Feb 27 '17 at 22:59
  • OK, Just to let you know, this isn't currently working. The promises are creating properly, but aren't resolving. I am not sure that `nsp.on` actually executes? – George Edwards Feb 28 '17 at 10:32
  • I added a debugging version of that function in my answer, so you can see in the console whether the promises resolve. I also changed a `this` to `that` in the `routes.map` callback, although it should work with `this` as well since it is an arrow function. – trincot Feb 28 '17 at 10:44
  • Yes, so that confirms what I thought, I get "creating promise" logged out 3 times and no "resolving"... I'll have a look at the socketio odcs to better understand the `nsp.on(` function. DO you have any ideas though? – George Edwards Feb 28 '17 at 10:47
  • According to the docs, the `.on("connection", ...)` fires when a client gets connected to the corresponding namespace. Are you sure a connection is made while running the code? You could debug to see if there are any clients after a timeout of 10 seconds with [this code](https://www.npmjs.com/package/socket.io#namespaceclientsfnfunction) wrapped in a `setTimeout(..., 10000)`. If you get an empty array, it confirms there are no clients. – trincot Feb 28 '17 at 12:53
  • Hmm, Well the issue is that the client connects once the `complete` promise resolves... However, the issue I was facing at the beginning is that if the client connects before the `on('connection` event has been registered, then my test timesout! – George Edwards Feb 28 '17 at 12:57
  • That seems like a cat trying to catch its tail: if you are waiting for new connections, but the client only connects after you have found a new client connection, then obviously something is wrong with the logic you have in mind. So the question is: why do you listen for a new connection to be made in the first place? You'll need to look into that timeout issue again. Waiting for a connection is surely not the solution to it. What is your requirement for the test: to have one connection, not necessarily a new one? – trincot Feb 28 '17 at 13:09