Habitica: code change needed for "Quest completed but rewards and completion not recorded for quest participant(s)"

[Alys]

As described in https://github.com/HabitRPG/habitrpg/issues/4252, when quests finish, they sometimes do not give the rewards and achievement to all participants. I don't believe there's anything buggy with the code; I think it's just timeouts or similar, so we need code that works in a way that can't be broken by slow / overloaded web or database servers.

I proposed an idea in this comment: https://github.com/HabitRPG/habitrpg/issues/4252#issuecomment-226402321 (copied below), and there were no complaints or other suggestions when I posted about it again a few days ago (https://github.com/HabitRPG/habitrpg/issues/4252#issuecomment-246515501), so I'm opening this issue for that fix to be done.

But if anyone does have better ideas, definitely tell us! Posting to https://github.com/HabitRPG/habitrpg/issues/4252 would be better than commenting here (I'll close this issue if something better is proposed there).

---

When the quest ends, we collate the participant's IDs and the rewards they are due, and store all that in a questRewardsPending object in the party's groups document - write all that in a single update. If the update fails, retry until it succeeds. The update would need to NOT overwrite any data from previous quests, so have the object keyed with timestamps.

Then we have a separate process that reads that object and handles the users one by one. When it has successfully updated a user, it deletes that user's data from the questRewardsPending object (and if the update fails, it tries again until it succeeds). That way the worst bug we can get is that a user doesn't get the update immediately, or they get the update twice if the update does succeed but the deletion from questRewardsPending fails, causing the update to later be applied again - we can put up with that bug!

That separate process could be run after the quest had ended. To cover the cases where it timed out, the same process (or a similar per-user process) could also be run at other times - e.g., when a user loaded the party page or cronned, the process could look in the user's party's questRewardsPending object and apply anything it finds there for the user.

There'd be edge cases where the end-of-quest process had timed out for a user and the user left the party before any future processing gave them the rewards, but at least there'd be a record in the database of what they were owed and it would be fairly easy for an admin to find it and apply it. Also, the code that's run when a user leaves the party could look for any pending rewards they were owned and apply them.

[Hus274]

It looks like this is still happening to people according to the issue linked above so I can take a look into this one.

[Alys]

@Hus274 Thank you!!!!!!

[Hus274]

I tried to really think through all the possibilities of how to solve this problem and ended up doing something different than the above suggestion. The solution that I built individually saves all the users in a party and then retries any of those that fail in the same method for up to thirty seconds (this can be adjusted).

The reason I did this solution was because it seemed less invasive than the above and as this quest is already called outside of user processes, it shouldn't have a significant latency concern. This also should prevent the possibility of giving a user double the rewards.

I'll upload the PR here to give you all the chance to look through it. If we decide that we don't want to go this direction, I'd be happy to give the above suggested solution a shot! Let me know if you have any thoughts or concerns!

[Alys]

@Hus274 That sounds good! I'm happy for us to give it a try. Thank you!