veliovgroup / flow-router

🚦 Carefully extended flow-router for Meteor
https://packosphere.com/ostrio/flow-router-extra
BSD 3-Clause "New" or "Revised" License
202 stars 29 forks source link

Meteor methods with client stubs not supported for waitOn()? Promise not resolved Error: Can't set timers inside simulations #112

Open drone1 opened 5 months ago

drone1 commented 5 months ago

I'm having an issue:

This does not work for me:

methods.js (imported on both client and server):

// METEOR METHOD WITH CLIENT STUB
Meteor.methods({
    async 'simulation.test'() {
        if (Meteor.isServer) {
            const slowOperation = () => new Promise(resolve => Meteor.setTimeout(resolve, 1000))
            return await slowOperation()
        }
    }
})

routes.js, imported on the client:

FlowRouter.route('/', {
        name: 'test',
        waitOn(params, query, ready) {
            return new Promise(async (resolve, reject) => {
                try {
                    console.log('calling simulation.test...')
                    await Meteor.callAsync('simulation.test')
                    console.log('done.')
                    resolve()
                } catch (err) {
                    reject(err)
                }
            })
        },

        // ...action...
})

Exception thrown, since you are calling Meteor.setTimeout here:

    const subWait = (delay) => {
      timer = Meteor.setTimeout(() => {           // PROBLEM
        if (this.checkSubscriptions(subscriptions)) {
          Meteor.clearTimeout(timer);
          _data = getData();
          if (_resources) {
            whileWaitingAction();
            getResources();
          } else {
            next(current, _data);
          }
        } else {
          wait(24);
        }
      }, delay);
    };

If I change the Meteor method to have no client stub, the exception goes away:

(...routes.js same as above...)

methods.js

if (Meteor.isServer) {
    Meteor.methods({
        async 'simulation.test'() {
            const slowOperation = () => new Promise(resolve => Meteor.setTimeout(resolve, 1000))
            return await slowOperation()
        }
    })
}

Definiing waitOn() as follows instead does NOT seem to cause the exception:

            return new Promise((resolve, reject) => {   // NOTE: No 'async' here
                try {
                    console.log('calling simulation.test...')
                    Meteor.call('simulation.test', (err, result) => {    // NOTE: Meteor.call rather than 'await Meteor.callAsync'
                        console.log('done.')
                        resolve()
                    })
                } catch (err) {
                    reject(err)
                }
            })

Originally I was definiing waitOn() with return new Promise(async (resolve, reject) => ... and calling a ValidatedMethod, which always defines a client stub as far as I can tell, like await myValidatedMethod.callAsync(), which caused this exception.

Would be great if you could support client stubs, but anyway, any suggestions here? Am I doing something unreasonable? Thanks.

dr-dimitru commented 5 months ago

@drone1 generally I'd say — Use stubs only for user-interaction actions. What expected behavior you assuming when waitOn receives stub then server response, page/route reload?

drone1 commented 5 months ago

Hey @dr-dimitru!

Maybe it would be helpful to throw an exception before Meteor.setTimeout() if in simulation? I'm not sure if this is an easy one-liner, since the code is running outside of a method, you may not be able to check isSimulation as you can within a Meteor method. Maybe not worth it.

The behavior I'd expect is to not get an exception thrown by Meteor.setTimeout() being called, and the code moving along as expected as data comes back from the server.

The Meteor docs recommend client stubs for all methods IIRC so I'd gotten into the habit of doing that.

Thanks for getting back.

dr-dimitru commented 2 months ago

@drone1 Do you want to push a PR?