Page MenuHomePhabricator

[native] cleanup SQLite handler code
AbandonedPublic

Authored by kamil on Nov 8 2023, 3:54 AM.
Tags
None
Referenced Files
F3376386: D9759.diff
Tue, Nov 26, 11:42 PM
Unknown Object (File)
Sun, Nov 24, 10:55 PM
Unknown Object (File)
Oct 27 2024, 5:22 PM
Unknown Object (File)
Oct 10 2024, 5:35 AM
Unknown Object (File)
Oct 5 2024, 9:35 PM
Unknown Object (File)
Oct 5 2024, 7:45 PM
Unknown Object (File)
Sep 28 2024, 9:30 PM
Unknown Object (File)
Sep 28 2024, 9:30 PM
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.