Do not delete all data by default on database errors

NetSysFire commented 2 years ago

[X] I have searched open and closed issues for duplicates
[X] I am using Signal-Desktop as provided by the Signal team, not a 3rd-party package.

Bug Description

I just updated Signal-Desktop and on startup it complained about a locked database. First thing I noticed was "Delete all data" is the default choice which is pretty catastrophic. I closed the error window, assuming this is non-fatal (a stray lockfile is definitely not fatal) and apparently this got interpreted as "Go ahead and delete all data" instead of "Ignore", probably (not sure about this one) triggering the next error about the uninitialized database.

Taken from main.log:

{"level":30,"time":"2022-04-22T22:05:31.863Z","pid":16,"hostname":"archlinux","msg":"MainSQL: updateSchema:\n  Current user_version: 50;\n  Most recent db schema: 54;\n  SQLite version: 3.37.2;\n  SQLCipher version: 4.5.1 community;\n  (deprecated) schema_version: 223;\n"}
{"level":50,"time":"2022-04-22T22:05:31.864Z","pid":16,"hostname":"archlinux","msg":"MainSQL: Database startup error: SqliteError: database is locked\n    at [REDACTED]/ts/sql/migrations/51-centralize-conversation-jobs.js:33:23\n    at sqliteTransaction ([REDACTED]/node_modules/better-sqlite3/lib/methods/transaction.js:65:24)\n    at updateToSchemaVersion51 ([REDACTED]/ts/sql/migrations/51-centralize-conversation-jobs.js:85:5)\n    at updateSchema ([REDACTED]/ts/sql/migrations/index.js:1594:5)\n    at Object.initialize ([REDACTED]/ts/sql/Server.js:342:40)\n    at MessagePort.<anonymous> ([REDACTED]/ts/sql/mainWorker.js:63:35)\n    at MessagePort.[nodejs.internal.kHybridDispatch] (node:internal/event_target:562:20)\n    at MessagePort.exports.emitMessage (node:internal/per_context/messageport:23:28)"}
{"level":50,"time":"2022-04-22T22:05:31.864Z","pid":16,"hostname":"archlinux","msg":"sql.initialize was unsuccessful; returning early"}
{"level":30,"time":"2022-04-22T22:05:31.865Z","pid":16,"hostname":"archlinux","msg":"close event {\"readyForShutdown\":false,\"shouldQuit\":false}"}
{"level":50,"time":"2022-04-22T22:05:41.137Z","pid":16,"hostname":"archlinux","msg":"Unhandled Promise Rejection: Error: Not initialized\n    at MainSQL.close ([REDACTED]/ts/sql/main.js:90:13)\n    at BrowserWindow.<anonymous> ([REDACTED]/app/main.js:443:15)"}
{"level":50,"time":"2022-04-22T22:05:45.766Z","pid":16,"hostname":"archlinux","msg":"(node:16) UnhandledPromiseRejectionWarning: Error: Not initialized\n    at MainSQL.close ([REDACTED]/ts/sql/main.js:90:13)\n    at BrowserWindow.<anonymous> ([REDACTED]/app/main.js:443:15)\n(Use `signal-desktop --trace-warnings ...` to show where the warning was created)"}
{"level":30,"time":"2022-04-22T22:05:46.857Z","pid":134,"hostname":"archlinux","msg":"key/initialize: Generating new encryption key, since we did not find it on disk"}
{"level":30,"time":"2022-04-22T22:05:46.858Z","pid":134,"hostname":"archlinux","msg":"config/set: Saving user config to disk"}

Of course, it would not accept my restored backup so it is extra annoying to me that all my chat history is gone and can not be restored/imported from my phone.

Steps to Reproduce

Not really applicable.

Actual Result:

Data directory gets cleared.

Expected Result:

Data directory does not get cleared without warning. Closing of error windows should be treated as Ignore or Abort.

Screenshots

n/a

Platform Info

Signal Version:

5.39.0

Operating System:

Arch Linux

Linked Device Version:

not relevant

Link to Debug Log

n/a, see above

scottnonnenberg-signal commented 2 years ago

@NetSysFire To fully understand what went wrong, we'll need your complete log. Looking at the code, the default cancel action shouldn't delete everything. But it does look like we were unable to find the decryption key for the database, in addition to being unable to access the database due to that lock.

Thinking about the underlying cause for the database lock, did you force-quit Signal Desktop? Did your computer crash while Signal Desktop was running? Are you accessing the database from other programs?

NetSysFire commented 2 years ago

