Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using new meteor mongo async api #395

Closed
ToyboxZach opened this issue Aug 3, 2023 · 9 comments · Fixed by #405
Closed

Using new meteor mongo async api #395

ToyboxZach opened this issue Aug 3, 2023 · 9 comments · Fixed by #405

Comments

@ToyboxZach
Copy link

So with meteor 3.0 moving to the new async api I've noticed that this package is not quite ready for it.

If you run this with the env variable
WARN_WHEN_USING_OLD_API=1

You end up with errors like this:


Calling method users.findOne from old API on server.
W20230803-14:05:48.617(-7)? (STDERR)    This method will be removed, from the server, in version 3.
W20230803-14:05:48.617(-7)? (STDERR)    Trace is below:
W20230803-14:05:48.617(-7)? (STDERR) [object Object]
W20230803-14:05:48.617(-7)? (STDERR)     at warnUsingOldApi (packages/mongo/collection.js:24:12)
W20230803-14:05:48.617(-7)? (STDERR)     at hook.findOne (packages/mongo/collection.js:452:5)
W20230803-14:05:48.618(-7)? (STDERR)     at RedisSubscriptionManager.getDoc (packages/cultofcoders:redis-oplog/lib/redis/RedisSubscriptionManager.js:208:30)
W20230803-14:05:48.618(-7)? (STDERR)     at RedisSubscriptionManager.process (packages/cultofcoders:redis-oplog/lib/redis/RedisSubscriptionManager.js:147:28)
W20230803-14:05:48.618(-7)? (STDERR)     at packages/cultofcoders:redis-oplog/lib/mongo/lib/dispatchers.js:20:42
W20230803-14:05:48.618(-7)? (STDERR)     at Array.forEach (<anonymous>)
W20230803-14:05:48.618(-7)? (STDERR)     at packages/cultofcoders:redis-oplog/lib/mongo/lib/dispatchers.js:14:20
W20230803-14:05:48.618(-7)? (STDERR)     at packages/meteor.js:365:18
W20230803-14:05:48.619(-7)? (STDERR)     at Meteor.EnvironmentVariable.EVp.withValue (packages/meteor.js:1389:31)
W20230803-14:05:48.619(-7)? (STDERR)     at dispatchEvents (packages/cultofcoders:redis-oplog/lib/mongo/lib/dispatchers.js:13:30)
W20230803-14:05:48.619(-7)? (STDERR)     at dispatchUpdate (packages/cultofcoders:redis-oplog/lib/mongo/lib/dispatchers.js:63:5)
W20230803-14:05:48.619(-7)? (STDERR)     at hook.update (packages/cultofcoders:redis-oplog/lib/mongo/Mutator.js:200:13)
W20230803-14:05:48.619(-7)? (STDERR)     at hook.update (packages/cultofcoders:redis-oplog/lib/mongo/extendMongoCollection.js:40:35)
W20230803-14:05:48.619(-7)? (STDERR)     at hook.Mongo.Collection.<computed> [as updateAsync] (packages/mongo/collection.js:1004:46)

The root problem looks to be this call:

getDoc(collection, subscribers, data) {
        const event = data[RedisPipe.EVENT];
        let doc = data[RedisPipe.DOC];

        if (collection._redisOplog && !collection._redisOplog.protectAgainstRaceConditions) {
            // If there's no protection against race conditions
            // It means we have received the full doc in doc

            return doc;
        }

        const fieldsOfInterest = getFieldsOfInterestFromAll(subscribers);

        if (fieldsOfInterest === true) {
            doc = collection.findOne(doc._id);
        } else {
            doc = collection.findOne(doc._id, { fields: fieldsOfInterest });
        }

        return doc;
    }

Are there plans to async-ify this package or is there an alternative solution?

@github-actions
Copy link

github-actions bot commented Aug 3, 2023

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

@matheusccastroo
Copy link
Contributor

I started working on this, here's the PR link: #405

@ToyboxZach
Copy link
Author

Ok great, I have pulled down this change locally and it has generally been working.

Meteor-Community-Packages#13

I'm curious if there we ill be any drastic differences

@matheusccastroo
Copy link
Contributor

@ToyboxZach Actually I wasn't even aware this package had another repository. There's nothing telling this on the README.md hehe, so in the end this is just duplicated work 🙃

@ToyboxZach
Copy link
Author

Yep its a bit of a mess, I only found it by looking around this tab https://github.com/cult-of-coders/redis-oplog/network

@matheusccastroo
Copy link
Contributor

Yeah, it's a shame. Either way, that PR seems to be outdated from the beta version and needs a rebase/merge, so since I'm already finishing mine, I will keep at it. Hopefully we will get one of the two merged soon!

@matheusccastroo
Copy link
Contributor

@StorytellerCZ Which repository is the right one here for the redis oplog?

@matheusccastroo
Copy link
Contributor

@ToyboxZach @nachocodoner @denihs @StorytellerCZ also, please do a review if possible 😄

@matheusccastroo matheusccastroo mentioned this issue May 6, 2024
2 tasks
@ToyboxZach
Copy link
Author

It's really hard to review it with the prettier changes mixed in.

I was strongly suggest doing a quick change with just the prettier fixes in it, and then doing a PR after that.

It would make it easier for someone to review it (I'm not the person who should review it likely, but feel this could help whoever else will review it)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants