meteor-collection2 icon indicating copy to clipboard operation
meteor-collection2 copied to clipboard

Meteor 3.0 potential bug

Open harryadel opened this issue 1 year ago • 7 comments

As reported in Slack by @StorytellerCZ :


W20240122-18:59:31.199(1)? (STDERR) meteor://💻app/packages/mongo.js:3584
W20240122-18:59:31.200(1)? (STDERR)             throw new Error("".concat(m, " +  is not available on the server. Please use ").concat(getAsyncMethodName(m), "() instead."));
W20240122-18:59:31.200(1)? (STDERR)                   ^
W20240122-18:59:31.200(1)? (STDERR) 
W20240122-18:59:31.200(1)? (STDERR) Error: update +  is not available on the server. Please use updateAsync() instead.
W20240122-18:59:31.200(1)? (STDERR)     at Object.ret.<computed> [as update] (packages/mongo/remote_collection_driver.js:52:15)
W20240122-18:59:31.200(1)? (STDERR)     at Collection.update (packages/mongo/collection.js:959:31)
W20240122-18:59:31.200(1)? (STDERR)     at Collection.Mongo.Collection.<computed> [as update] (packages/aldeed:collection2/main.js:236:23)
W20240122-18:59:31.200(1)? (STDERR)     at packages/accounts-oauth/oauth_server.js:94:41
W20240122-18:59:31.200(1)? (STDERR)     at AsynchronousCursor.forEach (packages/mongo/mongo_driver.js:1115:22)
W20240122-18:59:31.200(1)? (STDERR)     at processTicksAndRejections (node:internal/process/task_queues:95:5)
W20240122-18:59:31.200(1)? (STDERR) 
W20240122-18:59:31.200(1)? (STDERR) Node.js v20.9.0

If I add

if (Meteor.isServer && Meteor.isFibersDisabled) {
          return null
        }

to line 236 in main.js it goes away. Probably not the best approach though.

harryadel avatar Jan 23 '24 12:01 harryadel

Thank you for submitting this issue!

We, the Members of Meteor Community Packages take every issue seriously. Our goal is to provide long-term lifecycles for packages and keep up with the newest changes in Meteor and the overall NodeJs/JavaScript ecosystem.

However, we contribute to these packages mostly in our free time. Therefore, we can't guarantee you issues to be solved within certain time.

If you think this issue is trivial to solve, don't hesitate to submit a pull request, too! We will accompany you in the process with reviews and hints on how to get development set up.

Please also consider sponsoring the maintainers of the package. If you don't know who is currently maintaining this package, just leave a comment and we'll let you know

github-actions[bot] avatar Jan 23 '24 12:01 github-actions[bot]

Here's the relevant code associated with your problem:

// If async is supported and fibers are disabled
// wait for db operation promise to resolve 
// else just run the sync version as is
if (async && !Meteor.isFibersDisabled) {
        try {
          this[methodName.replace('Async', '')].isCalledFromAsync = true;
          _super.isCalledFromAsync = true;
          return Promise.resolve(_super.apply(this, args)); <---  wait for db operation here
        } catch (err) {
          const addValidationErrorsPropName =
            typeof validationContext.addValidationErrors === 'function'
              ? 'addValidationErrors'
              : 'addInvalidKeys';
          parsingServerError([err], validationContext, addValidationErrorsPropName);
          error = getErrorObject(validationContext, err.message, err.code);
          return Promise.reject(error);
        }
      } else {
        return _super.apply(this, args); <--- run sync version as is
      }

With that being said I don't see an problem inherently with an error being thrown out. You're not meant to be using sync DB calls on Meteor 3.0, duh! Moreover, it's in your best interest for an error to be thrown out so you can tell what's DB call failed instead of failing silently.

The best we can strive for is to beautify the error but IMO the error is informant as is, it doesn't get better than this.

Error: update +  is not available on the server. Please use updateAsync() instead.

Let me know what you think, otherwise I'm closing this issue.

harryadel avatar Jan 23 '24 12:01 harryadel

The problem though is that we weren't using any sync call on the server or client. Unless there was something in a package that we couldn't locate.

StorytellerCZ avatar Jan 24 '24 12:01 StorytellerCZ

hmmm, can you provide a replication of this bug? I was going simply by the error message you provided.

harryadel avatar Jan 24 '24 12:01 harryadel

I'm now thinking this could be related to this: https://github.com/meteor/meteor/pull/12978

While the current solution is technically correct, I don't think that it is a best behavior for collection2. I'll look more into this as I work on reproduction.

StorytellerCZ avatar Jan 26 '24 08:01 StorytellerCZ

Ok, keep me posted.

harryadel avatar Jan 27 '24 14:01 harryadel

@harryadel is this something that needs to be fixed for 3.0 or is this an edge case?

jankapunkt avatar Jul 12 '24 12:07 jankapunkt