Skip to content

First pass of easy to produce errors#577

Merged
jb3 merged 6 commits into
python-discord:mainfrom
ChrisLovering:Handle-DMChannels
Apr 2, 2021
Merged

First pass of easy to produce errors#577
jb3 merged 6 commits into
python-discord:mainfrom
ChrisLovering:Handle-DMChannels

Conversation

@ChrisLovering
Copy link
Copy Markdown
Member

@ChrisLovering ChrisLovering commented Feb 5, 2021

Relevant Issues

Closes #576

Description

I've made sure that anywhere with an on_message listener properly deals with messages from within DMs. This has been done by checking if message.guild is truethy, if so, its in a guild.

i found these by searching for on_message and running .help in a DM to see if anything raised an error.

Reasoning

This first pass just makes the bot ignore messages from DMs that were causing errors to be thrown.

Screenshots

For the riddle changes:
image

Additional Details

There are likely more cogs that raise errors when being ran through DMs, these changes were the easy to spot ones. I'm raising this PR to deal with these easy ones now, but intend to look into the other cogs further.

Did you:

  • Join the Python Discord Community?
  • If dependencies have been added or updated, run pipenv lock?
  • Lint your code (pipenv run lint)?
  • Set the PR to allow edits from contributors?

@ChrisLovering ChrisLovering requested a review from ks129 as a code owner February 5, 2021 19:22
Comment thread bot/exts/easter/easter_riddle.py
Comment thread bot/exts/evergreen/wolfram.py
Copy link
Copy Markdown
Contributor

@Shivansh-007 Shivansh-007 left a comment

Choose a reason for hiding this comment

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

Thanks! Looks Good To Me!

@Xithrius Xithrius added area: backend Related to internal functionality and utilities priority: 2 - normal type: feature Relating to the functionality of the application. labels Feb 7, 2021
@Xithrius Xithrius added status: WIP Work In Progress and removed priority: 2 - normal labels Feb 28, 2021
Base automatically changed from master to main March 13, 2021 20:09
Copy link
Copy Markdown
Contributor

@Akarys42 Akarys42 left a comment

Choose a reason for hiding this comment

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

LGTM

@Akarys42 Akarys42 enabled auto-merge March 19, 2021 14:19
@ChrisLovering ChrisLovering added status: needs review Author is waiting for someone to review and approve and removed status: WIP Work In Progress labels Mar 19, 2021
@Akarys42 Akarys42 disabled auto-merge March 19, 2021 14:34
Copy link
Copy Markdown
Contributor

@HassanAbouelela HassanAbouelela left a comment

Choose a reason for hiding this comment

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

One more thing: the issue command sitll errors out. I'll let you decide whether that's in scope or not.

Comment thread bot/exts/evergreen/issues.py Outdated
if not(
# Ignore messages from DMs
if not message.guild:
return
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should maybe send a message like riddle does? There's an argument there that it's a message, not a command, so it shouldn't error, but I don't know.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No, this is a message argument, so we don't want to send messages on every message the user puts in the DMs.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

agreed, we don't want to send an error on every message in a DM, failing silently here is fine imo.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If you move the regex up, you can get around this issue, and I think having an explicit “you can’t use issues in DM” is more useful than nothing. I can live without it.

Comment thread bot/exts/evergreen/wolfram.py
@jb3 jb3 enabled auto-merge April 2, 2021 23:38
Copy link
Copy Markdown
Member

@jb3 jb3 left a comment

Choose a reason for hiding this comment

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

:shipit:

@jb3 jb3 disabled auto-merge April 2, 2021 23:39
@jb3 jb3 merged commit 6ce46b6 into python-discord:main Apr 2, 2021
@Xithrius Xithrius removed the status: needs review Author is waiting for someone to review and approve label Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: backend Related to internal functionality and utilities type: feature Relating to the functionality of the application.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Automatic linking issues catches a error when message sent to it on DMs.

6 participants