diff --git a/test/api/v3/unit/libs/cron.test.js b/test/api/v3/unit/libs/cron.test.js index ef09868c27..dc76bb3666 100644 --- a/test/api/v3/unit/libs/cron.test.js +++ b/test/api/v3/unit/libs/cron.test.js @@ -603,25 +603,25 @@ describe('recoverCron', () => { } }); - it('increases status.times count and reruns up to 3 times', async (done) => { + it('increases status.times count and reruns up to 4 times', async (done) => { execStub.returns(Bluebird.resolve({_cronSignature: 'RUNNING_CRON'})); - execStub.onCall(3).returns(Bluebird.resolve({_cronSignature: 'NOT_RUNNING'})); + execStub.onCall(4).returns(Bluebird.resolve({_cronSignature: 'NOT_RUNNING'})); await recoverCron(status, locals); - expect(status.times).to.eql(3); + expect(status.times).to.eql(4); expect(locals.user).to.eql({_cronSignature: 'NOT_RUNNING'}); done(); }); - it('throws an error if recoverCron runs 4 times', async (done) => { + it('throws an error if recoverCron runs 5 times', async (done) => { execStub.returns(Bluebird.resolve({_cronSignature: 'RUNNING_CRON'})); try { await recoverCron(status, locals); } catch (err) { - expect(status.times).to.eql(4); + expect(status.times).to.eql(5); expect(err.message).to.eql(`Impossible to recover from cron for user ${locals.user._id}.`); done(); diff --git a/website/server/libs/api-v3/cron.js b/website/server/libs/api-v3/cron.js index 3f5c416ed9..6b46a7a435 100644 --- a/website/server/libs/api-v3/cron.js +++ b/website/server/libs/api-v3/cron.js @@ -12,6 +12,29 @@ const shouldDo = common.shouldDo; const scoreTask = common.ops.scoreTask; // const maxPMs = 200; +export async function recoverCron (status, locals) { + let {user} = locals; + + await Bluebird.delay(300); + + let reloadedUser = await User.findOne({_id: user._id}).exec(); + + if (!reloadedUser) { + throw new Error(`User ${user._id} not found while recovering.`); + } else if (reloadedUser._cronSignature !== 'NOT_RUNNING') { + status.times++; + + if (status.times < 5) { + await recoverCron(status, locals); + } else { + throw new Error(`Impossible to recover from cron for user ${user._id}.`); + } + } else { + locals.user = reloadedUser; + return null; + } +} + let CLEAR_BUFFS = { str: 0, int: 0, @@ -83,29 +106,6 @@ function performSleepTasks (user, tasksByType, now) { }); } -export async function recoverCron (status, locals) { - let {user} = locals; - - await Bluebird.delay(300); - - let reloadedUser = await User.findOne({_id: user._id}).exec(); - - if (!reloadedUser) { - throw new Error(`User ${user._id} not found while recovering.`); - } else if (reloadedUser._cronSignature !== 'NOT_RUNNING') { - status.times++; - - if (status.times < 4) { - await recoverCron(status, locals); - } else { - throw new Error(`Impossible to recover from cron for user ${user._id}.`); - } - } else { - locals.user = reloadedUser; - return null; - } -} - // Perform various beginning-of-day reset actions. export function cron (options = {}) { let {user, tasksByType, analytics, now = new Date(), daysMissed, timezoneOffsetFromUserPrefs} = options; diff --git a/website/server/middlewares/api-v3/cron.js b/website/server/middlewares/api-v3/cron.js index b6555cb682..18d9907d0d 100644 --- a/website/server/middlewares/api-v3/cron.js +++ b/website/server/middlewares/api-v3/cron.js @@ -103,13 +103,15 @@ async function cronAsync (req, res) { let _cronSignature = uuid(); - // To avoid double cron we first set _cronSignature to now and then check that it's not changed while processing + // To avoid double cron we first set _cronSignature and then check that it's not changed while processing let userUpdateResult = await User.update({ _id: user._id, _cronSignature: 'NOT_RUNNING', // Check that in the meantime another cron has not started }, { $set: { _cronSignature, + lastCron: now, // setting lastCron now so we don't risk re-running parts of cron if it fails + 'auth.timestamps.loggedin': now, }, }).exec(); @@ -170,8 +172,6 @@ async function cronAsync (req, res) { }, { $set: { _cronSignature: 'NOT_RUNNING', - lastCron: now, - 'auth.timestamps.loggedin': now, }, }).exec(); @@ -182,27 +182,23 @@ async function cronAsync (req, res) { // If cron was aborted for a race condition try to recover from it if (err.message === 'CRON_ALREADY_RUNNING') { // Recovering after abort, wait 300ms and reload user - // do it for max 4 times then reset _cronSignature so that it doesn't prevent cron from running + // do it for max 5 times then reset _cronSignature so that it doesn't prevent cron from running // at the next request let recoveryStatus = { times: 0, }; - recoverCron(recoveryStatus, res.locals); + await recoverCron(recoveryStatus, res.locals); } else { // For any other error make sure to reset _cronSignature so that it doesn't prevent cron from running // at the next request - try { - await User.update({ - _id: user._id, - }, { - _cronSignature: 'NOT_RUNNING', - }).exec(); + await User.update({ + _id: user._id, + }, { + _cronSignature: 'NOT_RUNNING', + }).exec(); - throw err; // re-throw original error - } catch (secondErr) { - throw secondErr; - } + throw err; // re-throw the original error } } } @@ -212,7 +208,5 @@ module.exports = function cronMiddleware (req, res, next) { .then(() => { next(); }) - .catch(err => { - next(err); - }); + .catch(next); }; diff --git a/website/server/models/group.js b/website/server/models/group.js index 46a5a4f449..cff731c034 100644 --- a/website/server/models/group.js +++ b/website/server/models/group.js @@ -279,6 +279,7 @@ export function chatDefaults (msg, user) { } const NO_CHAT_NOTIFICATIONS = [TAVERN_ID]; + schema.methods.sendChat = function sendChat (message, user) { this.chat.unshift(chatDefaults(message, user)); this.chat.splice(200); @@ -425,9 +426,9 @@ schema.statics.cleanGroupQuest = function cleanGroupQuest () { }; }; -// Participants: Grant rewards & achievements, finish quest -// Returns the promise from update().exec() -schema.methods.finishQuest = function finishQuest (quest) { +// Participants: Grant rewards & achievements, finish quest. +// Changes the group object update members +schema.methods.finishQuest = async function finishQuest (quest) { let questK = quest.key; let updates = {$inc: {}, $set: {}}; @@ -471,14 +472,14 @@ schema.methods.finishQuest = function finishQuest (quest) { let q = this._id === TAVERN_ID ? {} : {_id: {$in: _.keys(this.quest.members)}}; this.quest = {}; this.markModified('quest'); - return User.update(q, updates, {multi: true}).exec(); + + return await User.update(q, updates, {multi: true}).exec(); }; function _isOnQuest (user, progress, group) { return group && progress && group.quest && group.quest.active && group.quest.members[user._id] === true; } -// Returns a promise schema.statics.collectQuest = async function collectQuest (user, progress) { let group = await this.getGroup({user, groupId: 'party'}); if (!_isOnQuest(user, progress, group)) return; @@ -504,6 +505,7 @@ schema.statics.collectQuest = async function collectQuest (user, progress) { await group.finishQuest(quest); group.sendChat('`All items found! Party has received their rewards.`'); + return await group.save(); };