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:
We weren't changing into the directory of the git repository, so running
the script from anywhere but inside the git repo didn't work. We also
weren't pulling before checking for the latest commit. Now we do both of
these things.
Test Plan: Run locally
Reviewers: spang, juan, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4339
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:
We created the process so we can also kill it with no problems. This
makes it easier to automatically run benchmarks without human
intervention.
Test Plan: Run locally
Reviewers: spang, evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4332
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: See title
Test Plan: Run locally on older version of clang
Reviewers: juan, spang, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4329
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
