callAsync alternatives

[edemaine]

I was reading about the new Meteor.callAsync in Meteor 2.8, and specifically #12196 (comment) by @zodern, which I strongly agree with.

First I'd like to argue that the current behavior of Meteor.callAsync is extremely difficult to work with. It's pretty common to trigger a method call (with stubs) on the client in an event handler. A second event could easily happen while the previous method call is still running. And this is now invalid. I actually can't think of a situation where I could safely use the current Meteor.callAsync without writing my own queue system. This seems like we're making the common case much more difficult.

I have a suggestion for instead making recursive Meteor.call[Async] (where one method gets called during the execution of another method stub, something I do do but rarely) more difficult but still possible:

1. Meteor.callAsync provides to the method function a handle to identify the call. this already does this.
2. To call an async method within an async method stub, you must specify this handle to Meteor.callAsync. Actually, this.callAsync(...args) would probably make the most sense, or Meteor.callAsync.call(this, ...args).

Then the default meaning of Meteor.callAsync (without specifying the parent) would be to start a new call, which adds it to a global queue. If a user accidentally attempts a recursive call this way, the queue would hang, so the mistake would (hopefully) be clear. (We could add logs after a timeout to help debug as well.) But by providing the parent, it's understood that this is a recursive method call within a method stub, so this is still possible (just slightly harder).

[radekmie]

If a user accidentally attempts a recursive call this way, the queue would hang, so the mistake would (hopefully) be clear.

I don't get this part. Is it actually possible for it to hang indefinitely? I thought the worse thing is an incorrect state, i.e., a false positive error.

[edemaine]

Consider an async parent method that asynchronously calls a child method:

Meteor.methods({
  async parent() {
    await Meteor.callAsync('child');
  }
});

The current behavior will correctly call the child method within the parent method, but the current behavior makes it invalid to call multiple top-level methods at once.

The behavior I'm proposing is that Meteor.callAsync('child') would queue a child call to happen after the currently running method (or more precisely its stub) parent completes. This will never happen, because parent is awaiting the completion of child. Instead I'm proposing that this code be written as follows, so that Meteor knows that child is getting called from within parent:

Meteor.methods({
  async parent() {
    await this.callAsync('child');
  }
});

It's important to note, as in #12196 (comment), that calling methods within methods is probably fairly rare (though I've certainly done it once or twice, so it's nice to support), so I think changing the API for this case (and only for callAsync/applyAsync) seems like a fine price. On the other hand, calling multiple top-level methods at once is probably almost universal in all front-end apps — the current behavior doesn't support this at all, and my proposal makes this easy.

[Torgen]

if you can't call a stub while a stub is running, there should be a reactive data source you can query to find out if a stub is running.

[make-github-pseudonymous-again]

It's pretty common to trigger a method call (with stubs) on the client in an event handler. (@edemaine)

and

On the other hand, calling multiple top-level methods at once is probably almost universal in all front-end apps — the current behavior doesn't support this at all, and my proposal makes this easy. (@edemaine)

Is there a reason those common scenarios were overlooked? Is fixing this on the roadmap?

[denihs]

Hi, all this was a pretty early development.

I recently fixed the tests inside the ddp-client package and encountered a problem around this.

After changing some things, now we can call callAsync like this:

if (Meteor.isServer) {
  const coll = new Mongo.Collection('test');
  Meteor.methods({
    async insertData(data) {
      const id = await coll.insertAsync(data);
      console.log('inserted with: ', id);
      return id;
    },
    async updateData(id, data) {
      console.log('before update:', await Meteor.callAsync('getData', id));
      const r = await coll.updateAsync(id, { $set: data });
      console.log('after update:', await Meteor.callAsync('getData', id));
      return r;
    },
    async getData(id) {
      const data = await coll.findOneAsync(id);
      await coll.updateAsync(id, { $set: { gotData: true } });
      return data;
    },
  });
}
if (Meteor.isClient) {
    Meteor.callAsync('insertData', {a: 1})
      .then(async result => {
        const id = await result.stubValuePromise;
        return Meteor.callAsync('updateData', id, {a: 2});
      })
      .then(async result => {
        const count = await result.stubValuePromise;
        console.log('update result', count);
      });
}

The results are:

SERVER:
> inserted with:  mK3H62szCikeXsmTE
> before update: { _id: 'mK3H62szCikeXsmTE', a: 1 }
> after update: { _id: 'mK3H62szCikeXsmTE', a: 2, gotData: true }

CLIENT:
> update result 1

As you can see, in the server I have Meteor.callAsync() inside the updateData method, which calls a method with another stub inside.

In the client, the only difference is that I receive an object with a stubValuePromise property, and if I await for that property to finish, I get the stub value.

This is not available yet, but it'll be in the first alpha.

As for the why we have to do like this, it'll be explained in the docs. This may not be the final version, so it can change. But the good news is that it worked and we don't need to worry about queues and stuff.

[sebastianspiller]

Hello there,

is there an update to this issue?

We have started to asyncify our code and encounter many methods, which call methods, because it was much easier this way while implementing.

Moving everything into Meteor.isServer as @denihs proposed would mean to leave the simulation feature of meteor behind, which would be a pity (smooth loading).

[denihs]

Right now, we're moving with the solution on this PR. Improvements will be made in the future if needed, of course!