SinisterRectus
Member data persistence
Commit 4eefacad005903f6768654f0fbd53da9a7535e0f introduced a form of weak caching into regular caches, where deleted objects would be moved from a strong table to a weak table. The goal here was to persist objects during the course of their deletion, to prevent inconsistencies between HTTP deletions and gateway deletions. See #141 and #142.
Commit 89e71c2bd3f5f2ffb8775b72ec7835391aa1a018 addressed an issue where these weakly cached objects were sometimes not reused. See #226. The issue described below is similar in that it is related to objects that are recreated after they are deleted, except now the problem exists for objects that are reused.
When a user leaves a guild, Discord dispatches the GUILD_MEMBER_REMOVE event with only a guild_id and user object. If Discordia parses a cached member object for this event, the object's regular member data remains untouched. The member object is then emitted with this old data. Generally, having this data is useful. For example, bots may archive this data when the user leaves, and may reapply settings such as roles or nicknames if the user rejoins.
When a user joins a guild, Discord dispatches the GUILD_MEMBER_ADD event. Due to the weak caching described above, if this user had previously been a member of the guild and if Discordia has its old member object still cached, then the member object is reused. Because the member data was never invalidated, the emitted member object, which appears to be new, would incorrectly carry the old data. This not only makes the member data inaccurate, but may affect internal operations that rely on member state. For example, Member:addRole would incorrectly not attempt to apply a role for a member who appears to have the role but actually does not have it.
There are a few different ways to approach this issue.
The naive solution would be to invalidate member data on GUILD_MEMBER_REMOVE. There is technically no guarantee that a member has any roles or nickname, however, removing this data before emitting the memberLeave event would be disingenuous to bots that have a reasonable expectation for its existence, especially when cacheAllMembers is enabled.
To address the issue of Member:addRole and Member:removeRole not working, the internal Member:hasRole check could be removed. This check is more of a courtesy to prevent extraneous HTTP requests, but is not necessary for normal operation, and generally is not used in other places where a bot may attempt to idempotently PATCH data.
A special case could be made in caching objects that can be recreated after they are deleted. We now know that guilds and members are affected. Other objects such as channels, overwrites, reactions, and bans may also be affected. Some objects such as messages, roles, and emojis are guaranteed to never be recreated and should be unaffected.
Upon further inspection, I am now noticing that if an object is reused from a cache's weak table, it is incorrectly not updated with new data. This could be fixed relatively easily, but this would not address any issues in updating certain undefined properties. For example, incoming data with an undefined nick would not overwrite any cached nick string. I think that object creations need to be better distinguished from object updates, alluded to with the "special case" described above.
I think this issue will be resolved, but maybe not in the best way. Due to the behavior of privileged intents and Discord's discouragement, members and presences are currently not cached in 3.0. If I do add any caching for these objects, it would be weak caching for internal use only. Thus, member objects will likely NOT be available on the memberRemove event and it will otherwise be impossible to fetch old data for a member that does not exist. If you want to store any information about members, you will have to do it before they leave the guild. Options are the memberCreate or memberUpdate events; getMember, getMembers, searchMembers HTTP requests; or the requestMembers gateway request.