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

Handle CKError.changeTokenExpired #14

Merged
merged 3 commits into from Dec 12, 2017

Conversation

IngmarStein
Copy link
Contributor

This PR handles CKError.changeTokenExpired by deleting the stored token and retrying the request.

This fixes errors such as the following:

Error: Error fetching changes in zone <CKRecordZoneID: 0x151f6c9c0; ownerName=__defaultOwner__, zoneName=CarsZone>: client knowledge differs from server knowledge

Use UserDefaults.removeObject(forKey:) instead of NSObject.setNilValueForKey() because the latter raises an NSInvalidArgumentException.

Delete stored token and retry request.

Fixes errors such as the following:
```
Error: Error fetching changes in zone <CKRecordZoneID: 0x151f6c9c0; ownerName=__defaultOwner__, zoneName=CarsZone>: client knowledge differs from server knowledge
```

Use UserDefaults.removeObject(forKey:) instead of NSObject.setNilValueForKey() because the latter raises an NSInvalidArgumentException.
@caiyue1993
Copy link
Owner

Thanks @IngmarStein. It looks good!

Before merging this PR, could you please give us a routine to reproduce this issue? That will help us to check it and help the others to understand this issue. :)

@IngmarStein
Copy link
Contributor Author

Thinking about it, it may be because I started two SyncEngine objects and they both write to the same prefs key (https://github.com/IngmarStein/Kraftstoff/blob/master/Classes/AppDelegate.swift#L84). #8 will fix this, but I think the error should be handled anyways since there may be other circumstances leading to it.

@caiyue1993
Copy link
Owner

caiyue1993 commented Dec 11, 2017

I've checked it and you are right. We should use removeObject(forKey:) to remove value instead of setNilValueForKey().

But one more question, should we set databaseChangeToken = nil as well when changeTokenExpired error occurs?

@IngmarStein
Copy link
Contributor Author

It sure looks like, although this can be optimized by passing the information to retryOperationIfPossible if the operation concerns the database or a zone to only reset the respective token, not both.

@caiyue1993
Copy link
Owner

caiyue1993 commented Dec 12, 2017

Agree with you. But I haven't found a appropriate way to determine which operation is running into this error. I don't want to import a global variable to fix this issue. Setting both token to nil practically works.
(For the successors, this page could help you to know more about changeTokenExpired error)

@caiyue1993 caiyue1993 merged commit 2b03b41 into caiyue1993:master Dec 12, 2017
@IngmarStein IngmarStein deleted the change-token-expired branch December 12, 2017 05:09
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.

None yet

2 participants