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

Treat more APNs errors as permanent rejections #280

Merged
merged 6 commits into from
Dec 6, 2021

Conversation

squahtx
Copy link
Contributor

@squahtx squahtx commented Dec 2, 2021

Fixes #279

@squahtx squahtx requested a review from a team as a code owner December 2, 2021 21:38
sygnal/apnspushkin.py Outdated Show resolved Hide resolved
sygnal/apnspushkin.py Outdated Show resolved Hide resolved
Comment on lines +90 to +91
(400, "DeviceTokenNotForTopic"),
(400, "TopicDisallowed"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Do both of these mean a broken client?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think both of these mean a developer has messed up in some way. I'm not 100% clear on the how. I think the former means the token is valid but for a different app ID and the latter seems to be something to do with mixing up VOIP and non-VOIP tokens/topics.

Copy link
Contributor

Choose a reason for hiding this comment

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

Alright! Thanks for the explanation!

sygnal/apnspushkin.py Outdated Show resolved Hide resolved
Copy link
Contributor

@clokep clokep left a comment

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 now! 👍

@squahtx
Copy link
Contributor Author

squahtx commented Dec 6, 2021

Thanks for reviewing!

@squahtx squahtx merged commit e13b89e into main Dec 6, 2021
@squahtx squahtx deleted the squah/handle_more_apns_errors branch December 6, 2021 15:23
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.

We should treat more kinds of APNs errors as permanent rejections.
2 participants