-
-
Notifications
You must be signed in to change notification settings - Fork 483
fix: Prevent crashes from View.refresh
#3059
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Thanks for opening this pull request! This pull request can be checked-out with: git fetch origin pull/3059/head:pr-3059
git checkout pr-3059This pull request can be installed with: pip install git+https://github.com/Pycord-Development/pycord@refs/pull/3059/head |
Paillat-dev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I am nitpicking, pr lgtm di per se)
| try: | ||
| view._refresh(components) | ||
| except: | ||
| _log.warning( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure here whether we should prefer _log.warning or _log.exception - with the reasoning behind this being that this potentially removes from existence useful traceback info in case someone encounters this when they should not.
Alternatively we maybe could add a _log.debug with exc_info=True as well or something ? Idk just an idea.
| ) | ||
| if view and self.message: | ||
| self._state.prevent_view_updates_for(self.message.id) | ||
| _message = self.message or self._original_response |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe call it something different, like target, would be less confusing.
|
Oh and changelog |
Summary
Pycord currently has an issue with race conditions in views - sometimes when editing a message or interaction response, the gateway will receive said message's
MESSAGE_UPDATEfrom Discord and attempt to internallyrefreshthe view beforeViewStorehas updated.This is meant to be circumvented through
remove_message_tracking, but this is not possible in response edit scenarios where the library may not have knowledge of the relevantmessage_idbeforehand.This PR:
_original_responsethat's now returned from all interactionslogging.warningI'm not certain if the first change is entirely appropriate - if this happens to target the wrong message, I will instead revert that specfic change.
Information
examples, ...).
Checklist
type: ignorecomments were used, a comment is also left explaining why.