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

notifs: Allow token-registration retry from troubleshooting; show server error message #5721

Merged
merged 6 commits into from
Apr 24, 2023

Conversation

chrisbobbe
Copy link
Contributor

@chrisbobbe chrisbobbe commented Apr 18, 2023

The token-registration request (POST /users/me/apns_device_token for iOS, POST /users/me/android_gcm_reg_id for Android), like any API request, could fail or sit there forever without succeeding or failing. This PR will make those conditions visible to the client. And when there's a failure, we'll show it to the user, so no one has to go through server logs, and the user can retry.

There might be better user-facing text to use, but even if we don't find it, I think this new information and the retry ability are worth it.

To make this screen recording I added this line in my dev server 🙂

    raise JsonableError(_("oh no! problem"))

RPReplay_Final1681857700_AdobeExpress (1)

To see how this param is already passed, look for

  applyReducer('session', …)

in src/boot/reducers.js.
Copy link
Member

@gnprice gnprice left a comment

Choose a reason for hiding this comment

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

Thanks! One nit in comments, but otherwise LGTM — please go ahead and merge modulo that.

@@ -201,6 +215,36 @@ export default (state: SessionState = initialState, action: Action): SessionStat
pushToken: action.pushToken,
};

case REGISTER_PUSH_TOKEN_START: {
// TODO(#5006: Do for any account, not just the active one
Copy link
Member

Choose a reason for hiding this comment

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

nit: close-paren (here and below)

No need for a migration; session state isn't persisted.
Now the two enum values TokenNotAcked and TokenUnknown together
cover the conditions that were previously covered by just
TokenNotAcked, and with no overlap.

TokenUnknown will be chosen instead of TokenNotAcked when we haven't
yet gotten a token from the platform.
@chrisbobbe chrisbobbe force-pushed the pr-token-registration-manual-retry branch from 90c2d3a to 155c657 Compare April 24, 2023 22:54
@chrisbobbe chrisbobbe merged commit 155c657 into zulip:main Apr 24, 2023
@chrisbobbe
Copy link
Contributor Author

Thanks for the review! Done.

@chrisbobbe chrisbobbe deleted the pr-token-registration-manual-retry branch April 24, 2023 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants