Can some ELI5 why this is inserting my data twice?

help
1

I’m slowly learning the things I need to understand about Meteor 3, but in my template application I’m getting something I didn’t expect. Admittedly, I’m very new to the async/await stuff, so I’m guessing I’m doing something wrong here.

In the code below, I’m just setting a couple of flags for allowing registration for any user, or also system admin level users.

in the Meteor.method call I want to see if a document exists in the db before adding a new one, and if it does, then update the existing one instead. To do this in the past, I would just run a findOne and if found, then it would update instead of insert. I found that with the newer findOneAsync it was failing the test every time, and inserting new records even when one existed, so I figured it wasn’t waiting before moving on. Anyway, i have attempted to setup an async/await for that part, so it can check the status. It seems to work, but when I first insert because no data exists, it is adding a record twice.

'add.noSysAdminReg' (admReg, genReg) {
        check(admReg, Boolean);
        check(genReg, Boolean);

        if (!this.userId) {
            throw new Meteor.Error('Not able to change registration setting. Make sure you are logged in with valid system administrator credentials.');
        }

        console.log("Got here...");

        async function currConfig() {
            const curr = await SysConfig.findOneAsync({});
            if (typeof curr != 'undefined') {
                let configId = curr._id;
                Meteor.call('edit.noSysAdminReg', configId, admReg, genReg, function(err, result) {
                    if (err) {
                        console.log("    ERROR updating sys admin reg: " + err);
                    } else {
                        console.log("Success updating sys admin reg.");
                    }
                });
            } else {
                console.log("Adding new.");
                return SysConfig.insertAsync({
                    SysAdminReg: admReg,
                    dateAdded: new Date(),
                    allowReg: genReg,
                });
            }
        }
        currConfig();

What shows up in Mongo afterward:

meteor [direct: primary] meteor> db.sysConfig.find({});
[
  {
    _id: 'fGcJoB6CqQseufKXq',
    SysAdminReg: true,
    dateAdded: ISODate('2025-04-10T19:36:45.136Z'),
    allowReg: false
  },
  {
    _id: 'ucTXykh6YAYiGFwQq',
    SysAdminReg: true,
    dateAdded: ISODate('2025-04-10T19:36:45.139Z'),
    allowReg: false
  }
]
2

when you test the system, have you got a chance that it run into this case?

If your database was slow enough, and you call your method twice fast enough, you might see that duplication. You can set up unique index to prevent that happen.

3

First thing that jumped out at me is that this is a great use case for upsertAsync, second thing to confirm, what do the logs look like? I can see you’re already trying to bisect and find the problem but it’s unclear if both branches are being run somehow or how many times the method is run, from this example.

1 Like
4

Great follow up questions. This is set when a user toggles a checkbox in my settings UI on the front end. I’m currently only calling it once when toggled with the following in the template events section:

'change #allowGenReg, change #allowAdmReg' (evnnt) {
        let genReg = $("#allowGenReg").prop('checked');
        let admReg = $("#allowAdmReg").prop('checked');
        console.log("General Reg set to: " + genReg);

        const setNoSysAdminReg = async() => {
            try {
                const result = Meteor.callAsync("add.noSysAdminReg", admReg, genReg);
                if (result) {
                    console.log("Successfully added reg settings.");
                    showSnackbar("Updated Registration Settings.", "green");
                }
            } catch(error) {
                console.log("    ERROR setting registration: " + error);
            }
        }

        setNoSysAdminReg();

In my logging I only see the one branch being taken. I have logging in the other method as well, and it does not get triggered (or doesn’t seem to be getting triggered, and the “Got Here…” log is before the branch is taken, and you can ssee that it’s only showing up once. I thought maybe the await doesn’t stop the code from proceeding, so it has curr = 'undefined', and goes through, then the promise is returned after that and runs through that code again, but perhaps not.

1 Like
5

You should await that Meteor.callAsync call. result is a promise and will always be truthy in that function.

2 Likes
6

To be clear you’re saying it should be like this:

const setNoSysAdminReg = async() => {
            try {
                const result = await Meteor.callAsync("add.noSysAdminReg", admReg, genReg);
                if (result) {
                    console.log("Successfully added reg settings.");
                    showSnackbar("Updated Registration Settings.", "green");
                }
            } catch(error) {
                console.log("    ERROR setting registration: " + error);
            }
        }

        setNoSysAdminReg();
7

I still get two entries each time, but I think I’ll add the upsert instead of the split logic and see if that resolves it.

Method now looks like:

'add.noSysAdminReg' (admReg, genReg) {
        check(admReg, Boolean);
        check(genReg, Boolean);

        if (!this.userId) {
            throw new Meteor.Error('Not able to change registration setting. Make sure you are logged in with valid system administrator credentials.');
        }

        console.log("Got here...");
        console.log("Adding new.");
        return SysConfig.upsertAsync({ ruleNo: 1 }, {
            $set: {
                ruleNo: 1, // <-- added so I have a unique id to call without having to query for an id first.
                SysAdminReg: admReg,
                dateAdded: new Date(),
                allowReg: genReg,
                allowUpdates: true, // <-- added as I use it later anyway.
            }
        });
    },
1 Like
8

Hopefully that would resolve it but certainly it’s sort of plastering over another possible issue which would be good to isolate in case it rears its head again, eg a situation where each request does a fresh insert and there’s no use case for upserts.

Another defensive technique in that case would be to let the client pick a UUID to identify its request by, and if gets hit multiple times then you reject the subsequent calls. But it’s a bit messy and sorta old-school and with the normal Meteor approach of in-order method handling, you normally don’t need that.

9

D’oh, reviewing the original code I think I saw another issue, your method definition itself is (apparently) not async, but your inner function is async, and your DB call inside of that is awaited.

Then you call the inner function, but you don’t await it. So it will do stuff in the background essentially while the method ends before it’s complete. But worse, if it errors, you might bring down the server due to Node’s behaviour to crash on unhandled rejected promises (eg, if a promise goes out of memory scope, and nothing ever awaited it or you never used then/catch methods).

So you should have probably done:

-'add.noSysAdminReg' (admReg, genReg) {
+async 'add.noSysAdminReg' (admReg, genReg) {
        check(admReg, Boolean);
        check(genReg, Boolean);

        if (!this.userId) {
            throw new Meteor.Error('Not able to change registration setting. Make sure you are logged in with valid system administrator credentials.');
        }

        console.log("Got here...");

        async function currConfig() {
            const curr = await SysConfig.findOneAsync({});
            if (typeof curr != 'undefined') {
                let configId = curr._id;
                Meteor.call('edit.noSysAdminReg', configId, admReg, genReg, function(err, result) {
                    if (err) {
                        console.log("    ERROR updating sys admin reg: " + err);
                    } else {
                        console.log("Success updating sys admin reg.");
                    }
                });
            } else {
                console.log("Adding new.");
                return SysConfig.insertAsync({
                    SysAdminReg: admReg,
                    dateAdded: new Date(),
                    allowReg: genReg,
                });
            }
        }
-         currConfig();
+        await currConfig();
},

In your new definition, you don’t need that because you return the promise so it’s going to be handled by some other location in your code.

But if you did want to do something after the upsert, you would need to await it and in turn make the method async.