Skip to content

fix: handle Forbidden when sending confirm-thread-creation DM (#3442)#3447

Merged
StephenDaDev merged 5 commits into
modmail-dev:developmentfrom
13-Prabhat:fix/3442-confirm-thread-dm-forbidden
Jul 14, 2026
Merged

fix: handle Forbidden when sending confirm-thread-creation DM (#3442)#3447
StephenDaDev merged 5 commits into
modmail-dev:developmentfrom
13-Prabhat:fix/3442-confirm-thread-dm-forbidden

Conversation

@13-Prabhat

Copy link
Copy Markdown

Closes #3442

Bug

When both react to contact and confirm_thread_creation are enabled, and the recipient has DMs disabled, the confirmation prompt sent from ThreadManager.create() can raise an uncaught discord.Forbidden exception.

Because the exception occurs before view.wait() is reached, the thread remains cached in a "not ready" state and never gets cleaned up. As a result, subsequent contact attempts may report that a thread already exists, and the stale state persists until the bot is restarted.

Fix

Added a try/except discord.Forbidden block around the confirmation prompt send() call.

If the DM cannot be delivered, the code now:

  • Logs a warning
  • Marks the thread as cancelled
  • Removes the thread from the cache
  • Returns early

This mirrors the existing cleanup logic already used for the deny and timeout paths below.

Testing

  • Traced the affected code path against the behavior described in the issue
  • Confirmed that failed DM delivery now triggers proper cleanup
  • Verified that no behavior changes occur when recipients have DMs enabled
  • Ran python -m py_compile
  • Applied project-standard formatting with black

The repository does not currently include an automated test suite, so validation was performed through code review and manual tracing of the execution flow.

Taaku18 and others added 3 commits December 9, 2025 18:05
Replaces the claim plugin by fourjr to my claim plugin due to being fundamentally broken as of the current time. It has been created few support issues already that were not successfull to use the plugin.
@lorenzo132

lorenzo132 commented Jun 18, 2026

Copy link
Copy Markdown
Member

Hi, it looks like you are trying to merge this into the master branch.

Can you change this into the development branch?

Thanks for the PR tho!

@13-Prabhat

Copy link
Copy Markdown
Author

Hi! I've opened a PR for this issue and implemented a fix for the thread cleanup path when DM delivery fails.

Please have a look when convenient. Feedback is welcome. Thanks!

@13-Prabhat 13-Prabhat changed the base branch from master to development June 18, 2026 17:27
@13-Prabhat

Copy link
Copy Markdown
Author

Thanks for pointing that out. I've updated the PR to target the development branch now.

Please let me know if there's anything else that needs to be adjusted.

@lorenzo132

Copy link
Copy Markdown
Member

Thanks for the PR and update.
I will check this out coming weekend

@sebkuip sebkuip left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for the PR!

Since it's a small fix, we are looking into adding it to the V4.3.0 (beta/unstable for some time) update coming soon as well. We'll update your proposed changelog entry to fit correctly into the format we'll use then.

@StephenDaDev StephenDaDev added the changelog Changes in PR have been added to draft release that will be used for the changelog on the next ver. label Jul 13, 2026

@StephenDaDev StephenDaDev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good. Thanks for your contribution!

Deployed on test instance; tested working, bug has been resolved.

Added to changelog commit e8f141

@StephenDaDev StephenDaDev enabled auto-merge (rebase) July 13, 2026 13:53
@lorenzo132

Copy link
Copy Markdown
Member

Thanks for your contribution!

@StephenDaDev StephenDaDev merged commit 5d4175e into modmail-dev:development Jul 14, 2026
2 checks passed
@StephenDaDev

Copy link
Copy Markdown
Member

Thank you again for your excellent first-time contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog Changes in PR have been added to draft release that will be used for the changelog on the next ver.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: Confirm thread creation with react to contact causes bot to soft-block users with disabled DMs

6 participants