vladimiry / ElectronMail

Unofficial ProtonMail Desktop App
GNU General Public License v3.0
1.49k stars 96 forks source link

Local store indexing timeout #551

Open gregbtm opened 1 year ago

gregbtm commented 1 year ago

Unhandled Promise Rejection Error: Failed index emails in 30000ms (mails portions size: 1000) at C:/Users/gregb/AppData/Local/Programs/ElectronMail/resources/app.asar/app/electron-main/index.cjs:88645:50 at doInnerSub (C:/Users/gregb/AppData/Local/Programs/ElectronMail/resources/app.asar/app/electron-main/index.cjs:75156:33) at outerNext (C:/Users/gregb/AppData/Local/Programs/ElectronMail/resources/app.asar/app/electron-main/index.cjs:75150:40) at OperatorSubscriber._this._next (C:/Users/gregb/AppData/Local/Programs/ElectronMail/resources/app.asar/app/electron-main/index.cjs:73611:15) at Subscriber.next (C:/Users/gregb/AppData/Local/Programs/ElectronMail/resources/app.asar/app/electron-main/index.cjs:71880:123) at AsyncAction. (C:/Users/gregb/AppData/Local/Programs/ElectronMail/resources/app.asar/app/electron-main/index.cjs:73465:26) at AsyncAction._execute (C:/Users/gregb/AppData/Local/Programs/ElectronMail/resources/app.asar/app/electron-main/index.cjs:77744:18) at AsyncAction.execute (C:/Users/gregb/AppData/Local/Programs/ElectronMail/resources/app.asar/app/electron-main/index.cjs:77737:28) at AsyncScheduler.flush (C:/Users/gregb/AppData/Local/Programs/ElectronMail/resources/app.asar/app/electron-main/index.cjs:77816:32) at listOnTimeout (node:internal/timers:559:17)

vladimiry commented 1 year ago

There should be something that prevents the indexing to complete in the expected time. Is there any other error in the log file, the line number of which is close to the original error (the error line would likely include electron-preload part/keyword)?

You could increase the timeouts.indexingBootstrap value in the config.json file located in the app settings folder, but it's unlikely to help, as the default value should be sufficient to index the 1k mails portion.

vladimiry commented 1 year ago

Handling #571 should prevent this kind of error, as a real database will be doing the indexing.

vladimiry commented 1 year ago

Based on pure guesses, I've dropped "html to txt" conversion via iframe as a fallback for conversion using turndown library (so if turndown fails, html content will be indexed vs extracted text content). So maybe indexing gets more stable in the next app release.

itisafrickinhighlander commented 1 year ago

Based on pure guesses, I've dropped "html to txt" conversion via iframe as a fallback for conversion using turndown library (so if turndown fails, html content will be indexed vs extracted text content). So maybe indexing gets more stable in the next app release.

Unfortunately, if the aforementioned next release is 5.1.8, then it didn't help.

I enabled local store yesterday, let it run overnight to index and download everything.

Today I noticed that one of issues I experienced in the past, when writing a new email starts lagging as I type returned (it looks it is somehow related to this issue), I restarted eM today and besides a very high RAM usage (initially almost 4GB for 16k emails) it once again thrown the Error: Failed index emails in 30000ms (mails portions size: 1000) - I have been experiencing this issue for a very long time and I know you used to recommend increasing it to 60 seconds, but at least in my case it never helped.

What is the worst about this very specific error is that even restarting eM and disabling local store won't help - this error keeps coming even without the local store enabled and so far the only workaround which worked for me is to switch API point or switch off and back on the persistent session (I do both at once, I am not sure which actually helps).

vladimiry commented 1 year ago

What is the worst about this very specific error is that even restarting eM and disabling local store won't help

Disabling "full-text search" or "local store" features/toggles should prevent the live/on-app-start indexing used for "full-text search" feature to happen. So looks like one day I need to look into it to make sure those toggles when turned off actually disable/prevent the indexing to happen.

vladimiry commented 1 year ago

It could that "turndown" library doing "html-to-text" thing is freezing on some data (mails body content), as at my side on very low performant laptop (fanless one), on ~10k mails db it gets indexed without any issues in less than 10s.

Since v5.1.9 I disabled the iframe-based "html-to-text" fallback if "turndown" fails, but it might be freezing vs failing. So I guess I might provide an opt-in option of disabling "html-to-text" completely or automatically dropping freezing calls (would complicate implementation, so unlikely to happen).

So looks like one day I need to look into it to make sure those toggles when turned off actually disable/prevent the indexing to happen.

Not sure what happens first, I switch to SQlite database and its built-in full-text indexing as pointed in #571, or I polish the existing indexing.

itisafrickinhighlander commented 1 year ago

You got a good point there. It might also suggest why I started noticing issue gradually and over time. I have just tested turning on the local store for another account with only under 100 emails in total and it was able to do it all and never timed out even after a restart of the app.

I wonder if the decision to switch to SQLite could also yield some improvements in performance when working with large amount of emails and what the search performance could be like. Each of those, however, seem like quite an endeavour.

vladimiry commented 1 year ago

I wonder if the decision to switch to SQLite could also yield some improvements in performance when working with large amount of emails and what the search performance could be like. Each of those, however, seem like quite an endeavour.

Switching to db/SQLite would allow dropping some code (reduced maintenance burden), increase startup performance as no need to decrypt the entire db and reduce memory use as no need to load entire decrypted local store data in memory.

So switching to SQLite is better for UX, but some privacy of encrypted data will be lost. For example, count of records/emails and emails IDs will likely remain open as using db is not like using an encrypted data blob.

The app was initially designed and built for myself and a few friends, but currently used by thousands (according to downloads count), so switching makes sense.