Thinking about the underlying cause for the database lock, did you force-quit Signal Desktop? Did your computer crash while Signal Desktop was running? Are you accessing the database from other programs?

Nope. I just updated signal-desktop, did the usual ctrl+q from inside the application which is a graceful quit to then start the new version. No idea what happened.

To fully understand what went wrong, we'll need your complete log.

The log is gone since it got rotated, unfortunately, I only copy-pasted the relevant part in here. But everything else before that was benign I am pretty sure, I did not see anything suspicious.

Looking at the code, the default cancel action shouldn't delete everything.

The problem is that the default action is "delete everything" (highlighed by default and on the right side of the window, where the default button tends to be) and closing the window which shows the error appears to invoke this default action.

scottnonnenberg-signal commented 2 years ago

@NetSysFire Thanks for the additional detail. We'll rearrange the buttons and make sure that Escape does the right thing.

NetSysFire commented 2 years ago

This was not me pressing Escape. I just closed the window, as in pressing the X on the top right of the window. Escape should be handled by signal-desktop itself.

scottnonnenberg-signal commented 2 years ago

@NetSysFire Top right of what window? The one that had the error and buttons in it? Escape is the only way to trigger that cancel action on macOS.

NetSysFire commented 2 years ago

This is on Linux as already mentioned. I just closed the window which had the error and "delete all data?" in it, not the main signal window, hoping it would just ignore that error and carry on.

I have no idea about macOS and if it reserves Escape for the system, but in any case this is not what I pressed. Escape would be passed to signal-desktop. So in order to reproduce that:

Corrupt your database on purpose or do something else to trigger the dialog.
Close the window (on macOS it is I think clicking that red orb, on windows and many linux graphical environments it is pressing that X) which reports said error. Alt+F4 will do the very same thing.
Expected result: It should cancel that action. Actual result: Your data gets deleted.

scottnonnenberg-signal commented 2 years ago

@NetSysFire There's no red orb on macOS because dialogs like that don't have those elements. Thanks for the detail.

staffa commented 1 month ago

I'm not 100% positive, but I believe I have encountered the same behavior on Windows. I'll email my logfile with my username in the subject. I found several entries in the log that suggest a similar behavior, though without spending more time understanding it fully, I cannot say for sure. IF however you would like me to put this in a new post, just let me know :)

What happened:

Yesterday (8/20, PDT), I opened signal desktop (after 30 days) and so the view had the following: previous chat history (up to last time device was linked), as well as a notification that the device needed to be relinked. (It MAY have also had the update needed message, but I could be thinking of my other computer that I did right before this one—if it DID show update needed, I definitely clicked it.)
(If the update initialized, it apparently didn't work, since when I opened it today, it was on version 7.17.0)
I tried to relink multiple times, but could not, getting a brief toast notification of "Network error" (though I didn't notice this the first few times). I searched this error, and then I checked my connections and that I was able to send/receive messages, and I checked to update my app (but only looked at the phone version, perhaps foolishly, but I assumed that my desktop app was up to date) and it was up to date.
I tried a few more times and eventually got it to 'finish on your device' where you can name the device. I did so and hit enter, but either (a) experienced an error that I can't recall the details of (but BELIEVE it had an option to delete/start over or something), then hit the X button to close the error, and all of signal closed unexpectedly or (b) all of signal closed upon hitting enter (Sorry, my memory is impaired due to sleep deprivation...). This is where I believe I experienced the same result as the original poster, though didn't know it yet.
Upon opening it again, it went straight to the "link your device" QR code screen.
I tried linking a few more times, but to no avail.
Today (8/21, ~18:20 PDT) I tried linking again, and got an error saying it failed and I should update. THIS time I thought to check the app version, and indeed I was on 7.17.0 (even though when I eventually succesfully linked, I saw that "automatically download updates" was checked [possible other ticket?]).
I downloaded from the signal website, started the update while signal was open, so clicked OK in the prompt to close Signal and update.
Upon opening signal, it as previous, went straight to the link device QR code screen. I scanned, it worked, great.
BUT: all history that was present previously had been deleted.

This was not expected, and at no time did I receive any indication that the history would be deleted, and if it was presented as an option, I never clicked it, or even accidentally selected it with a poorly timed pop-up.

I do have a recent backup of the disk, but I'm not sure if just copying the files back over would work. /shrug

As mentioned, I've emailed the logfile to support@signal.org. Please let me know if you need more data or have any updates! Thanks so much!

signalapp / Signal-Desktop