fix(modal): fix timeout edge cases with `custom_id` reuse and long-running callbacks
Summary
This PR fixes two edge cases with ui.Modal timeouts (send_modal(title, id, components) isn't affected, to be clear).
1.
Since modals are keyed by (user_id, custom_id), sending two modals with the same custom_id to the same user (e.g. when they run the command on two devices, or close + rerun) could result in a situation where the timeout of the first modal removes the second modal from the store.
example
Let's assume a command /test exists, which responds with a modal with a 30 second timeout, and a fixed custom_id:
- t=0: user runs
/teston device A, and does not submit the modal- modal instance A gets added to store
- 30s timeout starts
- t=25: user runs
/teston device B- new modal instance B gets added to store, overwriting the first one
- 30s timeout starts
- timeout task of the first modal keeps running
- t=30: timeout of modal A removes modal B from store (same key)
- t=40: user submits modal B, nothing happens as modal was removed from the store
- user is confused as to why nothing happened, since the modal should've only expired at t=55
This PR fixes that by canceling the timeout of the first modal before adding the second one to the store.
Ideally, custom IDs should always be unique when used with ui.Modal, but the examples were doing exactly the opposite. I've updated those and added a documentation note.
2.
Previously, the modal timeout would (indirectly) be stopped after the callback ran.
If there's a long-running callback however, the timeout could fire while the callback was still running, which is both incorrect and resulted in an exception when the dispatch function would try to remove the modal after the callback did finish.
example
Again, let's assume there's a modal with a 30 second timeout:
- t=0: modal gets sent to user, 30s timeout starts
- t=15: user submits modal; the bot responds, starts some long-running task in the callback, and waits for it
- t=30: since the modal is still in the store, the timeout handler removes it and calls
on_timeout - t=100: the long-running task finishes, the callback returns, and the
remove_modalcall fails
This PR fixes that by cancelling the timeout before executing the callback (and later possibly reinstating it if the modal interaction wasn't responded to).
I believe this is also what could've happened here, but it's difficult to say for sure.
Checklist
- [x] If code changes were made, then they have been tested
- [x] I have updated the documentation to reflect the changes
- [x] I have formatted the code properly by running
task lint - [x] I have type-checked the code by running
task pyright
- [x] This PR fixes an issue
- [ ] This PR adds something new (e.g. new method or parameters)
- [ ] This PR is a breaking change (e.g. methods or parameters removed/renamed)
- [ ] This PR is not a code change (e.g. documentation, README, ...)