- fix some accounts tests

- bring zodern's solution to async stubs

packages/accounts-base/accounts_tests.js

@@ -238,7 +238,7 @@ Tinytest.addAsync('accounts - login token', async (test) => {
   connection = DDP.connect(Meteor.absoluteUrl());
   // evil plan foiled
   await test.throwsAsync(
-    async () => await connection.callAsync('login', { returnStubValue: true }, { resume: stolenToken2 }),
+    async () => await connection.callAsync('login', { resume: stolenToken2 }),
     /You\'ve been logged out by the server/
   );
   connection.disconnect();
@@ -251,7 +251,7 @@ Tinytest.addAsync('accounts - login token', async (test) => {
   const stampedToken2 = Accounts._generateStampedLoginToken();
   await insertUnhashedLoginToken(userId4, stampedToken2);
   connection = DDP.connect(Meteor.absoluteUrl());
-  await connection.callAsync('login', { returnStubValue: true }, { resume: stampedToken2.token });
+  await connection.callAsync('login', { resume: stampedToken2.token });
   connection.disconnect();
 
   // The token is no longer available to be stolen.
@@ -296,15 +296,15 @@ Tinytest.addAsync('accounts - get new token', async test => {
     await Accounts._insertLoginToken(userId, stampedToken);
 
     const conn = DDP.connect(Meteor.absoluteUrl());
-    await conn.callAsync('login', { returnStubValue: true }, { resume: stampedToken.token });
-    test.equal(await conn.callAsync('getCurrentLoginToken', { returnServerPromise: true }),
+    await conn.callAsync('login', { resume: stampedToken.token });
+    test.equal(await conn.callAsync('getCurrentLoginToken'),
       Accounts._hashLoginToken(stampedToken.token));
 
-    const newTokenResult = await conn.callAsync('getNewToken', { returnServerPromise: true });
+    const newTokenResult = await conn.callAsync('getNewToken');
     test.equal(newTokenResult.tokenExpires,
       Accounts._tokenExpiration(stampedToken.when));
-    const token = await conn.callAsync('getCurrentLoginToken', { returnServerPromise: true });
-    test.equal(await conn.callAsync('getCurrentLoginToken', { returnServerPromise: true }),
+    const token = await conn.callAsync('getCurrentLoginToken');
+    test.equal(await conn.callAsync('getCurrentLoginToken'),
       Accounts._hashLoginToken(newTokenResult.token));
     conn.disconnect();
 
@@ -329,7 +329,7 @@ Tinytest.addAsync('accounts - remove other tokens', async (test) => {
       await Accounts._insertLoginToken(userId, stampedTokens[i]);
       const conn = DDP.connect(Meteor.absoluteUrl());
       await conn.callAsync('login', { resume: stampedTokens[i].token });
-      test.equal(await conn.callAsync('getCurrentLoginToken', { returnServerPromise: true }),
+      test.equal(await conn.callAsync('getCurrentLoginToken'),
         Accounts._hashLoginToken(stampedTokens[i].token));
       conns.push(conn);
     }
@@ -339,7 +339,7 @@ Tinytest.addAsync('accounts - remove other tokens', async (test) => {
     simplePoll(async () => {
         let tokens = [];
         for (const conn of conns) {
-          tokens.push(await conn.callAsync('getCurrentLoginToken', { returnServerPromise: true }));
+          tokens.push(await conn.callAsync('getCurrentLoginToken'));
         }
         return !tokens[1] &&
           tokens[0] === Accounts._hashLoginToken(stampedTokens[0].token);
@@ -381,18 +381,18 @@ Tinytest.addAsync(
     // On a new connection, Meteor.userId() should be null until logged in.
     let validateAttemptExpectedUserId = null;
     const onLoginExpectedUserId = userId;
-    await conn.callAsync('login', { returnServerPromise: true }, { resume: stampedToken.token });
+    await conn.callAsync('login', { resume: stampedToken.token });
 
     // Now that the user is logged in on the connection, Meteor.userId() should
     // return that user.
     validateAttemptExpectedUserId = userId;
-    await conn.callAsync('login', { returnServerPromise: true }, { resume: stampedToken.token });
+    await conn.callAsync('login', { resume: stampedToken.token });
 
     // Trigger onLoginFailure callbacks
     const onLoginFailureExpectedUserId = userId;
     await test.throwsAsync(
       async () =>
-        await conn.callAsync('login', { returnServerPromise: true }, { resume: "bogus" }), '403');
+        await conn.callAsync('login', { resume: "bogus" }), '403');
 
     // Trigger onLogout callbacks
     const onLogoutExpectedUserId = userId;
@@ -437,23 +437,23 @@ Tinytest.addAsync(
 
     // test a new connection
     let allowLogin = true;
-    await conn.callAsync('login', { returnServerPromise: true }, { resume: stampedToken.token });
+    await conn.callAsync('login', { resume: stampedToken.token });
 
     // Now that the user is logged in on the connection, Meteor.userId() should
     // return that user.
-    await conn.callAsync('login', { returnServerPromise: true }, { resume: stampedToken.token });
+    await conn.callAsync('login', { resume: stampedToken.token });
 
     // Trigger onLoginFailure callbacks, this will not include the user object
     allowLogin = 'bogus';
     await test.throwsAsync(
       async () =>
-        await conn.callAsync('login', { returnServerPromise: true }, { resume: "bogus" }), '403');
+        await conn.callAsync('login', { resume: "bogus" }), '403');
 
     // test a forced login fail which WILL include the user object
     allowLogin = false;
     await test.throwsAsync(
       async () =>
-        await conn.callAsync('login', { returnServerPromise: true }, { resume: stampedToken.token }), '403');
+        await conn.callAsync('login', { resume: stampedToken.token }), '403');
 
     // Trigger onLogout callbacks
     const onLogoutExpectedUserId = userId;

packages/accounts-password/password_tests.js

@@ -292,7 +292,7 @@ if (Meteor.isClient) (() => {
     },
     // Make sure the new user has not been inserted
     async function (test) {
-      const result = await Meteor.callAsync('countUsersOnServer', { returnServerPromise: true }, {
+      const result = await Meteor.callAsync('countUsersOnServer', {
         username: this.newUsername,
       });
       test.equal(result, 0);
@@ -400,7 +400,7 @@ if (Meteor.isClient) (() => {
     },
     // Make sure the new user has not been inserted
     async function (test) {
-      const result = await Meteor.callAsync('countUsersOnServer', { returnServerPromise: true }, {
+      const result = await Meteor.callAsync('countUsersOnServer', {
         'emails.address': this.newEmail,
       });
       test.equal(result, 0);
@@ -428,7 +428,7 @@ if (Meteor.isClient) (() => {
         }));
     },
     async function (test) {
-      const token = await Meteor.callAsync("getResetToken", { returnServerPromise: true });
+      const token = await Meteor.callAsync("getResetToken");
       test.isTrue(token);
       this.token = token;
     },
@@ -452,7 +452,7 @@ if (Meteor.isClient) (() => {
         loggedInAs(this.username, test, expect));
     },
     async function (test) {
-      const token = await Meteor.callAsync("getResetToken", { returnServerPromise: true });
+      const token = await Meteor.callAsync("getResetToken");
       test.isFalse(token);
     },
     logoutStep,
@@ -1253,7 +1253,7 @@ if (Meteor.isServer) (() => {
           test.fail('observe should be gone');
       })
 
-      const result = await clientConn.callAsync('login', { returnServerPromise: true }, {
+      const result = await clientConn.callAsync('login', {
         user: { username: username },
         password: hashPassword('password')
       });

packages/ddp-client/client/client.js

@@ -4,3 +4,4 @@ import '../common/livedata_connection';
 
 // Initialize the default server connection and put it on Meteor.connection
 import './client_convenience';
+import './queueStubsHelpers';

packages/ddp-client/client/queueStubsHelpers.js

@@ -0,0 +1,216 @@
+import { DDP } from '../common/namespace.js';
+import { isEmpty, last } from "meteor/ddp-common/utils.js";
+
+// https://forums.meteor.com/t/proposal-to-fix-issues-with-async-method-stubs/60826
+
+let queueSize = 0;
+let queue = Promise.resolve();
+
+function queueFunction(fn, promiseProps = {}) {
+  queueSize += 1;
+
+  let resolve;
+  let reject;
+  const promise = new Promise((_resolve, _reject) => {
+    resolve = _resolve;
+    reject = _reject;
+  });
+
+  queue = queue.finally(() => {
+    fn(resolve, reject);
+    return promise;
+  });
+
+  promise.finally(() => {
+    queueSize -= 1;
+    if (queueSize === 0) {
+      Meteor.connection._maybeMigrate();
+    }
+  });
+
+  promise.stubPromise = promiseProps.stubPromise;
+  promise.serverPromise = promiseProps.serverPromise;
+
+  return promise;
+}
+
+let oldReadyToMigrate = Meteor.connection._readyToMigrate;
+Meteor.connection._readyToMigrate = function () {
+  if (queueSize > 0) {
+    return false;
+  }
+
+  return oldReadyToMigrate.apply(this, arguments);
+};
+
+let currentMethodInvocation = null;
+
+/**
+ * Meteor sets CurrentMethodInvocation to undefined for the reasons explained at
+ * https://github.com/meteor/meteor/blob/c9e3551b9673a7ed607f18cb1128563ff49ca96f/packages/ddp-client/common/livedata_connection.js#L578-L605
+ * The app code could call `.then` on a promise while the async stub is running,
+ * causing the `then` callback to think it is inside the stub.
+ *
+ * With the queueing we are doing, this is no longer necessary. The point
+ * of the queueing is to prevent app/package code from running while
+ * the stub is running, so we don't need to worry about this.
+ */
+
+let oldApplyAsync = Meteor.connection.applyAsync;
+Meteor.connection.applyAsync = function () {
+  let args = arguments;
+  let name = args[0];
+
+  if (currentMethodInvocation) {
+    DDP._CurrentMethodInvocation._set(currentMethodInvocation);
+    currentMethodInvocation = null;
+  }
+
+  const enclosing = DDP._CurrentMethodInvocation.get();
+  const alreadyInSimulation = enclosing?.isSimulation;
+  const isFromCallAsync = enclosing?._isFromCallAsync;
+
+  if (
+    Meteor.connection._getIsSimulation({
+      isFromCallAsync,
+      alreadyInSimulation,
+    })
+  ) {
+    // In stub - call immediately
+    return oldApplyAsync.apply(this, args);
+  }
+
+  const applyAsyncPromise = oldApplyAsync.apply(this, args);
+  return queueFunction(
+    (resolve, reject) => {
+      let finished = false;
+
+      Meteor._setImmediate(() => {
+        applyAsyncPromise
+          .then((result) => {
+            finished = true;
+            resolve(result);
+          })
+          .catch((err) => {
+            finished = true;
+            reject(err);
+          });
+      });
+
+      Meteor._setImmediate(() => {
+        if (!finished) {
+          console.warn(
+            `Method stub (${name}) took too long and could cause unexpected problems. Learn more at https://github.com/zodern/fix-async-stubs/#limitations`
+          );
+        }
+      });
+    },
+    {
+      stubPromise: applyAsyncPromise.stubPromise,
+      serverPromise: applyAsyncPromise.serverPromise,
+    }
+  );
+};
+
+let oldApply = Meteor.connection.apply;
+Meteor.connection.apply = function () {
+  // [name, args, options]
+  let options = arguments[2] || {};
+  let wait = options.wait;
+
+  // Apply runs the stub before synchronously returning.
+  //
+  // However, we want the server to run the methods in the original call order
+  // so we have to queue sending the message to the server until any previous async
+  // methods run.
+  // This does mean the stubs run in a different order than the methods on the
+  // server.
+  // TODO: can we queue Meteor.apply in some situations instead of running
+  // immediately?
+
+  let oldOutstandingMethodBlocks = Meteor.connection._outstandingMethodBlocks;
+  // Meteor only sends the method if _outstandingMethodBlocks.length is 1.
+  // Add a wait block to force Meteor to put the new method in a second block.
+  let outstandingMethodBlocks = [{ wait: true, methods: [] }];
+  Meteor.connection._outstandingMethodBlocks = outstandingMethodBlocks;
+
+  let result;
+  try {
+    result = oldApply.apply(this, arguments);
+  } finally {
+    Meteor.connection._outstandingMethodBlocks = oldOutstandingMethodBlocks;
+  }
+
+  if (outstandingMethodBlocks[1]) {
+    let methodInvoker = outstandingMethodBlocks[1].methods[0];
+
+    if (methodInvoker) {
+      queueMethodInvoker(methodInvoker, wait);
+    }
+  }
+
+  return result;
+};
+
+function queueMethodInvoker(methodInvoker, wait) {
+  queueFunction((resolve) => {
+    let self = Meteor.connection;
+    // based on https://github.com/meteor/meteor/blob/e0631738f2a8a914d8a50b1060e8f40cb0873680/packages/ddp-client/common/livedata_connection.js#L833-L853C1
+    if (wait) {
+      // It's a wait method! Wait methods go in their own block.
+      self._outstandingMethodBlocks.push({
+        wait: true,
+        methods: [methodInvoker],
+      });
+    } else {
+      // Not a wait method. Start a new block if the previous block was a wait
+      // block, and add it to the last block of methods.
+      if (
+        isEmpty(self._outstandingMethodBlocks) ||
+        last(self._outstandingMethodBlocks).wait
+      ) {
+        self._outstandingMethodBlocks.push({
+          wait: false,
+          methods: [],
+        });
+      }
+
+      last(self._outstandingMethodBlocks).methods.push(methodInvoker);
+    }
+
+    // If we added it to the first block, send it out now.
+    if (self._outstandingMethodBlocks.length === 1) methodInvoker.sendMessage();
+
+    resolve();
+  });
+}
+
+/**
+ * Queue subscriptions in case they rely on previous method calls
+ */
+let queueSend = false;
+let oldSubscribe = Meteor.connection.subscribe;
+Meteor.connection.subscribe = function () {
+  queueSend = true;
+  try {
+    return oldSubscribe.apply(this, arguments);
+  } finally {
+    queueSend = false;
+  }
+};
+
+let oldSend = Meteor.connection._send;
+Meteor.connection._send = function () {
+  if (!queueSend) {
+    return oldSend.apply(this, arguments);
+  }
+
+  queueSend = false;
+  queueFunction((resolve) => {
+    try {
+      oldSend.apply(this, arguments);
+    } finally {
+      resolve();
+    }
+  });
+};

packages/ddp-client/common/livedata_connection.js

@@ -579,71 +579,7 @@ export class Connection {
       );
     }
 
-    const applyOptions = ['returnStubValue', 'returnServerResultPromise', 'returnServerPromise'];
-    const defaultOptions = {
-      returnServerResultPromise: true,
-    };
-    const options = {
-      ...defaultOptions,
-      ...(applyOptions.some(o => args[0]?.hasOwnProperty(o))
-        ? args.shift()
-        : {}),
-    };
-
-    const invocation = DDP._CurrentCallAsyncInvocation.get();
-
-    if (invocation?.hasCallAsyncParent) {
-      return this.applyAsync(name, args, { ...options, isFromCallAsync: true });
-    }
-
-    /*
-    * This is necessary because when you call a Promise.then, you're actually calling a bound function by Meteor.
-    *
-    * This is done by this code https://github.com/meteor/meteor/blob/17673c66878d3f7b1d564a4215eb0633fa679017/npm-packages/meteor-promise/promise_client.js#L1-L16. (All the logic below can be removed in the future, when we stop overwriting the
-    * Promise.)
-    *
-    * When you call a ".then()", like "Meteor.callAsync().then()", the global context (inside currentValues)
-    * will be from the call of Meteor.callAsync(), and not the context after the promise is done.
-    *
-    * This means that without this code if you call a stub inside the ".then()", this stub will act as a simulation
-    * and won't reach the server.
-    *
-    * Inside the function _getIsSimulation(), if isFromCallAsync is false, we continue to consider just the
-    * alreadyInSimulation, otherwise, isFromCallAsync is true, we also check the value of callAsyncMethodRunning (by
-    * calling DDP._CurrentMethodInvocation._isCallAsyncMethodRunning()).
-    *
-    * With this, if a stub is running inside a ".then()", it'll know it's not a simulation, because callAsyncMethodRunning
-    * will be false.
-    *
-    * DDP._CurrentMethodInvocation._set() is important because without it, if you have a code like:
-    *
-    * Meteor.callAsync("m1").then(() => {
-    *   Meteor.callAsync("m2")
-    * })
-    *
-    * The call the method m2 will act as a simulation and won't reach the server. That's why we reset the context here
-    * before calling everything else.
-    *
-    * */
-    DDP._CurrentMethodInvocation._set();
-    DDP._CurrentMethodInvocation._setCallAsyncMethodRunning(true);
-    let applyAsyncPromise = {};
-    const promise = new Promise((resolve, reject) => {
-      DDP._CurrentCallAsyncInvocation._set({ name, hasCallAsyncParent: true });
-      const p = this.applyAsync(name, args, { isFromCallAsync: true, ...options })
-      p.then(resolve)
-        .catch(reject)
-        .finally(() => {
-          DDP._CurrentCallAsyncInvocation._set();
-          DDP._CurrentMethodInvocation._setCallAsyncMethodRunning(false);
-        });
-      applyAsyncPromise = p;
-    });
-
-    promise.stubPromise = applyAsyncPromise.stubPromise;
-    promise.serverPromise = applyAsyncPromise.serverPromise;
-
-    return promise;
+    return this.applyAsync(name, args, { returnServerResultPromise: true });
   }
 
   /**

packages/ddp-client/test/livedata_tests.js

@@ -1213,11 +1213,13 @@ testAsyncMulti('livedata - methods with nested stubs', [
   },
 ]);
 
- Tinytest.addAsync('livedata - isAsync call', async function (test) {
-  Meteor.call('isCallAsync', (err, result) => test.equal(result, false))
-  const result = await Meteor.callAsync('isCallAsync', { returnStubValue: true })
-  test.equal(result, true)
-})
+// TODO [FIBERS] - check if this still makes sense to have
+
+//  Tinytest.addAsync('livedata - isAsync call', async function (test) {
+//   Meteor.call('isCallAsync', (err, result) => test.equal(result, false))
+//   const result = await Meteor.callAsync('isCallAsync', { returnStubValue: true })
+//   test.equal(result, true)
+// })
 
 // XXX some things to test in greater detail:
 // staying in simulation mode