Summary:
Previously we stored message bodies uncompressed inside two different
databases. This was bad for a few reasons:
* Duplicate data in multiple places is an obvious waste of disk space
* Uncompressed data also made the disk footprint bigger than it might
otherwise be
* Storing these large message bodies in the database made the db file
larger than it otherwise could have been, increasing the size of
tables and slowing down queries.
This diff adds support for storing message bodies outside of the
database in compressed flat files. It changes the use of the body column
in the K2 database and the MessageBody table in the edgehill database to
contain a blob of JSON that contains a path to the file on disk. We
use the new format in an incremental fashion without having to perform an
actual database migration by first checking if the body matches our expected
JSON format and treating it appropriately if it doesn't. Both databases refer
to the same file on disk, thus deduplicating the messages bodies. We also
transparently support gzipping the message bodies stored on disk when we
read from/write to the files. The real world space savings depends on the
compressibility of the messages, but we've seen up to ~60% improvement in disk
space usage for certain inboxes. Typical savings are closer to 20%. Also, by
storing messages as separate files on disk we can potentially integrate with
Spotlight search at some point in the future.
Test Plan:
Run locally, make sure that upgrade to this doesn't hose things,
look at size of DB, read emails
Reviewers: evan, spang, halla, juan
Reviewed By: halla, juan
Differential Revision: https://phab.nylas.com/D4422
Summary:
We weren't passing benchmarkMode through in the default window options,
so the worker window (or any other spawned window) would think it wasn't in
benchmark mode.
Test Plan:
Run locally, verify that the worker window thinks it's in
benchmark mode
Reviewers: halla, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4427
Summary:
Given that this error is unrecoverable it shouldn't be retryable.
Additionally, this commit improves error handling for this error in a
few ways:
- Don't delete all databases, just edgehill.db which is the one we know is corrupted. (Use `handleDatabaseUnrecoverableError` instead of `Actions.resetEmailCache`)
- Message the user about the database being reset so the app doesn't restart without notice, and make sure that this message is only displayed once by moving it to the main process
Test Plan: manual
Reviewers: mark, spang, evan, halla
Reviewed By: evan, halla
Differential Revision: https://phab.nylas.com/D4433
Summary:
This commit addressess issue T8135, which prevents the app from
starting.
This was happening because when 3111c166 landed, we added database
access from the main (backend) electron process to be able to read the
identity now stored in the database: https://github.com/nylas/nylas-mail/commit/3111c166#diff-1efa26fa0ae1603366b2c0033d971028R44
However, we omitted to add any error handling, so if the database failed to open
due to a database malformed error (which it does:
https://sentry.io/nylas/nylas-mail/?query=is%3Aunresolved+release%3A2.0.14+malformed&statsPeriod=14d), the app will just fail to start,
given that this happens during the initialization of the main process.
Additionally, the fact that we had no error handling increased the error
reports for malformed errors given that we would never handle them, so
every-time we opened the app we would report the same error
This commit adds the same error handling we have in the DatabaseStore
and moves the code around so it's available both in the main and
renderer processes.
After this commit, if the database fails to open during main process
initialization, due to malformed errors or others, we will correctly
inform the user that the database is corrupted, rebuild it, and restart
the app.
Test Plan:
manually throw errors during setup, verify that we handle them
correctly
Reviewers: mark, spang, evan, halla
Reviewed By: evan, halla
Differential Revision: https://phab.nylas.com/D4431
Summary:
Which was arguably already a code smell being inside the
DatabaseStore
Test Plan: manual
Reviewers: mark, halla, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4430
Summary:
It seems that sometimes we get events that don't have `event.detail.reason`.
This commits tries to make our best guess, or report info about the event shape
Test Plan: manual
Reviewers: mark, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4420
Summary:
This changes the `name` option we pass to `electron-windows-installer`,
which makes it so that the app is installed under
`AppData\Local\NylasMail` instead of `AppData\Local\Nylas` where the
old N1 binary lives
Test Plan: manually tested the builds from `ci-test-4` branch on the surface machine
Reviewers: mark, spang, khamidou, halla
Reviewed By: khamidou, halla
Differential Revision: https://phab.nylas.com/D4419
Summary:
This makes grepping the codebase really painful. We don't need these
docs here
Test Plan: manual
Reviewers: mark halla spang evan
Subscribers:
Summary:
We listen for unhandled rejections on both the `process` and the `window`. From
my understanding, both are necessary because some errors might escape
one or the other. However, most errors get caught by both handlers, so
we don't want to double report them to sentry (or the console).
To prevent this, this commit debounces the unhandled rejection handler
for a very short time so that the same error only gets reported once.
Additionally, it rate limits reports to sentry based on the stack trace
Test Plan: manual
Reviewers: mark, halla
Reviewed By: mark, halla
Differential Revision: https://phab.nylas.com/D4413
Summary:
Previously, we were listening for unhanlded rejections on both the
`window` and the `process`. However, the window handler didn't receive
an error as a parameter, but rather a window event. When trying to
report this event and sending it over IPC we would get an ipc parse
error.
This commit makes it so we correctly report the error instead of the
window event.
Test Plan: manual
Reviewers: mark, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4412
Summary:
We were reporting 1m+ of these events, heavily contributing towards our
quota
Test Plan: manual
Reviewers: halla, mark
Reviewed By: halla, mark
Differential Revision: https://phab.nylas.com/D4406
Summary:
Add a sequentialId check in isDependentOnTask() so that the first
SyncbackMetadataTask won't get blocked on other SyncbackMetadataTasks
in the queue for the same pluginId.
Addresses T8112
Test Plan: Manual
Reviewers: spang, mark, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4404
Summary:
Put the IMAP option back on the provider selection page within
onboarding. Also remove the 'hidden' property on account types since I
don't think we'll be using it for any other providers, and remove the
'Add Custom IMAP Account' from the application menu.
Addresses T8097
Test Plan: Manual
Reviewers: spang, mark, juan
Reviewed By: mark, juan
Differential Revision: https://phab.nylas.com/D4402
Summary:
Previously, every time our NylasLongConnection changed its status to
`connected`, we would reset our backoff scheduler, assuming that
every time our status changed to `connected` it meant that we would not
receive any errors.
However, if our backend is down or overloaded (or returning 401s like in
our recent outage), we would do the following:
1. Establish connection, change status to `connected`
2. Reset backoff scheduler
3. Receive error
4. Close connection
5. Schedule next retry using backoff scheduler. Delay will be 1s or less, because scheduler was reset in 2.
6. Retry after delay,
7. Repeat from 1., ad infinitum
This caused us to consistantly hammer our servers and render our
exponential backoff useless.
This commit makes it so we conly reset the backoff scheduler when we
actually receive deltas, and increments the max backoff to 10 minutes
Test Plan:
run against local n1Cloud, manually return 401s from the api, verify that we
didn't backoff at all before this diff, verify that we backoff correctly after
this diff
Reviewers: mark, spang, halla
Reviewed By: mark, spang, halla
Differential Revision: https://phab.nylas.com/D4401
Summary:
There are some settings that apply to dev and prod modes that we don't want
to use while benchmarking. E.g. the folder where we store messages,
whether we generate long stack traces in our bluebird promises, etc.
This diff adds a benchmark mode so that we can change these settings to
something that works better for benchmarking.
Test Plan: Run locally, verify it works
Reviewers: evan, juan, spang
Reviewed By: juan, spang
Differential Revision: https://phab.nylas.com/D4374
Summary:
Previously, SyncbackMetadataTasks for the same plugin did not depend on
each other, so they could try to modify the same piece of metadata
concurrently, which would likely produce version conflict errors.
This was happening with send reminders because we were queuing 2 of
these tasks back to back upn send success. Adding this dependency fixes
the version conflict errors when setting the metadata
Test Plan: manual
Reviewers: evan, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4393
Summary:
This was annoying because we wouldn't fetch the identity when running `local`
env which produced all sorts of errors
Test Plan: manual
Reviewers: evan, spang, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4380