ggruen / CloudKitSyncMonitor

Monitor current state of NSPersistentCloudKitContainer sync
MIT License
465 stars 43 forks source link

Unrecognized AccountStatus error? #8

Closed sowenjub closed 3 years ago

sowenjub commented 3 years ago

Hi. I'm getting a "Can't return account credentials for account (redacted) because the account needs to verify new terms or is in suspended mode." (and indeed I need to accept new terms) in the logs, but:

What can I do to help?

sowenjub commented 3 years ago

OK, so one way to access this error is by performing a fetch directly on CKContainer.default().privateCloudDatabase. The most generic one being fetchAllRecordZones.

Too bad NSPersistentCloudkitContainer does not bubble this up.

ggruen commented 3 years ago

TL;DR: check to see if CloudKit is syncing - it probably is, so CloudKitSyncMonitor's summary methods (e.g. syncStateSummary) will probably return the right value for you, because they compensate for exactly this sort of thing. If CloudKitSyncMonitor is reporting that sync is happening, but it isn't, then let me know and I can help you debug it (I'd debug it, but I'd need a broken, er, partially broken, account).

The Too Long version:

Welcome to the exciting world of CloudKit. The "Code=2" error is "partial failure": https://developer.apple.com/documentation/cloudkit/ckerror/code/partialfailure

You can look up the codes here, by manually clicking through each enum value until you see the one that matches the code you received: https://developer.apple.com/documentation/cloudkit/ckerror/code (there's probably a better way, but fortunately, I haven't had to do it enough to find that better way).

If iCloudAccountStatus is .available, that should mean that CloudKit sent along that status after logging the partial failure. In short, the account is available, and it should sync fine, it was just letting you know that there's something wrong with it. Since CloudKitSyncMonitor's job is to let you know if sync is working or not, the summary methods (e.g. syncStateSummary) will check to see if an import or export happened, regardless of the state of setupState (because if an import or export happened, then setupState is irrelevant).

See my comment on commit 5dc3349 - I had to update the syncStateSummary (I think for one of the dot-releases of iOS 14) exactly because CloudKitSyncMonitor started reporting that sync was broken when it wasn't, exactly because it was getting that "partial failure".

sowenjub commented 3 years ago

Thanks for taking the time for such a detailed response. And for the welcome 😆

syncStateSummary returns an .error, which I use to display the appropriate symbol. I do think it's broken because I can't find my models in the iCloud dashboard. I'll see when I accept the terms.

For now, I created another ObservableObject to publish any error encountered by trying to fetchAllRecordZones, which returns the text about the terms. I don't know if this should end up in CloudKitSyncMonitor (it's basically the same as your updateiCloudAccountStatus).

ggruen commented 3 years ago

CloudKitSyncMonitor is designed to show the current state of the application's ability to sync, whereas CloudKit's notifications are designed to be a running stream of updates. As such, CloudKitSyncMonitor only stores the most recent error message. So, I agree that an object to publish errors wouldn't belong in CloudKitSyncMonitor.

But, if you write something that does look like a good addition to CloudKitSyncMonitor, please submit a pull request. :)

For now, I'll close this issue.

sowenjub commented 3 years ago

I'd be happy to write a PR if you think it helps. This is in essence what I use and would be adapted to CloudKitSyncMonitor.

    var cloudContainer: CKContainer

    @Published public private(set) var isAccessingZones: Bool? = nil
    @Published public private(set) var zonesError: Error?

    init(cloudContainer: CKContainer = CKContainer.default()) {
        self.cloudContainer = cloudContainer
        updateZonesAccessibility()
    }

    private func updateZonesAccessibility() {
        cloudContainer.privateCloudDatabase.fetchAllRecordZones { _, error in
            DispatchQueue.main.async {
                self.isAccessingZones = error == nil
                self.zonesError = error
            }
        }
    }
ggruen commented 3 years ago

Had to ponder this for a bit. It looks like you're using actual CloudKit, whereas I wrote CloudKitSyncMonitor mostly to check up on NSPersistentCloudKitContainer (which, to its credit, hasn't needed much checking up on since I wrote CloudKitSyncMonitor). As such (because I'm not too familiar with directly fetching from CloudKit), I'm having trouble seeing how isAccessingZones and zonesError would fit into CloudKitSyncMonitor. I'd say that if zonesError containing an error would produce a state in which NSPersistentCloudKitContainer was unable to sync, and if that error is not already detected via some other means in CloudKitSyncMonitor, then we should add in your code. Otherwise, let's keep it out to keep CloudKitSyncMonitor's purpose clear: it interprets the stream of notifications from CloudKit and translates them into current state and pretty icons.

sowenjub commented 3 years ago

I am using NSPersistentCloudKitContainer, but for some reason, NSPersistentCloudKitContainer is not bubbling the error, and I thought I'd get closer to the metal to try and grab the error.

So CloudKit is the only way I found to surface the error I would otherwise see in the logs. I use an API that doesn't require any knowledge about the NSPersistentCloudKitContainer/CoreData implementation (at first I thought I'd have to try and fetch the records directly, but just fetching the zones gave me the error).

Maybe there's another way to get the error using only NSPersistentCloudKitContainer, but I didn't find it.

ggruen commented 3 years ago

Ah, I see, so SyncMonitor.syncStateSummry shows .error, but lastError and/or setupState.error shows the obscure Error Domain=CKErrorDomain Code=2 "(null)" error instead of the underlying error, which is Can't return account credentials for account (redacted) because the account needs to verify new terms or is in suspended mode., and calling cloudContainer.privateCloudDatabase.fetchAllRecordZones grabs the underlying error if there's a fundamental problem (e.g. not being able to access the zones).

I'm curious to see how you'd work your fix into SyncMonitor, since it's kinda a workaround to NSPersistentCloudKitContainer not showing the underlying error (although now I'm wondering if there is some way to access that, since lots of error reporting gives some means to access "underlying error").

What do you do in your existing code, have SyncMonitor control an icon, check your ObservableObject and show the ObservableObject's error if it exists?

Anyway, if you feel inspired to work it into SyncMonitor, I do think it's a state that's worth being able to handle and report to the user (in fact, last week I had to make updates to my app's subscription error handling code for the same thing), so I think others would benefit from having it in SyncMonitor. I wonder if we can add a new icon for it - if we can find an SF symbol that can communicate "go agree to the terms". :)

sowenjub commented 3 years ago

This is what I display in my app.

CleanShot 2021-07-12 at 16 29 05

I'm not sure how this could be merged into the setupState even though it would make sense to have it there since there's already an error that could be useful in some instances.

For now, I treat it the same way you treat the iCloudAccountStatus (it's the Data Access row)

ggruen commented 3 years ago

That's a really nice status screen. I'm also not sure how to merge the Data Access into setupState, but I do like the display. If that's SwiftUI and is compact enough to include in the CloudKitSyncMonitor, I think it would be a nice addition.

To be clean, I'm going to close this issue, but if you find a way to include the status screen, please submit a pull request for it. :)

aehlke commented 1 year ago

this seems like a critical gap for the library's purpose, are there more recent thoughts on how to integrate?

ggruen commented 1 year ago

Hi @aehlke - I haven't thought about it recently. My main purpose for the library was to just detect whether or not cloud sync was broken, not to guide the user to fix it (because if iCloud sync is broken for them, it's broken for every app on the device and/or every device they own, so it's really up to iOS/macOS to tell them, and it probably is telling them).