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

[5.0.0] Better handling server error responses #1279

Merged
merged 2 commits into from
Jul 19, 2023

Conversation

nan-li
Copy link
Contributor

@nan-li nan-li commented Jul 19, 2023

Description

One Line Summary

Better, but not yet perfect, handling of error responses for requests including conflicts and deleted users.

Details

Motivation

The SDK can receive many types of error responses to the requests it makes such as when we update a user but find the user has been deleted on the server, etc. We want to handle them gracefully.

Scope

Make OSResponseStatusType enum to categorize status codes

  • Borrow from Android and introduce 5 categories of server responses: Invalid, Unauthorized, Missing, Conflict, Retryable
  • Use this enum to categorize server error responses.

Handling errors

  • When we receive a missing status code, such that the user has been deleted, we will effectively perform a logout within the SDK and create a fresh, blank, anonymous user. But only if the SDK user is still the same one, otherwise, do nothing.
  • When a user is deleted, their subscriptions are as well. Therefore, set the subscription_id to nil so that the createUser request will hydrate with a new subscription_id. We still keep the same subscription model whose data is sent in the createUser request.
  • Requests to add alias can return a 409 (conflict) if any of the aliases exist on another user, and does not apply any of them, even the ones that can be applied. In this case, we remove these aliases from our local model.
  • A 404 response for removing an alias can be misleading. It may be that this user is deleted, or that the alias didn't exist on this user. Therefore, treat it as a no-op so we don't inadvertently nix the SDK's user.

Testing

Manual testing

Tested on iPhone 13 device using iOS 16.5. Sample of scenarios tested:

  1. SDK has an anonymous user -> delete the user via REST API -> add a tag -> a new anonymous user will be created and receive a new subscription_id
  2. Delete user via REST API -> add a tag and then an alias -> only 1 createUser request is made -> SDK creates a blank user
  3. Kill app -> delete user via REST API -> open app which calls fetch user -> a new createUser request is made
  4. Test adding an email to a deleted user
  5. In all scenarios I send a notification after these flows and receive it.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • [] Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

This change is Reviewable

* Borrow from Android and introduce 5 categories of server responses:
- Unauthorized
- Missing
- Conflict
- Retryable
* Use OSResponseStatusType to categorize server error responses.
* When we receive a missing status code, we will perform a logout in the SDK and start with a fresh blank anonymous user. But only if the SDK user is still the same one, otherwise, do nothing.
Base automatically changed from 5.0.0/support_multiple_listeners to major_release_5.0.0 July 19, 2023 21:36
@nan-li nan-li merged commit 651446b into major_release_5.0.0 Jul 19, 2023
1 check failed
@nan-li nan-li deleted the 5.0.0/server_error_responses branch July 19, 2023 21:37
nan-li added a commit that referenced this pull request Oct 30, 2023
[5.0.0] Better handling server error responses
nan-li added a commit that referenced this pull request Oct 30, 2023
[5.0.0] Better handling server error responses
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