From a5b0bb515fe53a359a3a62efb673a0353132cc04 Mon Sep 17 00:00:00 2001 From: Gabriel Grubba <70247653+Grubba27@users.noreply.github.com> Date: Tue, 21 Nov 2023 16:42:10 -0300 Subject: [PATCH 1/3] Add zodern solution --- .../ddp-client/client/client_convenience.js | 175 +++++++++++++- packages/ddp-client/package.js | 2 + .../ddp-client/test/async_stubs/client.js | 225 ++++++++++++++++++ .../test/async_stubs/server_setup.js | 49 ++++ 4 files changed, 450 insertions(+), 1 deletion(-) create mode 100644 packages/ddp-client/test/async_stubs/client.js create mode 100644 packages/ddp-client/test/async_stubs/server_setup.js diff --git a/packages/ddp-client/client/client_convenience.js b/packages/ddp-client/client/client_convenience.js index 35bd3bb480..c747ad9587 100644 --- a/packages/ddp-client/client/client_convenience.js +++ b/packages/ddp-client/client/client_convenience.js @@ -1,6 +1,6 @@ import { DDP } from '../common/namespace.js'; import { Meteor } from 'meteor/meteor'; - +import { isEmpty, last } from "meteor/ddp-common/utils.js"; // Meteor.refresh can be called on the client (if you're in common code) but it // only has an effect on the server. Meteor.refresh = () => {}; @@ -42,6 +42,179 @@ Meteor.connection = DDP.connect(ddpUrl, { onDDPVersionNegotiationFailure: onDDPVersionNegotiationFailure }); + +// https://forums.meteor.com/t/proposal-to-fix-issues-with-async-method-stubs/60826 + +let queueSize = 0; +let queue = Promise.resolve(); + +function queueFunction(fn) { + queueSize += 1; + + let resolve; + let reject; + let 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(); + } + }); + + 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 oldCallAsync = Meteor.connection.callAsync; +Meteor.connection.callAsync = function () { + currentMethodInvocation = DDP._CurrentMethodInvocation.get(); + + return oldCallAsync.apply(this, arguments); +} + +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); + } + + return queueFunction((resolve, reject) => { + let finished = false; + Meteor._setImmediate(() => { + oldApplyAsync.apply(this, args).then((result) => { + finished = true; + resolve(result); + }, (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`); + } + }); + }); +}; + +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(); + }); +} + + // Proxy the public methods of Meteor.connection so they can // be called directly on Meteor. [ diff --git a/packages/ddp-client/package.js b/packages/ddp-client/package.js index b5706947a1..f081801d23 100644 --- a/packages/ddp-client/package.js +++ b/packages/ddp-client/package.js @@ -60,4 +60,6 @@ Package.onTest((api) => { api.addFiles('test/livedata_tests.js'); api.addFiles('test/livedata_test_service.js'); api.addFiles('test/random_stream_tests.js'); + api.addFiles('test/async_stubs/client.js', 'client'); + api.addFiles('test/async_stubs/server_setup.js', 'server'); }); diff --git a/packages/ddp-client/test/async_stubs/client.js b/packages/ddp-client/test/async_stubs/client.js new file mode 100644 index 0000000000..0cb627204d --- /dev/null +++ b/packages/ddp-client/test/async_stubs/client.js @@ -0,0 +1,225 @@ + +let events = []; +Meteor.methods({ + 'sync-stub'() { + events.push('sync-stub'); + return 'sync-stub-result' + }, + async 'async-stub'() { + events.push('start async-stub'); + await 0; + events.push('end async-stub'); + return 'async-stub-result' + }, + callAsyncFromSyncStub() { + events.push('callAsyncFromSyncStub'); + Meteor.callAsync('async-stub'); + }, + async callSyncStubFromAsyncStub() { + events.push('start callSyncStubFromAsyncStub'); + await 0 + let result = Meteor.call('sync-stub'); + events.push('end callSyncStubFromAsyncStub'); + return result; + }, + callSyncStubFromSyncStub() { + events.push('callSyncStubFromSyncStub'); + return Meteor.call('sync-stub'); + }, + callAsyncStubFromAsyncStub() { + events.push('callAsyncStubFromAsyncStub'); + return Meteor.callAsync('async-stub'); + } +}); + +Tinytest.addAsync('applyAsync - server only', async function (test) { + let serverResolver; + let serverPromise = new Promise((resolve) => { + serverResolver = resolve; + }); + + let stubResult = await Meteor.applyAsync('server-only-sync', [], { returnStubValue: true }, (err, result) => { + console.log(err); + if (!err) { + serverResolver(result); + } + }); + + let serverResult = await serverPromise; + + test.equal(stubResult, undefined); + test.equal(serverResult, 'sync-result'); +}); + +Tinytest.addAsync('applyAsync - sync stub', async function (test) { + let serverResolver; + let serverPromise = new Promise((resolve) => { + serverResolver = resolve; + }); + + let stubResult = await Meteor.applyAsync('sync-stub', [], { + returnStubValue: true + }, (err, result) => { + console.log(err); + if (!err) { + serverResolver(result); + } + }); + + let serverResult = await serverPromise; + + test.equal(stubResult, 'sync-stub-result'); + test.equal(serverResult, 'sync-server-result'); +}); + +Tinytest.addAsync('applyAsync - callAsync', async function (test) { + let serverResult = await Meteor.callAsync('async-stub'); + + test.equal(serverResult, 'async-server-result'); +}); + +Tinytest.addAsync('applyAsync - callAsync twice', async function (test) { + events = []; + let promise1 = Meteor.callAsync('async-stub'); + let promise2 = Meteor.callAsync('async-stub'); + + let results = await Promise.all([promise1, promise2]); + + test.equal(events, ['start async-stub', 'end async-stub', 'start async-stub', 'end async-stub']); + test.equal(results, ['async-server-result', 'async-server-result']); +}); + +// Broken in Meteor 2.13: https://github.com/meteor/meteor/issues/12889#issue-1998128607 +Tinytest.addAsync('applyAsync - callAsync from async stub', async function (test) { + await Meteor.callAsync('getAndResetEvents'); + events = []; + let serverResolver; + let serverPromise = new Promise((resolve) => { + serverResolver = resolve; + }); + let stubResult = await Meteor.applyAsync('callAsyncStubFromAsyncStub', [], { returnStubValue: true }, (err, result) => { + if (!err) { + serverResolver(result); + } + }); + let serverResult = await serverPromise; + + let serverEvents = await Meteor.callAsync('getAndResetEvents'); + + test.equal(stubResult, 'async-stub-result'); + test.equal(serverResult, 'server result'); + test.equal(events, ['callAsyncStubFromAsyncStub', 'start async-stub', 'end async-stub']); + test.equal(serverEvents, ['callAsyncStubFromAsyncStub']); +}); + + +Tinytest.addAsync('applyAsync - callAsync in then', async function (test) { + await Meteor.callAsync('getAndResetEvents'); + + events = []; + let result = await Meteor.callAsync('async-stub').then(() => Meteor.callAsync('async-stub')); + let serverEvents = await Meteor.callAsync('getAndResetEvents'); + + test.equal(events, ['start async-stub', 'end async-stub', 'start async-stub', 'end async-stub']); + test.equal(serverEvents, ['async-stub', 'async-stub']); + test.equal(result, 'async-server-result'); +}); + +Tinytest.addAsync('applyAsync - call from async stub', async function (test) { + await Meteor.callAsync('getAndResetEvents'); + events = []; + let serverResolver; + let serverPromise = new Promise((resolve) => { + serverResolver = resolve; + }); + let stubResult = await Meteor.applyAsync('callSyncStubFromAsyncStub', [], { returnStubValue: true }, (err, result) => { + if (!err) { + serverResolver(result); + } + }); + let serverResult = await serverPromise; + + let serverEvents = await Meteor.callAsync('getAndResetEvents'); + + test.equal(stubResult, 'sync-stub-result'); + test.equal(serverResult, 'server result'); + test.equal(events, ['start callSyncStubFromAsyncStub', 'sync-stub', 'end callSyncStubFromAsyncStub']); + test.equal(serverEvents, ['callSyncStubFromAsyncStub']); +}); + +Tinytest.addAsync('apply - call from sync stub', async function (test) { + await Meteor.callAsync('getAndResetEvents'); + events = []; + let serverResolver; + let serverPromise = new Promise((resolve) => { + serverResolver = resolve; + }); + let stubResult = Meteor.apply('callSyncStubFromSyncStub', [], { returnStubValue: true }, (err, result) => { + if (!err) { + serverResolver(result); + } + }); + let serverResult = await serverPromise; + + let serverEvents = await Meteor.callAsync('getAndResetEvents'); + + test.equal(stubResult, 'sync-stub-result'); + test.equal(serverResult, 'server result'); + test.equal(events, ['callSyncStubFromSyncStub', 'sync-stub']); + test.equal(serverEvents, ['callSyncStubFromSyncStub']); +}); + +Tinytest.addAsync('apply - proper order with applyAsync', async function (test) { + await Meteor.callAsync('getAndResetEvents'); + events = []; + let serverResolver; + let serverPromise = new Promise((resolve) => { + serverResolver = resolve; + }); + + let promise1 = Meteor.callAsync('callSyncStubFromAsyncStub'); + let stubResult = Meteor.apply('callSyncStubFromSyncStub', [], { returnStubValue: true }, (err, result) => { + if (!err) { + serverResolver(result); + } + }); + let promise2 = Meteor.callAsync('server-only-sync'); + let [ + serverResult, + result1, + result2 + ] = await Promise.all([serverPromise, promise1, promise2]); + + let serverEvents = await Meteor.callAsync('getAndResetEvents'); + + test.equal(stubResult, 'sync-stub-result'); + test.equal(serverResult, 'server result'); + test.equal(result1, 'server result'); + test.equal(result2, 'sync-result'); + test.equal(events, ['callSyncStubFromSyncStub', 'sync-stub', 'start callSyncStubFromAsyncStub', 'sync-stub', 'end callSyncStubFromAsyncStub']); + test.equal(serverEvents, ['callSyncStubFromAsyncStub', 'callSyncStubFromSyncStub', 'server-only-sync']); +}); + +Tinytest.addAsync('apply - wait', async function (test) { + await Meteor.callAsync('getAndResetEvents'); + events = []; + let serverResolver; + let serverPromise = new Promise((resolve) => { + serverResolver = resolve; + }); + + let stubResult = Meteor.apply( + 'callSyncStubFromSyncStub', + [], + { returnStubValue: true, wait: true }, + (err, result) => { + if (!err) { + serverResolver(result); + } + }); + + const serverResult = await serverPromise; + + test.equal(stubResult, 'sync-stub-result'); + test.equal(serverResult, 'server result'); +}); diff --git a/packages/ddp-client/test/async_stubs/server_setup.js b/packages/ddp-client/test/async_stubs/server_setup.js new file mode 100644 index 0000000000..bacef00bbd --- /dev/null +++ b/packages/ddp-client/test/async_stubs/server_setup.js @@ -0,0 +1,49 @@ +let events = []; + +Meteor.methods({ + getAndResetEvents() { + let oldEvents = events; + events = []; + + return oldEvents; + }, + 'server-only-sync' () { + events.push('server-only-sync'); + return 'sync-result'; + }, + async 'server-only-async' () { + events.push('server-only-async'); + await 0 + return 'server-only-async-result'; + }, + echo(message) { + events.push('echo'); + return message; + }, + + 'sync-stub' () { + events.push('sync-stub'); + return 'sync-server-result' + }, + 'async-stub' () { + events.push('async-stub'); + return 'async-server-result' + }, + 'callAsyncFromSyncStub'() { + events.push('callAsyncFromSyncStub'); + }, + 'callSyncStubFromAsyncStub'() { + events.push('callSyncStubFromAsyncStub'); + + return 'server result'; + }, + 'callSyncStubFromSyncStub'() { + events.push('callSyncStubFromSyncStub'); + return 'server result'; + }, + 'callAsyncStubFromAsyncStub'() { + events.push('callAsyncStubFromAsyncStub'); + + return 'server result'; + } +}); From bd3e9c753f70ca1e9a788ad3094f2d68778aa417 Mon Sep 17 00:00:00 2001 From: filipenevola Date: Sat, 25 Nov 2023 08:06:01 -0300 Subject: [PATCH 2/3] Removes duplicated echo method in tests of ddp-client Fixes 'Error: A method named 'echo' is already defined' on Travis --- packages/ddp-client/test/async_stubs/server_setup.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/ddp-client/test/async_stubs/server_setup.js b/packages/ddp-client/test/async_stubs/server_setup.js index bacef00bbd..065b725182 100644 --- a/packages/ddp-client/test/async_stubs/server_setup.js +++ b/packages/ddp-client/test/async_stubs/server_setup.js @@ -16,11 +16,6 @@ Meteor.methods({ await 0 return 'server-only-async-result'; }, - echo(message) { - events.push('echo'); - return message; - }, - 'sync-stub' () { events.push('sync-stub'); return 'sync-server-result' From 17d026f8fdad678d3ffb080f7d709b50084d8524 Mon Sep 17 00:00:00 2001 From: zodern Date: Wed, 29 Nov 2023 10:59:44 -0600 Subject: [PATCH 3/3] Queue subscriptions --- .../ddp-client/client/client_convenience.js | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/packages/ddp-client/client/client_convenience.js b/packages/ddp-client/client/client_convenience.js index c747ad9587..ba4f791241 100644 --- a/packages/ddp-client/client/client_convenience.js +++ b/packages/ddp-client/client/client_convenience.js @@ -214,6 +214,35 @@ function queueMethodInvoker(methodInvoker, wait) { }); } +/** + * 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(); + } + }); +}; // Proxy the public methods of Meteor.connection so they can // be called directly on Meteor.