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:
After https://github.com/nylas/nylas-mail-all/commit/008cb4c, the shape of
account.connectionSettings changed, which means that ids for accounts
will also change, given that they are based on the connection settings (see
Account.hash()).
This is fine in most cases, except for accounts that were running on a
version of Nylas Mail before 008cb4c, then upgraded to a version
after 008cb4c, and then re-authed their account.
In this scenario, re-authing the account will create a second account with
a different `id` but with the same email address, along with an extra
sequelize database, and will start a second SyncWorker for the same
account. App-side, the `AccountStore` will correctly overwrite the first account,
so users would correctly see just 1 account in the sidebar. However, given
that 2 SyncWorkers would be running for the same account,
message ids would collide in edgehill.db and this would cause
threads to disappear from the ui as if they were being deleted.
To fix this, we need to do 2 things:
- Upon app start, we remove any duplicate accounts that might have been created due to this bug before starting any sync workers
- If we detect that we are going to create a duplicate account upon auth success, we delete the old account first. This will effectively cause sync to restart for the account.
Test Plan:
Verified problem: Checked out a commit before 008cb4c, authed an account, checked out master, re-authed account, verified that duplicate accounts are created.
Then test 2 scenarios:
- With duplicate accounts present, checked out this commit, verified that duplicate account would be removed and sync would function normally.
- With an account authed on a build before 008cb4c, checked out this commit, re-authed the account, and verified that duplicate account wouldn't be created
Reviewers: mark, halla, khamidou, spang
Reviewed By: spang
Subscribers: tomasz
Differential Revision: https://phab.nylas.com/D4425
Summary:
By nullifying the key inside `workersByAccountId` we would attempt to
access accountIds that no longer existed in other parts of the code by
using Object.keys()
This makes it so we correctly completely remove the record from the map
Test Plan: manual
Reviewers: halla, spang, mark
Reviewed By: mark
Differential Revision: https://phab.nylas.com/D4426
Summary:
This was showing up in Sentry. Check to see if the recipient has an email
before calling toLowerCase.
Test Plan: yolo
Reviewers: halla, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4423
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:
When we had to refresh our credentials for the mail listener connection we
weren't disposing of the old connection which caused us to slowly leak
connections from the IMAP connection pool, eventually requiring a
restart. Now we always dispose of the connection prior to requesting a
new one from the pool.
Test Plan: Run locally always refreshing credentials, verify we don't get stuck
Reviewers: spang, evan, halla, juan
Reviewed By: halla, juan
Subscribers: halla
Differential Revision: https://phab.nylas.com/D4414
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:
This reverts commit c808438ee9.
Adding an overarching transaction to `processMessage` is causing
database locked errors when processing deltas, specifically when trying
to write to the Transaction table, given that that write happens
asynchronously inside the execution of `processMessage`, but doesn't
have access to its transaction. See T8117
Given that the performance gains from wrapping messgae processing in a
transaction were not significant, let's revert this to prevent db
locking issues
Test Plan: manual
Reviewers: halla, mark
Reviewed By: mark
Differential Revision: https://phab.nylas.com/D4411
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:
When checking for flag updates for gmail accounts, when syncing for the
first time the all mail folder won't be available because we haven't
fetched the folder list, so it will throw a runtime error
Test Plan: manual
Reviewers: mark, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4405
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:
Since buildMime isn't always used explictly for the send operation (we
mostly use it for stuffing the sent folder, which is a separate
operation), move it to MessageUtils. Additionally, add an includeBcc
option. We don't want the BCC header to be visible on outbound messages,
but it needs to be present on the version that is saved to the sent
folder.
When we weren't including the BCC header in the sent folder version, we
were getting an id mismatch between our optimistic message and the
message that gets processed by the sync loop (because we include the bcc
participants when computing the id hash). When the sent message was a
reply, the optimistic message included the threadId, and this had the
side effect of leaving both versions of the sent message in the thread.
Since the optimistic message never receieved it's imap UID, any action
performed on the thread would fail with the missing UID error as seen in
T7941. Adding this BCC header will allow us to compute the proper id
hash and reconcile the synced message with the optimistic message.
Test Plan: manual
Reviewers: evan, mark, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4365
Summary: Just to mixpanel. Addresses T8095
Test Plan: Meh
Reviewers: mark, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4403
Summary:
Normally, the imap connection emits an 'update' event when there are any
flag changes. Gmail, however, only emits the 'update' event when there
are new or removed messages. If there's a fairly large batch size being
synced in the sync loop, we don't want to wait until the end of that to
check if the highestmodseq has changed. To get around this, this diff
updates the new mail listener connection to periodically poll for the
highestmodseq and trigger the same update handler when changes are
detected.
Test Plan: manual
Reviewers: evan, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4397
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:
Something weird happens with the mime building when the participant name
has an @ symbol in it (e.g. a name and email of hello@gmail.com turns
into 'hello@ <gmail.com hello@gmail.com>'), so replace it with
whitespace.
Test Plan: manual
Reviewers: evan, mark, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4363
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:
We do lots of writes while processing a single message. To reduce the
write churn from lots of mini-transactions, this diff threads one
overarching transaction to everything in processMessage.
Test Plan: Run locally
Reviewers: spang, evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4394
Summary:
The defaults are a little low. edgehill.db already does this. There's no
obvious effect on smaller dbs, but we should at least make the two
consistent.
Test Plan: Run locally
Reviewers: spang, evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4395
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:
Fixes T8076. So, the Gmail OAuth API returns us an expiration time in milliseconds, but we convert it to seconds everywhere, except in the cloud-api. This caused us to never refresh tokens because we'd be comparing seconds to milliseconds.
Fix this by converting the `expiry_date` from our credentials to seconds.
Test Plan: Tested manually. Will do more testing tomorrow, after my credentials expire.
Reviewers: evan, juan
Reviewed By: evan, juan
Maniphest Tasks: T8076
Differential Revision: https://phab.nylas.com/D4390
Summary:
MySQL would insert bad JSON due to emoji errors. This would cause JSON
parse errors and prevent us from ever reading objects from the DB. We now
catch and return null instead with an error which will at least let the
query complete to let us isolate and deal with the malformed obeject (like
destroy it)
Test Plan: manual
Reviewers: spang, halla, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4389
Summary:
We need this in order to be able to store 4-byte emoji and other
extended unicode characters in the database.
Test Plan: deploy to staging - need the change that sets the connection charset on staging :(
Reviewers: khamidou, juan, evan
Reviewed By: juan, evan
Differential Revision: https://phab.nylas.com/D4387
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
Summary:
We were not generating the XOAuth2 token on the first go when connecting
accounts. This would cause the first send later to fail. Since this
happens in ISO core, we needed to move the function out of cloud-core and
import from other places that use it
Test Plan: Manual
Reviewers: khamidou, juan, halla, spang
Reviewed By: spang
Differential Revision: https://phab.nylas.com/D4375
Summary:
Previously, we would initialize an `AccountPool` with an account
instance and reference the account as an instance variable. Then,
every time we created a connection, we used the account we had a
reference to obtain the credentials.
However, if the credentials on the account changed, e.g. when we refresh
the access token for gmail accounts, the connection pool would never
realize that the account had new credentials and always try to create
the connection with the old credentials, causing the sync worker to
enter an error loop of `Invalid credentials` errors
To fix this, we could reload the account every time we try to create a
connection, but it seems that the fresh credentials should just be
passed every time we create connections without the pool having to be
aware of potential changes under the rug.
This commit makes it so the pool no longer holds a reference to the
account, but rather it is passed in every time we check out a connection
Test Plan: manual. how did this even work before? :(
Reviewers: evan, halla, spang, mark
Reviewed By: mark
Differential Revision: https://phab.nylas.com/D4370
Summary:
Previously, the generic IMAP auth screen presented one security option to
users: "Require SSL". This was ambiguous and difficult to translate into
the correct security options behind the scenes, causing confusion and problems
connecting some accounts.
This patch does the following:
* Separates security settings for IMAP and SMTP, as these different protocols
may also require different SSL/TLS settings
* Reworks the generic IMAP auth page to allow specifying security settings
with higher fidelity. We looked at various different email apps and decided
that the best solution to this problem was to allow more detailed
specification of security settings and to ease the burden of more options
by having sane defaults that work correctly in the majority of cases.
This new screen allows users to pick from "SSL / TLS", "STARTTLS", or "none"
for the security settings for a protocol, and also to instruct us that
they're OK with us using known insecure SSL settings to connect to their
server by checking a checkbox.
We default to port 993 / SSL/TLS for IMAP and port 587 / STARTTLS for SMTP.
These are the most common settings for providers these days and will work
for most folks.
* Significantly tightens our default security. Now that we can allow folks to
opt-in to bad security, by default we should protect folks as best we can.
* Removes some now-unnecessary jank like specifying the SSLv3 "cipher"
in some custom SMTP configs. I don't think this was actually necessary
as SSLv3 is a protocol and not a valid cipher, but these custom
configs may have been necessary because of how the ssl_required flag was
linked between IMAP and SMTP before (and thus to specify different
settings for SMTP you'd have to override the SMTP config).
* Removes hard-coding of Gmail & Office365 settings in several
locations. (This was a major headache while working on the patch.)
This depends on version 2.0.1 of imap-provider-settings, which has major
breaking changes from version 1.0. See commit for more info:
9851054f91
Among other things, I did a serious audit of the settings in this file and
"upgraded" a few servers which weren't using the SSL-enabled ports for their
provider to the secure ones. Hurray for nmap and openssl.
Test Plan: manual
Reviewers: evan, mark, juan, halla
Reviewed By: juan, halla
Differential Revision: https://phab.nylas.com/D4316
Summary: It needs to be wrapped in an object.
Test Plan: manual
Reviewers: evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4367
Summary:
Make sure SyncActivity.getLastSyncActivityForAccount returns a sensible
default to prevent runtime errors when the SyncProcessManager uses it
Test Plan: manual
Reviewers: evan, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4366
Summary:
Previously, calling the IMAPConnectionPool `onDone` callback when checking out a connection would just return the connection to the pool without re-establishing it the next time we checked it out of the pool.
When the `onDone` callback was called in an error scenario, this had the unintended consequence of returning a bad connection to the pool, and then re-using this bad connection. The only scenario when the connection was re-established was if `onConnected` threw an error, but if the connection was kept open outside the scope of onConnected, this logic would never run.
To make things simpler and more consistent, `onDone` will always destroy all connections and connections will always be re-established from scratch every time they are checked out of the pool. The pool acts more as a limit to the number of connections per account, than an actual shared pool of connections.
Test Plan: manual
Reviewers: evan, spang, halla, mark
Reviewed By: halla, mark
Differential Revision: https://phab.nylas.com/D4360
Summary:
When the sync worker got stuck inside `performSync`, we would never
release the connection back to the connection pool. We would then try to
start a new sync worker, and it would claim a new connection. If this
happened enough times, we would eventually exhaust the whole pool and
keep the sync worker blocked forever.
Test Plan: manual
Reviewers: evan, spang, halla, mark
Reviewed By: halla, mark
Differential Revision: https://phab.nylas.com/D4359
Summary:
This removes the old SignalFX reporter that we weren't using
Adding documentation around logging
Test Plan: manual
Reviewers: juan, halla, spang
Reviewed By: juan, halla, spang
Differential Revision: https://phab.nylas.com/D4341
Summary:
According to bunyan, https://github.com/trentm/node-bunyan, errors are
treated special. They either need to be the first argument of a log, or
they need to be attached to the special `err` (NOT `error`) key of the
logger object.
Test Plan: manual
Reviewers: halla, juan, spang
Reviewed By: halla, juan, spang
Differential Revision: https://phab.nylas.com/D4337
Summary:
The unapply transformation was incorrectly passing the props parameter
to buildAnchorTag() as a string, even though it expects an object.
buildAnchorTag() goes on to stringify this parameter, which causes extra
surrounding quotations and escape slashes in front of all the other
quotations. The unapply transform is applied on several re-renders,
which causes the number of escape characters to unboundedly increase.
Firstly, this was causing inline images to not appear properly in the
draft, and secondly, the large number of escape characters was making
the draft body large enough to make the app unresponsive.
This generally wasn't an issue because the unapply transformation is
only applied when there has been an apply transformation, and this is
triggered by the SyncbackDraftTask. This task was previously only queued
upon send, in which case the user never re-opens the draft. Now,
enabling send-later on a draft will queue the task, and these issues
appear if the user re-opens the draft for editing.
This diff makes it so that the unapply transform passes in the props as
an object, like the buildAnchorTag function expects.
Test Plan: manual
Reviewers: evan, juan
Reviewed By: evan, juan
Differential Revision: https://phab.nylas.com/D4348
Summary:
Remove packages from the disabled package list when they have been
migrated to be enabled by default.
Also, the last migration would not have worked properly anyways because
the daily channel was already on 1.0.56, and we check for greater-than
rather than equal-to. Bump that version to match the next update.
Test Plan: manual
Reviewers: juan, evan
Reviewed By: juan, evan
Differential Revision: https://phab.nylas.com/D4335
Summary:
Previously, is you signed out of your NylasID, you would not pick up new
updates because we never updated the autoupdater url to use the new id.
Test Plan: manual
Reviewers: mark, halla, evan
Reviewed By: halla, evan
Differential Revision: https://phab.nylas.com/D4346
Summary:
Sometimes, when logging out of your NylasID and restarting the app, we
would continue making some requests from the worker window that required a
NylasID. This would make the app enter a restart loop and become
completely unresponsive because when we made a request without a
NylasID, we would force the user to log out and restart the app, and
then we would again make the requests without the id, ad infinitum.
To fix this, we make sure we have a NylasID before making any requests
that require it
Test Plan: manual
Reviewers: halla, evan
Reviewed By: halla, evan
Differential Revision: https://phab.nylas.com/D4344
Summary:
Before this commit, we would just establish a single mail listener imap
connection and never check if we needed to re-connect it due to an
expired access token. Even though we correctly refreshed the access
token at the beggining of each sync loop (hidden under
`ensureSMTPConnection`), we would never re-establish the mail listener
connection with the new access token.
This would cause the app to enter an `Invalid Credentials` error loop in
the sync loop, preventing from syncing any mail at all (T8064)
Test Plan: manual
Reviewers: spang, halla, mark, evan
Reviewed By: mark, evan
Differential Revision: https://phab.nylas.com/D4338
Summary:
Given that we keep the imap mail listener conenction open outside of the
imap connection pool, it might close itself after a while or after
putting your computer to sleep.
Previously, if we had the connection object we just assumed it was
connected and proceeded with sync, and if it wasn't, we would fall into
an error loop in the sync worker preventing it from syncing at all (T8065)
To fix this, we just call `connect` every time to ensure the connection is open.
(This was the case before we introduced the connection pool)
Test Plan: manual
Reviewers: mark, spang, halla, evan
Reviewed By: halla, evan
Differential Revision: https://phab.nylas.com/D4334
Summary:
Previously we would always search all mail. Now, if the user has focused
a particular folder we will limit our search to that folder. The inbox
is an exception--it will always search all mail unless the user
explicitly uses an "in:" clause.
Test Plan: Run locally, verify that searching folders returns the correct results.
Reviewers: evan, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4328
Summary:
Check that there aren't any other categories with a role before we try
to assign it to the current category.
Addresses T7835
Test Plan: manual
Reviewers: juan, evan, mark
Reviewed By: mark
Differential Revision: https://phab.nylas.com/D4298
Summary:
Before this patch, the IdentityStore would initialize in our empty hot
window. However, hot windows don't receive any `action-bridge-message`s,
which include DB updates. Since the hot window loads first, it was with a
stale verison of the Identity. The main window fetches a fresh identity,
but that fresh update failed to get to new composers because the hot
window wasn't listening to changes to the DB.
This makes it such that the IdentityStore properly boots up when the
window props change.
Test Plan: manual
Reviewers: halla, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4327
Summary:
This adds feature limit modals (graphics pending) for reminders and send
later
Test Plan: manual
Reviewers: juan, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4322
Summary: This shaves off ~150ms trying to issue this query
Test Plan: manual
Reviewers: evan, spang, mark
Reviewed By: mark
Differential Revision: https://phab.nylas.com/D4317
Summary:
Bodies that already exist on K2 shouldn't be overridden.
See the comment for how this caused open/link tracking to fail in certain
cases when a lot of messages were being sent
Test Plan: manual
Reviewers: halla, mark, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4319
Summary:
We were using a join for the contact ranking query and for whatever
reason on large databases this was extremely slow in SQLite. This diff
splits the query into first finding the sent Folder/Label and then
searching for non-draft Messages in that category.
Test Plan: Run locally, verify the query is faster
Reviewers: juan, evan, spang
Reviewed By: spang
Differential Revision: https://phab.nylas.com/D4315
Summary: It was really slow, and we no longer need it.
Test Plan: Run locally, verify that we no longer do that super slow query
Reviewers: evan, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4312
Summary:
Removes some VERY long running `ANLAYZE` queries. Was taking up to 50
seconds on my 9GB database on every boot
https://sqlite.org/pragma.html#pragma_optimize
Test Plan:
I tested to make sure the app still quits quickly. It does. The SQLite
docs also say this should be fast.
Reviewers: halla, spang, mark, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4314
Summary:
We were using a couple of left outer joins to avoid checking whether we
should be joining on Labels or Folders. We can greatly simplify the query
by just checking which we should use and issuing the correct inner join.
Test Plan: Run locally, verify contact ranking still works.
Reviewers: spang, juan, evan
Reviewed By: juan, evan
Differential Revision: https://phab.nylas.com/D4310
Summary:
This improves window open times by about 70ms
We would spend a very long time in the backend browser process building a
new hot window when we didn't need to do that until much later
Test Plan: manual
Reviewers: halla, mark, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4311
Summary:
Most of the times when removing a sync worker (i.e. when removing an
account), we would see database errors in the console. This happened
because more often than not we would interrupt in the middle of message
processing, but `processMessage` is not interruptible, which means that
the worker would be interrupted right after processing its current
message. However, if `processMessage` would take more than 500ms (the
current timeout for stopping the worker), we would destroy the database
before processing was done and it would throw a bunch of errors.
To fix this, we just don't set a timeout when removing the worker as a consequence
of removing an account. However, when we are removing the worker when we
detect that it is stuck, we set a time out of 5 seconds.
Test Plan: manually test removing accounts, verify that it doesn't error
Reviewers: mark, halla, evan
Reviewed By: halla, evan
Differential Revision: https://phab.nylas.com/D4308
Summary:
Previously, it was possible for the sync worker to continue running after being
interrupted, e.g. it would break out of `performSync` and then try to run
`onSyncCompleted`. This is fine if we were just interrupting to restart the loop,
but when we stop it we don't want it to continue running anything at all.
This also refactors the syncworker to have a single `destroy` method, which sets
a `destroyed` flag and uses that one exclusively instead of the `stopped` flag.
Test Plan: manually check it works
Reviewers: spang, halla, mark, evan
Reviewed By: mark, evan
Differential Revision: https://phab.nylas.com/D4307
Summary:
This prevents us from hitting the endpoint before the database for that
account is initialized and it's actually syncing.
Before this commit, we would throw errors upon adding an account
Test Plan: manually add, remove accounts, run benchmark script
Reviewers: mark, halla, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4305
Summary:
- Change files to .es6 so they recognize the syntax
- Pass arguments down to _run()
- Make sure the responses get returned
Test Plan: manual, even though that somehow failed before. :(
Reviewers: evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4309
Summary:
Creates a new `DatabaseStore.write` method alongside
`DatabaseStore.inTransaction` that doesn't always wrap trivial queries in
a transaction. SQLite does this automatically.
Test Plan: manual
Reviewers: spang, mark, halla, juan
Reviewed By: halla, juan
Differential Revision: https://phab.nylas.com/D4303
Summary: Renamed in prep for next few diffs
Test Plan: manual
Reviewers: juan, mark, spang, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4302
Summary:
Use the wonderful `debug` library instead of all our random flags.
You can now do things like: `DEBUG="app:*,sync:*" npm start` to print out
everything.
Test Plan: manual
Reviewers: mark, halla, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4297
Summary:
Report how long they take and if they're stopped (due to timeout).
Part of T7978
Test Plan: manual
Reviewers: evan, juan
Reviewed By: evan, juan
Differential Revision: https://phab.nylas.com/D4272
Summary: See title
Test Plan: Run locally, verify that attachments are returned
Reviewers: spang, evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4293