Skip to content
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

fix(Action): Don't crash when partials are disabled #4822

Merged
merged 2 commits into from Sep 15, 2020

Conversation

wasdennnoch
Copy link
Contributor

Please describe the changes this PR makes and why it should be merged:

Fixes #4821
this.getMember() returns undefined if there is no cached member and partials are disabled for members.

Also only checks for the correct channel type in TypingStart if there actually is a known channel, I've missed this in my previous PR.

Status

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating

Semantic versioning classification:

  • This PR changes the library's interface (methods or parameters added)
    • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)
  • This PR only includes non-code changes, like changes to documentation, README, etc.

@@ -7,7 +7,7 @@ const textBasedChannelTypes = ['dm', 'text', 'news'];
class TypingStart extends Action {
handle(data) {
const channel = this.getChannel(data);
if (!textBasedChannelTypes.includes(channel.type)) {
if (channel && !textBasedChannelTypes.includes(channel.type)) {
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we exit early if no channel is present?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Though that way we don't cache any member data anymore if it's there. Depends on if we want to cache everything we possibly can or are more lax.

Copy link
Member

Choose a reason for hiding this comment

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

I mean... if we don't have the channel but we cache the member...we still don't emit any events, so I believe it's probably best to exit early. That, of course, is open for discussions tho

@kyranet kyranet requested a review from iCrawl September 15, 2020 06:47
@iCrawl iCrawl merged commit 8fa3a89 into discordjs:master Sep 15, 2020
@kyranet kyranet mentioned this pull request Sep 22, 2020
1 task
GiorgioBrux pushed a commit to GiorgioBrux/discord.js that referenced this pull request Oct 1, 2020
GiorgioBrux pushed a commit to GiorgioBrux/discord.js that referenced this pull request Oct 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error due to PR #4791 Cannot read property user of undefined
4 participants