[lib][native][web] Wait until database deletion before rejecting login/registration attempt
ClosedPublic
Actions

Authored by ashoat on Apr 30 2024, 11:37 AM.

Details

Reviewers

varun

Commits

rCOMMe27a6dfb78f0: [lib][native][web] Wait until database deletion before rejecting…

Summary

In the prior diff, we introduce some logic for resetting login state when authoritative keyserver auth fails during login or registration.

However, we don't wait for the login state to be reset before letting the user retry. That can lead to TASK_CANCELLED errors when we attempt ops on SQLite while it is in the process of being cleared (initially described on Linear).

To avoid this, we won't let the user retry until the database deletion is complete. During this period the UI will continue to show the login or registration as being in progress.

Note – would normally put @tomek and @inka on this review, but since they're out for the rest of this week, I'm asking @varun to review.

Depends on D11845

Test Plan

Replicate the test plan in the parent diff (D11845)
Repeatedly press the registration button while it's spinning, so that when it stops spinning it is immediately triggered
Confirm that there is no TASK_CANCELLED error during identity auth, and no TASK_CANCELLED error is printed to the logs

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ashoat created this revision.Apr 30 2024, 11:37 AM

ashoat added a child revision: D11847: [native] Show "unknown error" alert if keyserver auth fails during registration.

ashoat added inline comments.Apr 30 2024, 11:41 AM

native/data/sqlite-data-handler.js
46	I searched for other calls to `clearSensitiveData`. There was only one other one, which was for resetting the database in `wipeAndExit`. I don't think that's relevant here (we don't need to wait for that to happen, since we don't expect it to happen)
web/shared-worker/sqlite-data-handler.js
46	I searched for other calls to `sharedWorker.init` with `clearDatabase: true`. There was only one other one, which was for resetting the database if some of our SQLite ops fail. I don't think that's relevant here (we don't need to wait for that to happen, since we don't expect it to happen)