Page MenuHomePhabricator

[native] cleanup SQLite handler code
AbandonedPublic

Authored by kamil on Nov 8 2023, 3:54 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Jun 27, 8:53 AM
Unknown Object (File)
Mon, Jun 24, 6:14 AM
Unknown Object (File)
Sun, Jun 23, 11:16 AM
Unknown Object (File)
Sat, Jun 22, 11:10 PM
Unknown Object (File)
Thu, Jun 20, 9:22 PM
Unknown Object (File)
Wed, Jun 19, 1:35 PM
Unknown Object (File)
May 21 2024, 1:59 AM
Unknown Object (File)
May 21 2024, 1:51 AM
Subscribers

Details

Reviewers
atul
Summary

Removing some no-longer needed code after D9682.

Test Plan

login/logout and confirm it works, make sure this log SQLite database deletion was triggered by change in logged-in user credentials is printed

Diff Detail

Repository
rCOMM Comm
Branch
refactors-us-to-ops
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil held this revision as a draft.
kamil published this revision for review.Nov 8 2023, 5:37 AM
kamil added inline comments.
native/data/sqlite-data-handler.js
215–216

Looks like those are unused so can be removed.

There is also cookie in deps and not used directly, but I believe we want this code to run when the cookie changes

atul requested changes to this revision.Nov 8 2023, 9:16 AM

Probably missing something here.

My understanding is that D9682 handled removing the messageStore.threads asserts, but aren't there other exceptions that could be caught here? Anything from await commCoreModule.getClientDBStore(), threadStoreOpsHandlers.translateClientDBData(threads), and reportStoreOpsHandlers.translateClientDBData(reports)?

Apologize for churn, but would appreciate a bit more context

native/data/sqlite-data-handler.js
194–200

Are there no other exceptions that we might want to log?

This revision now requires changes to proceed.Nov 8 2023, 9:16 AM

Appreciate feedback @atul.

I was sure this alert was introduced exclusively for message/thread store asserts. Also, I had in mind this issue: ENG-5160 (but it's not occurring anymore).

That's why I was convinced we can remove this code but you're right, we can keep logging different errors, could always improve debugging (however I haven't seen anything like this for a long time).

I am abandoning this diff, if anyone still thinks it's better to remove this code I can reopen it.