Why do we have to do this?

What is this cookie? We don't have a cookie in AppState top level

It's a convention, we do the same for migrating other stores

I copy-pasted in from here but looks like it's not needed anymore.

It was used to log-out user if migration fails - wondering how we should handle this now.

Hmmm, if usingCommServicesAccessToken is false, then we can probably set the cookie of Ashoat's keyserver to null... If usingCommServicesAccessToken is true, then I'm not sure what to do... We should probably have a similar logic that logs the user out if the CSAT is missing, but I don't really know. This logic, if it even existed, would probably try to do a recovery, and that's not what we want to happen
(for the record, I think the old logic didn't work for non-dev users, because resolveKeyserverSessionInvalidation would be called and the user wouldn't be logged out)
I think you should talk to @varun and @ashoat to discuss how to force a logout for users using CSAT

(for the record, I think the old logic didn't work for non-dev users, because resolveKeyserverSessionInvalidation would be called and the user wouldn't be logged out)

Isn't that what we want for the old logic? If the Redux store is corrupt, it forces a recovery login, which should result in the Redux data being fixed because the keyserver provides new data.

This logic, if it even existed, would probably try to do a recovery, and that's not what we want to happen

I think you're right that logging into identity again won't fix the Redux data. Perhaps what we want here is a version of a recovery login that clears all of the Redux state. Right now recovery login always leaves the Redux state in place.

In theory, a version of recovery login that clears the Redux state would be the same as a user-initiated login, right? It seems like a good idea to hide this from the user.

We should probably have a similar logic that logs the user out if the CSAT is missing, but I don't really know.

I think there are three things we need to do here:

1. When the app is started, if the CSAT is missing we should not show the app.
   • On native, this can be achieved by modifying LogInHandler to look at CSAT instead of hasUserCookie. @varun, can you create a task to track that?
   • This discussion is scoped to native, but we might need the same try-catch for web. The Redux lifecycle on web is currently a little different, so I'm not sure, but I think this will be handled by ENG-6547. In that case, the catch on web would just need to clear the CSAT from Redux.
2. If the CSAT is missing, we should try to automatically log in.
   • On native, this is tracked in ENG-6546.
   • On web, we can't automatically log in because we don't have credentials. As such, this is just ENG-6547.
3. We probably need a version of recovery login that resets the Redux state.
   • This is necessary if the recovery login was triggered by corrupt Redux state.
   • @inka, what do you think of this idea? This could potentially be handled by slightly modifying my task ENG-6586... instead of a boolean, we could have something like false | 'transparent_recovery' | 'reset_recovery'.
   • In addition, we'd need to modify the actions so that they can conditionally clear the Redux state. One way to do this is if 'reset_recovery' is set, we could add 'reset_recovery' to the action somewhere. This is perhaps something you could handle, since you're currently working on resetting state during login anyways.

I thought about this some more. If all we want is to clear the Redux state in this particular case, it seems like it would be more simple to just call resetUserSpecificStateOnIdentityActions here like we do in eg. D10860.

This avoids the need for any special differentiation between 'transparent_recovery' and 'reset_recovery'.

As for triggering keyserver session recovery code, this was some confusion on my part. We would want to trigger identity session recovery code here, so it has nothing to do with my task ENG-6586, which is about keyserver session recovery.

So here's what I think now:

1. If we see an exception here, we should reset the store.
   • We should clear Redux by calling resetUserSpecificStateOnIdentityActions (and maybe rename that function so it doesn't seem identity-action-specific).
   • We might need to do something additional to clear the auth metadata from CommCoreModule.
   • We may also need to call clearSensitiveData.
2. After the store is reset, an automatically identity recovery should occur on native.
   • Varun's work in ENG-6546 should handle this.
   • In the future, when this login concludes, the user should be able to get the KeyserverStore from the backup restoration.

@kamil, what do you think of making the changes from point 1 here? Do you know if we need to additionally clear the auth metadata or call clearSensitiveData?

I might be missing some context but it looks okay - and I think we have to call clearSensitiveData, A failed migration could leave the database in a bad state so we should delete and recreate it.

Not sure if this is the right approach to later automatically recover the session but I would suggest something like this:

try {
     await commCoreModule.processKeyserverStoreOperations(dbOperations);
   } catch (exception) {
     if (isTaskCancelledError(exception)) { //database is being deleted anyway
       return state;
     }
     const stateAfterReset = resetUserSpecificStateOnIdentityActions( 
       state,
       defaultState,
       nonUserSpecificFieldsNative, //this does not include `keyserverStore`
     );
     const keyserverInfos = {
       ...stateAfterReset.keyserverStore.keyserverInfos,
     };
     for (const key in keyserverInfos) { // similar approach to: https://github.com/CommE2E/comm/blob/406f32b4afe1bc12f42ea82610d9db63ae680946/native/redux/client-db-utils.js#L50
       keyserverInfos[key] = { ...keyserverInfos[key], cookie: null }; // removing cookie to logout and trigger database deletion
     }
     const keyserverStore = {
       ...stateAfterReset.keyserverStore,
       keyserverInfos,
     };
     return {
       ...stateAfterReset,
       keyserverStore,
     };
   }
   return state;
 },

I think we can add this on both web and native as a general approach to handle failed migrations.
This code is now causing some issues in selectors (baseCreateLoadingStatusSelector):
{F1109588}.

so before fixing it I want to ask if this is okay (cc. @inka, @ashoat)

I think this makes sense to me. We should probably factor that new logic out into a separate function, and call it from migrations 53, 58, and 59 in order to resolve ENG-6636.

Additionally – it looks like your example code is missing clearSensitiveData, which you indicated should be called. Will that additionally handle clearing the auth metadata?

I think this makes sense to me. We should probably factor that new logic out into a separate function, and call it from migrations 53, 58, and 59 in order to resolve ENG-6636.

make sense

Additionally – it looks like your example code is missing clearSensitiveData, which you indicated should be called.

I thought setting cookie: null would automatically cause clearSensitiveData in SQLiteDataHandler - I'll double-check that.

Will that additionally handle clearing the auth metadata?

yes, clearSensitiveData clears everything on native with metadata from SecureStore, on web it's persisted in redux-persist which we store in SQLite

We should also clear the fields of keyserver store cleared on logout: connection.connectionIssue, connection.queuedActivityUpdates , connection.lateResponses, and probably remove keyservers that are not Ashoat's keyserver.
We should probably also do that in the code you linked from client-db-utils.js....
If no user is logged in, we don't want to have the keyservers of the last user in the store... And I don't think they will be removed in some other way...