Skip to content

Fix AttributeError when reminder jump_url is None#3496

Closed
oliveman-au wants to merge 1 commit intopython-discord:mainfrom
oliveman-au:fix/reminder-jump-url-none-crash
Closed

Fix AttributeError when reminder jump_url is None#3496
oliveman-au wants to merge 1 commit intopython-discord:mainfrom
oliveman-au:fix/reminder-jump-url-none-crash

Conversation

@oliveman-au
Copy link
Copy Markdown

send_reminder crashes with an AttributeError when jump_url is None. This can happen for older reminders created before the field was added to the API.

reminder.get("jump_url") returns None if the key is absent, but .split() was being called on it unconditionally, which causes the reminder to never be delivered.

This fix guards against None before calling .split() and skips the jump URL embed line if there's no URL, falling back to a plain channel.send instead.

@L3viathan
Copy link
Copy Markdown
Contributor

Does this issue actually occur? When was the field added to the API?

@oliveman-au
Copy link
Copy Markdown
Author

Does this issue actually occur? When was the field added to the API?

yeah it can, any reminder made before jump_url was added to the api would have it as None. the .get() instead of direct key access kinda hints the original dev knew it might be missing but just didnt guard the .split() call on the next line

@L3viathan
Copy link
Copy Markdown
Contributor

The reminders cog was added in 2018, the jump_url in 2019.
There's also no reminders in the actual DB without a jump_url, so no, it can't happen anymore.

@oliveman-au
Copy link
Copy Markdown
Author

The reminders cog was added in 2018, the jump_url in 2019. There's also no reminders in the actual DB without a jump_url, so no, it can't happen anymore.

thanks for checking that, good to know. i'll close this one

@oliveman-au
Copy link
Copy Markdown
Author

closing this as the maintainers confirmed all reminders in the db have jump_url set so this can't actually occur anymore

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.

2 participants