percolatestudio / meteor-synced-cron

A simple cron system for Meteor. It supports syncronizing jobs between multiple processes.
https://atmospherejs.com/littledata/synced-cron
MIT License
499 stars 110 forks source link

Unhandled duplicate key errors with Meteor 1.7 #134

Closed oneeman closed 6 years ago

oneeman commented 6 years ago

This has already been filed as https://github.com/percolatestudio/meteor-synced-cron/issues/133, but I'm filing a new issue since that one was closed and the problem hasn't been fixed yet.

The issue is due to Meteor 1.7 having switched from v2 of the node mongo driver to v3. One of the breaking changes is how errors in bulk writes are surfaced. See "BulkWriteResult & BulkWriteError" at https://github.com/mongodb/node-mongodb-native/blob/master/CHANGES_3.0.0.md . As a result, when an .insert() call results in a duplicate key error, the error's name has changed from MongoError to BulkWriteError, and so the handler at https://github.com/percolatestudio/meteor-synced-cron/blob/a82a3569322c0e9cb878de7c3baab94648e0260d/synced-cron-server.js#L215 no longer recognizes it correctly.

A fix which will work with both versions of the mongo driver is to check just the error code, not the error name. I've done this on my project and confirmed that it fixes the issue. I will open up a pull request, though I don't know how actively this repo is being maintained.

For those who need to fix this issue without waiting for a new release, see https://forums.meteor.com/t/how-to-replace-an-atmosphere-package-with-own-local-copy/25922 for how to use a local version of the package instead.

TheGame2500 commented 6 years ago

I can merge it in, but I can’t publish it on atmosphere though. I could probably start another package.

On Tue, 10 Jul 2018 at 18:56, Or Neeman notifications@github.com wrote:

This has already been filed as #133 https://github.com/percolatestudio/meteor-synced-cron/issues/133, but I'm filing a new issue since that one was closed and the problem hasn't been fixed yet.

The issue is due to Meteor 1.7 having switched from v2 of the node mongo driver to v3. One of the breaking changes is how errors in bulk writes are surfaced. See "BulkWriteResult & BulkWriteError" at https://github.com/mongodb/node-mongodb-native/blob/master/CHANGES_3.0.0.md . As a result, when an .insert() call results in a duplicate key error, the error's name has changed from MongoError to BulkWriteError, and so the handler at https://github.com/percolatestudio/meteor-synced-cron/blob/a82a3569322c0e9cb878de7c3baab94648e0260d/synced-cron-server.js#L215 no longer recognizes it correctly.

A fix which will work with both versions of the mongo driver is to check just the error code, not the error name. I've done this on my project and confirmed that it fixes the issue. I will open up a pull request, though I don't know how actively this repo is being maintained.

For those who need to fix this issue without waiting for a new release, see https://forums.meteor.com/t/how-to-replace-an-atmosphere-package-with-own-local-copy/25922 for how to use a local version of the package instead.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/percolatestudio/meteor-synced-cron/issues/134, or mute the thread https://github.com/notifications/unsubscribe-auth/AMVCo1VJDU_-FMCe_35dYgMUTH4it8RUks5uFM6tgaJpZM4VJrjb .

etyp commented 6 years ago

@TheGame2500 do you know who has atmosphere access?

acomito commented 6 years ago

damnnn I need this

TheGame2500 commented 6 years ago

@acomito @etyp @oneeman fixed by migrating to https://atmospherejs.com/littledata/synced-cron