Commit graph

127 commits

Author SHA1 Message Date
Mark Hahnenberg 0508180712 [client-sync] Don't throttle while syncing first 500 threads
Summary:
We want to get to a usable inbox as quickly as possible, and throttling
prevents this. We should basically only be throttling for historical
mail syncing.

Test Plan: Run initial sync benchmark, it's 117% faster on battery

Reviewers: evan, juan, spang

Reviewed By: juan, spang

Differential Revision: https://phab.nylas.com/D4266
2017-03-28 10:52:34 -07:00
Mark Hahnenberg 60e6113f87 [client-sync] Implement the /contacts/rankings endpoint
Summary:
Prior to Nylas Mail, the Nylas Cloud API provided an endpoint that
returned rankings for contacts which it computed based on how frequently
and how recently a user sent mail to a recipient. This diff reimplements
that functionality in Nylas Mail. This should improve contact
auto-complete when composing emails to frequently contacted recipients.

Test Plan: Run locally, verify that frequent contacts are suggested earlier

Reviewers: spang, evan, juan

Reviewed By: evan, juan

Maniphest Tasks: T7948

Differential Revision: https://phab.nylas.com/D4253
2017-03-28 10:51:24 -07:00
Juan Tejada ff9d49e752 [client-app] Fix runtime error in FetchMessagesInFolderTask
We were accessing `this._db.accountId` before `this._db` was set, which
caused an error when attempting to run the task
2017-03-28 09:15:03 -07:00
Juan Tejada caf5380659 [client-app] Fix importing SyncActivity
In some places we were not accessing `.default` when using `require`,
and when using `import` we couldn't really destructure unless we
accessed `.default` too, or if the functions were regular exports
instead of properties on a default exported object.

This was causing our sync loop to error.

To remain consistent, we always just `require` or `import` the
SyncActivity singleton and access that
2017-03-28 09:09:39 -07:00
Halla Moore adff6896e6 [client-sync] Change where we report sync activity
Summary:
These locations will provide enough activity for monitoring and are also
more helpful in helping us debug what the sync loop was doing when it
got stuck.

Test Plan: manual

Reviewers: juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4274
2017-03-27 21:49:56 -07:00
Halla Moore 4ad4596e2f [client-app, client-sync] Add specs for detecting stuck sync/worker window
Summary: See title. Part of T7681.

Test Plan: ran the specs

Reviewers: evan, mark, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4264
2017-03-27 15:45:33 -07:00
Halla Moore 619c69a522 [client-sync] Detect when sync workers are stuck
Summary:
Periodically check the latest sync activity times to determine if a sync
worker has been inactive for too long. If it has been too long, discard
the worker and create a new one for that account. Part of T7681.

Test Plan: manual, specs

Reviewers: evan, mark, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4262
2017-03-27 15:41:31 -07:00
Halla Moore b6887e386e [client-sync] Report latest sync activity
Summary:
Add infrastructure to report and retrieve the latest sync activity, and
start reporting when we download a message or detect that a folder
has no new messages. This will be used to detect if the sync loop is
stuck. Part of T7681.

Test Plan: manual, specs

Reviewers: evan, mark, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4261
2017-03-27 15:39:53 -07:00
Juan Tejada ae1c667e37 [client-app] Fix in identity store 2017-03-24 14:04:44 -07:00
Juan Tejada 41ad424752 [client-app] Don't try to restart sync on every IdentityStore change
Summary:
The IdentityStore can trigger any number of times, but we only want to
start sync if we previously didn't have an identity available

Test Plan: manual

Reviewers: spang, evan, halla

Reviewed By: evan, halla

Differential Revision: https://phab.nylas.com/D4246
2017-03-24 13:06:47 -07:00
Mark Hahnenberg 4ef8e7614e [client-sync] Don't handle IMAP timeouts in the connection pool
Summary:
Different clients can have different policies for retrying after
timeouts.

Test Plan: Run locally, run tests

Reviewers: evan, spang, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4247
2017-03-23 11:33:53 -07:00
Mark Hahnenberg 2fe8bee38f [client-sync] Refactor sync worker IMAPConnectionPool callbacks
Summary: In preparation for removing timeout handling from the IMAPConnectionPool.

Test Plan: Run locally

Reviewers: spang, evan, juan

Reviewed By: evan, juan

Differential Revision: https://phab.nylas.com/D4245
2017-03-21 12:15:40 -07:00
Juan Tejada 3554fb2510 [client-app] Fix passing cursor to delta streams
Summary:
Currently, when we auth an account for the first time in Nylas Mail (or
we blow away the database), the app is going to request transactions
since cursor `null` from the /delta/streaming endpoint and from the local-sync
delta observable, instead of requesting transactions since cursor `0`

This is due to a subtle bug with the use of default values when
destructuring an object. Our coded did the following:
```
const {cursor = 0} = this._state
```
Which at a glance seems correct. However, this will only work as
expected if `this._state` has the following shape:
```
{cursor: undefined}
```
And unfortunately, our `this._state` looked like this when authing an
account for the first time:
```
{cursor: null}
```

Which would make `cursor === null` instead of `0`.
This is because when using default values, null is considered an
intentional argument/value, as opposed to not passing any argument/value
(which will mean that the argument is undefined).

This was a regression introduced in d60a23c and 8bc2ec5

Test Plan: manual, will add regression test in upcoming diff

Reviewers: evan, spang, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D4243
2017-03-20 16:16:46 -07:00
Juan Tejada 6532d3c647 [client-sync] Add error handling when creating syncback requests
Summary: see title. also convert to es6

Test Plan: manual

Reviewers: evan, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D4225
2017-03-17 13:59:37 -07:00
Halla Moore 4492b73507 [client-sync] Only update lastReceivedDate if the message was actually received
Summary:
We don't want to bump threads to the top of the inbox when a user sends a
reply. We originally used `!isSent` to prevent this, but that was removed in
a diff that made sure messages showed up in the inbox when users send emails
to themselves. In order to implement both of these cases properly, this diff
introduces `isReceived` and uses that to determine whether lastReceivedDate
should be updated. Addresses T7991.

Also changes the order of some `or` statements, so that we actually check that
the variable exists before comparing against it.

Test Plan: manual

Reviewers: evan, juan, spang

Reviewed By: spang

Differential Revision: https://phab.nylas.com/D4226
2017-03-16 12:25:56 -07:00
Juan Tejada 7fc1e043c1 [client-app] Restart sync when computer awakes from sleep
Summary:
Use electron's `powerMonitor` module to detect when the computer resumes
from sleep, and restart the sync loop when that happens in order to
sync the inbox immediately, in case we received any new mail events
while the computer was asleep

Test Plan: manual

Reviewers: evan, spang, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D4216
2017-03-13 22:44:09 -07:00
Juan Tejada f157ddb867 [client-app] Make resetEmailCache work without forcing re sign-in
Summary:
This commit makes it so `resetEmailCache` works as expected, i.e. it
removes all databases, without forcing the user to re sign-in to their
accounts or NylasID

Previously, this method removed the database without removing the
accounts, left users in an un-authed state that was hard to recover
from. This was fixed in D4212 which makes sure that when we get a new
identity, sync and deltas are restarted

However, resetEmailCache would still force you to log in to yoru NylasID
because it was deleted from the database. However, if we reuse the
command `application:relaunch-to-initial-windows` instead of manually
deleting the database, we can relaunc the app while preserving the users
NylasID session, so they don't have to sign back in manually.

Test Plan: manual

Reviewers: evan, spang, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D4215
2017-03-13 22:42:52 -07:00
Juan Tejada 4d1628b914 [client-app] Don't start sync or delta connections without an identity
Summary:
This prevents the app from entering a restart loop when there's no
identity.

Specifically, when a user logs out of their identity, or when she resets the
email cache, or any other scenario that leaves the app without an
identity but with accounts added, the sync loop (and deltas) will start
without an Identity.

This will cause NylasAPIRequest to throw an error that
forces the user to close the app. When the app restarts, sync will start
again without an identity, and the user will be forced to close the app
again, and so on and so forth for the rest of eternity

Relevant error:
https://github.com/nylas/nylas-mail-all/blob/master/packages/client-app/src/flux/nylas-api-request.es6#L165-L174

Additionaly, this makes sure that after resetting the email cache, the sync
process starts when the identity becomes available

This solves T7989

Test Plan: manual

Reviewers: evan, spang, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D4212
2017-03-13 20:00:59 -07:00
Juan Tejada 588163acda [client-sync] Correctly wait for sync to stop before resetting email cache
Summary:
Previously, while resetting the email cache, we would try to stop sync
and just wait for an arbitraty amount of time for it to stop and the
proceed to blow away the database.

This commit makes it so we correctly wait for sync to stop, and then we
blow away the db. It adds a timeout anyway in case sync is stuck and we
can't stop it

Depends on D4207

Test Plan: manual

Reviewers: evan, spang, halla

Reviewed By: spang, halla

Differential Revision: https://phab.nylas.com/D4209
2017-03-13 19:57:58 -07:00
Juan Tejada ebe16aa1ae [client-sync]: Fix error in SyncProcessManager._resetEmailCache
Summary:
We no longer keep `this._accounts` state, which was being accessed
inside `_resetEmailCache`

Test Plan: manual

Reviewers: evan, spang, halla

Reviewed By: spang, halla

Differential Revision: https://phab.nylas.com/D4207
2017-03-13 19:56:37 -07:00
Mark Hahnenberg 595adde687 [client-sync] Fix queueMessageForProcessing
Summary:
We were always incrementing the queue length and resolving the promise
in the constructor :-/

Test Plan: Run locally, make sure queue doesn't start rejecting messages

Reviewers: evan, spang, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4198
2017-03-10 17:39:35 -08:00
Mark Hahnenberg 792994d95c [client-sync] Refactor MessageProcessor to be more robust to errors
Summary:
Errors in the MessageProcessor were causing sync to get stuck
occasionally. This diff refactors queueMessageForProcessing and friends
so that they're more robust to errors during promise construction.

Test Plan: Run locally

Reviewers: juan, spang, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D4190
2017-03-10 13:40:04 -08:00
Mark Hahnenberg c971ed03e2 [client-sync] Shim sequelize to timeout after 1 minute
Summary:
Sequelize can sometimes return promises that will never resolve or
reject. We can wrap the promises we get back from sequelize in a
bluebird promise which gives us the ability to timeout these abandoned
promises. This way, we can track down issues in sequelize as well as
unblocking stuck sync loops.

Test Plan: Run locally, verify that timeouts occur

Reviewers: juan, evan, spang

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D4192
2017-03-10 13:39:27 -08:00
Juan Tejada d24fc9a235 [client-app] More defensive error handling to prevent sync from halting
Summary:
This commit adds error handling to the sync-loop's `onSyncError` and
`scheduleNextSync`.

These functions generally don't fail, as they are in the `catch` and
`finally` blocks respectively of the sync loop. But as we've seen in
D4152, the datbase can sometime error if it's in a bad state. If it
errors inside these functions, we will never schedule the next run of
the sync loop.

Depends on D4152

Test Plan: manual

Reviewers: evan, spang, mark, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D4153
2017-03-09 21:22:07 -08:00
Juan Tejada da463c250f [client-app] Consolidate delta connection stores, rm deltas internal_pkg
Summary:
This commit consolidates the `DeltaConnectionStatusStore` and the
`DeltaConnectionStore` which kept track of very similar state and made
sense to be the same store (as per feedback in D4118#77647)

Given that this state needs to be available app-wide for plugins to
query the status of delta connections, `internal_packages/deltas` was
removed (given that it only activated that store), in favor of having the
unified store inside `src/flux/stores` and available via `nylas-exports`

The `deltas` package also contained some contacts-ranking code, which is
no longer in use until we restore that fetaure, so I created a
`internal_packages/contact-rankings` which contains this unused code for
now.

Test Plan:
manually open, close, end delta connections, verify that I'm getting
correct results. unit tests to come

Reviewers: halla, spang, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D4145
2017-03-09 15:27:06 -08:00
Juan Tejada 20a05d3340 [client-app] Rate limit error reporting for message processing errors
Summary:
This should prevent us from flooding Sentry with errors like this one:
https://sentry.io/nylas/nylas-mail/issues/230605150/

Test Plan: manual

Reviewers: spang, mark, halla, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D4151
2017-03-09 15:22:08 -08:00
Juan Tejada 521ca86efc [client-app] Prevent database malformed error restart loop
Summary:
This error is can flood Sentry pretty badly: https://sentry.io/nylas/nylas-mail/issues/230629801/

My initial thought was to rate limit it, but rate limiting wouldn't
do any good because when we get that error we destroy the databases and
restart the app, and we will loose the in-memory rate limiting data.

The real fix, and reason why Sentry is being flooded with this error by
a single user, is that once you encounter this error the app will enter
a restart loop that constantly throws this error. The reason being that
we weren't properly awaiting for the K2 account databases to be dropped
before closing the app, so on restart, the database would still be
malformed.

The fix is to properly `await` for the database drops

Test Plan: manual

Reviewers: spang, mark, halla, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D4127
2017-03-09 15:21:07 -08:00
Mark Hahnenberg 6a17073fe5 [client-sync] Refactor file download IMAP pool use
Summary:
This is in preparation for removing the ability of the IMAP pool to
automatically handle timeout errors.

Test Plan: Run locally, verify file download still works

Reviewers: juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4167
2017-03-09 14:47:07 -08:00
Mark Hahnenberg 19528ca78a [client-sync] Refactor raw message IMAP connection pool use
Summary: In preparation for removing timeout handling from IMAPConnectionPool.

Test Plan: Run locally

Reviewers: juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4168
2017-03-09 14:46:51 -08:00
Mark Hahnenberg acd3bf5954 [client-sync] Refactor _search Observable
Summary:
We had this weird nesting where we would keep the IMAPConnection open
beyond the scope of the connection pool and return the created
Observable. This diff inverts that relationship and flattens out some of
the indentation, making the code easier to read. This is in preparation
for refactoring the IMAPConnectionPool to no longer automatically handle
timeouts.

Test Plan: Run locally, make sure search still works

Reviewers: juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4165
2017-03-09 14:26:06 -08:00
Mark Hahnenberg 0ccb2b51ad [client-sync] Make cancelling SyncUnknownUIDs actually work
Summary: Misspelled INPROGRESS-NOTRETRYABLE :-(

Test Plan: Run locally, make sure things get cancelled

Reviewers: juan

Differential Revision: https://phab.nylas.com/D4163
2017-03-09 12:25:54 -08:00
Mark Hahnenberg 47f57a6c02 [client-sync] Gracefully handle large amounts of unknown UIDs from search
Summary:
Previously if we got back a huge number of unknown UIDs from IMAP search
we would try to sync all of them at once. This could lead to hanging the sync
loop trying to download tons of messages. This diff limits the UIDs we're
willing to sync per task to 500 and splits each task up into chunks of 25
messages so that we don't try to download all of them at once. If we need
to sync more than 500 uids then at the end of the syncback task it will
queue another task to run the next time the sync loop rolls back around.

Test Plan:
Run locally, verify that we gracefully handle various situations
including cancelling during the syncback task, cancelling between syncback
tasks, huge numbers of results, etc

Reviewers: evan, spang, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4142
2017-03-09 11:47:18 -08:00
Mark Hahnenberg 1c1459f306 [client-sync] Make SyncUnknownUIDs NOTRETRYABLE
Summary:
We don't want to constantly retry to sync a bunch of random UIDs if
we're not longer in the search perspective so make these syncback tasks
not retryable.

Test Plan: Run locally, verify that we don't retry

Reviewers: spang, evan, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4149
2017-03-09 11:07:38 -08:00
Mark Hahnenberg e441632293 [client-sync] Refactor message processing throttling
Summary:
We want to enable code to control whether fetching is throttled or not.
We basically only want to throttle syncing the historical message
archive. New messages, initial inbox sync, and unknown search results
should not be throttled. Also some drive-by code refactoring.

Test Plan: Run locally, verify that things still work

Reviewers: spang, evan, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4126
2017-03-08 14:18:44 -08:00
Juan Tejada 12cbd383bf [client-app] (deltas P6) Split local and cloud deltas
Summary:
This commit splits apart the `AccountDeltaConnection` class, which was
in charge of listening to both cloud /and/ local deltas by way of an
artificial interface, `DeltaStreamingInMemoryConnection`.

Splitting this into 2 modules with separate responsibilities will hopefully
make this code easier to reason about and reduce some cruft and unnecessary
indirection.

Specifically, this commit makes it so:

- `DeltaConnectionManager` is only in charge of starting and ending `DeltaStreamingConnection`s, which are solely in charge of listening to deltas from the cloud api
- `LocalSyncDeltaEmitter` no longer unnecessarily emits events for the `deltas` package to listen to but rather directly processes and saves those deltas from the K2 db to edgehill.db
- `LocalSyncDeltaEmitter` is also in charge of keeping track of the latest received cursor, under its own JSONBlob key in edgehill.db. This migrates localSync cursors saved under the old key.
- `LocalSyncDeltaEmitter` is now instantiated and managed from within the `SyncProcessManager` as opposed to the `SyncWorker`. Apart from removing extra state from the `SyncWorker`, this removes dependencies on the client-app environment from the sync-worker.
- `DeltaStreamingInMemoryConnection` and `AccountDeltaConnection` are now gone

(Sorry for the big diff! This one was a little hard to split up without landing something broken)

Depends on D4121

Test Plan: manual + unit tests planned in upcoming diff

Reviewers: halla, mark, evan, spang

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D4122
2017-03-08 12:12:50 -08:00
Karim Hamidou b1ba489065 Revert "Revert "[feat] Add support for send later""
Arc land messed up and landed a not fully merged branch. (Seriously – I
had merged a copy of my branch with master to see how easy it would be.
Because I didn't want to merge the whole thing, I blindly committed my
changes and switched back to my real branch). To my great surprise, arc
decided to use the wrong branch when landing it.

Original commit message:

Summary:
    Finally, here it is! Send later, with support for open tracking but
without support for attachments yet. It took me some time to find the
right way to do things.

    **The send later dilemna**

    There's two ways we could handle send later:
    1. do everything on the client
    2. process the message in the cloud

    1. is very tempting because it would make the cloud server very
simple. Unfortunately, it has some serious limitations, for example,
setting the "Date" message header. That's why I chose to go with 2. When
a user presses the "Send Later" button, we save the open/link tracking
metadata and fills in all the required fields. I added a custom endpoint
to the K2 API to do this, `/drafts/build`. After that, we save the JSON
contents of the message as metadata.

    When we process metadata, we simply create a MIME message from the
JSON and send it.

    **Limitations**

    Right now, send later doesn't support and attachments. There's also
some minor code duplication which needs to be refactored away.

Test Plan: Tested manually. Checked that regular send still worked, too.

Reviewers: mark, spang, halla, juan, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D4054
2017-03-07 17:21:29 -08:00
Karim Hamidou 2f67d8ac8b Revert "[feat] Add support for send later"
This reverts commit 683a550d49.
2017-03-07 16:18:25 -08:00
Karim Hamidou 683a550d49 [feat] Add support for send later
Summary:
Finally, here it is! Send later, with support for open tracking but without support for attachments yet. It took me some time to find the right way to do things.

**The send later dilemna**

There's two ways we could handle send later:
1. do everything on the client
2. process the message in the cloud

1. is very tempting because it would make the cloud server very simple. Unfortunately, it has some serious limitations, for example, setting the "Date" message header. That's why I chose to go with 2. When a user presses the "Send Later" button, we save the open/link tracking metadata and fills in all the required fields. I added a custom endpoint to the K2 API to do this, `/drafts/build`. After that, we save the JSON contents of the message as metadata.

When we process metadata, we simply create a MIME message from the JSON and send it.

**Limitations**

Right now, send later doesn't support and attachments. There's also some minor code duplication which needs to be refactored away.

Test Plan: Tested manually. Checked that regular send still worked, too.

Reviewers: mark, spang, halla, juan, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D4054
2017-03-07 16:06:30 -08:00
Mark Hahnenberg b119b43f82 [client-sync] Don't throttle message processing when syncing specific UIDs
Summary:
This diff adds a timeout parameter to queueMessageForProcessing that can
be used to override the CPU-throttled-by-default timeout used by the
message processing queue. The FetchMessagesInFolder sync task now
overrides with a timeout of 0 when it is queuing specific UIDs that it
has downloaded. The thinking here is that if we've requested a specific
set of UIDs, it's very important to sync those quickly as this indicates
a higher priority (e.g. initial inbox sync, search result syncing, etc).

Test Plan: Run locally, verify that initial sync and search syncing run faster

Reviewers: spang, evan, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4114
2017-03-07 11:34:19 -08:00
Mark Hahnenberg 280a5ba6e2 [client-sync] Fetch unknown search results
Summary:
When searching using IMAP/Gmail commands we sometimes get back UIDs for
messages that we have yet to sync. Previously we would just ignore these
results, which would decrease the quality search results for quite some
time during initial sync. This diff enables us to eagerly sync the unknown
messages we get back from the provider by creating a syncback task which
interrupts the sync loop and runs a sync task for the unknown UIDs.

Test Plan: Run locally, verify that we sync unknown messages

Reviewers: spang, evan, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4101
2017-03-07 11:30:58 -08:00
Juan Tejada 3bbb76902f [client-app] Add basic rate limiting to Sentry
Summary:
This commit adds a new option to the `reportError` interface to allow
to rate limit specific kinds of errors that are known to be reported a
lot if they fall in a loop for example.

For now, it only allows to impose a fixed reporting rate per hour for a specific
error, and will not report any errors past that rate before an hour has
passed.

Depends on D4099

Test Plan:
manually check that it reports and does not report errors based on the rate
limit data passed

Reviewers: khamidou, mark, evan, halla, spang

Reviewed By: evan, halla, spang

Differential Revision: https://phab.nylas.com/D4100
2017-03-06 10:56:20 -08:00
Juan Tejada 8eea5fa8ac [client-sync] Improve reporting of refresh access token errors
Summary:
Add better messaging and logging to the console.

Depends on D4098

Test Plan: manual

Reviewers: evan, khamidou, spang

Reviewed By: spang

Differential Revision: https://phab.nylas.com/D4099
2017-03-06 09:55:25 -08:00
Juan Tejada a041b79aaa [client-sync] Don't double report refresh access token API errors
Summary:
Given that NylasAPIRequest reports all APIErrors for requests that
return 5xx, we were double reporting access token errors, which caused 2
groups in sentry and makes it difficult to track.

This commit removes the extra reporting for that error in the sync loop

Test Plan: manual

Reviewers: evan, khamidou, spang

Reviewed By: spang

Differential Revision: https://phab.nylas.com/D4098
2017-03-06 09:54:51 -08:00
Juan Tejada 8bd686bc6e [client-sync] Sync accounts when app comes back online
Summary:
Given that we backoff exponentially in the sync loop when we encounter
RetryableErrors (e.g. network errors), if the app goes offline and
reaches the maximum retry backoff of 5 minutes, and if then the app comes
back online, in the worst case, the sync loop will idle for 5 minutes before trying to
sync, which is undesireable.

To mitigate this, everytime we come online we can wake all of the sync
workers. This shouldn't have any adverse effects

Test Plan: manual

Reviewers: evan, mark, spang

Reviewed By: spang

Differential Revision: https://phab.nylas.com/D4097
2017-03-06 09:50:47 -08:00
Juan Tejada 225613565a [client-sync] Use ExponentialBackoffScheduler for sync loop retries
Summary:
This is mostly to clean up the logic here a little bit.

Specifically:

- The backoff when encountering retryable errors is now truly exponential
- When encountering a permanent error, we back off for a minute
- If we don't encounter consecutive RetryableErrors, we clear the exponential backoff. Previously, we would just clear it when sync completed without errors.

Test Plan: manual

Reviewers: khamidou, spang, evan, mark

Reviewed By: mark

Differential Revision: https://phab.nylas.com/D4086
2017-03-05 23:17:20 -08:00
Mark Hahnenberg 8d057ca256 [client-sync] Refactor GmailSearchClient to use X-GM-RAW
Summary:
Previously we used the Gmail REST API to get search results. This API
returns the X-GM-MSGID which is different from the folder UID. Gmail also
offers the X-GM-RAW extension for IMAP's UID SEARCH command which allows
us to pass through the raw search query. This enables us to vastly
simplify the GmailSearchClient by having it subclass ImapSearchClient
and override a few select methods. Yay!

Test Plan: Run locally, verify that Gmail search queries still work

Reviewers: evan, spang, juan

Reviewed By: spang, juan

Differential Revision: https://phab.nylas.com/D4094
2017-03-02 17:27:07 -08:00
Mark Hahnenberg 9ec7b57f9a [client-app] Add proper IMAP search backend
Summary:
Previously for IMAP we just grabbed the search text and fed it into a TEXT
query. Now we have a proper backend that generates the appropriate
search criteria according to the IMAP spec. Important to note that we
don't support 'in:' yet, which is complicated due to the way that IMAP
search is scoped to the currently selected folder.

Test Plan: Run tests, run locally and verify IMAP search still works.

Reviewers: evan, juan, spang

Reviewed By: juan, spang

Differential Revision: https://phab.nylas.com/D4071
2017-03-02 14:50:35 -08:00
Juan Tejada df96678470 [client-sync] 🎨 lint errors 2017-03-01 15:48:16 -08:00
Mark Hahnenberg eebdc295ef [client-sync] Scale sync batch size based on folder SELECT duration
Summary:
If it's expensive for us to SELECT a folder (which happens a lot for
large gmail accounts), we should try to sync more messages so that we
don't waste a ton of time SELECT-ing during initial sync. This speeds up
initial sync quite a bit.

Test Plan: Run locally, verify batches are properly computed

Reviewers: evan, juan, spang

Reviewed By: juan, spang

Differential Revision: https://phab.nylas.com/D4050
2017-03-01 12:24:58 -08:00
Christine Spang 8c30697241 [client-sync] Enable logging in prod builds
Summary:
Disabling verbose logging in production builds just makes it harder to help our
users help us fix their problems.

Test Plan: run app without --dev

Reviewers: mark, evan, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4070
2017-03-01 08:11:00 -08:00
Christine Spang 5d440c6b93 [client-sync] Fix wording in comment 2017-02-28 12:12:39 -08:00
Christine Spang 6e1dabadbb [client-sync] Add check for syncWorker to SyncbackTaskRunner 2017-02-28 12:12:39 -08:00
Juan Tejada bb76da14cc [client-app] Fix EnsureMessageInFolderTask - no need to get acct from imap connection 2017-02-28 11:26:58 -08:00
Evan Morikawa 9fd2f097b0 [client-sync] add assertion about Reference not cleaned up properly 2017-02-28 11:09:17 -08:00
Juan Tejada de0e30d8dd Revert "[client-app] Limit search to focused perspective"
Temporarily reverting this commit for 1.0.30 release.
See https://phab.nylas.com/T7910 for details
This reverts commit da6bc473f8.
2017-02-28 11:07:44 -08:00
Juan Tejada de9484cc52 [client-sync] Fix missing pass in of syncWorker in EnsureMessageInSentFolder 2017-02-26 10:36:31 -08:00
Mark Hahnenberg 6bd6242398 [client-sync] Use IMAPConnectionPool in client sync worker
Summary: See title

Test Plan: Run locally, make sure sync still works

Reviewers: evan, spang, juan

Reviewed By: spang, juan

Differential Revision: https://phab.nylas.com/D3998
2017-02-24 13:09:32 -08:00
Christine Spang e991349708 [client-sync] Fix missing pass in of syncWorker to FetchNewMessagesInFolder 2017-02-24 12:49:37 -08:00
Evan Morikawa f23a96c481 [client-sync] limit attributes returned for messages
Summary:
I did a grep for `Message.find.*` to see if there were any other places
we could limit the attributes returned

There were.

This used to be INSANELY wasteful to return all bodies when we only needed
the ids.

Test Plan: Manually bootup app and ensure search still works

Reviewers: mark, juan, spang

Reviewed By: mark, juan, spang

Differential Revision: https://phab.nylas.com/D4042
2017-02-24 11:04:18 -08:00
Evan Morikawa c7bdb5804a [client-sync] fix unbouded query
Summary:
Fixes T7783

This unbounded query, plus a yet-to-be-diagnosed bug causing large numbers
of messages with no folderIds, led to frequent crashes of the worker
window at the end of every sync loop.

This took a fair amount to diagnose. Notes on that are here:
https://paper.dropbox.com/doc/Diagnosing-Crash-X329VTxevrqtIMESBtHNV

Huge thanks to @mark.

Test Plan: Reboot app with patch, no more crashes!

Reviewers: spang, halla, mark, juan

Reviewed By: mark, juan

Subscribers: mark

Maniphest Tasks: T7783

Differential Revision: https://phab.nylas.com/D4039
2017-02-24 10:58:21 -08:00
Christine Spang 15eac9c9db [client-sync] Parse comma-separated References properly
This goes against the spec, but we've seen emails which use comma-separated
References instead of space-separated references. This could cause threading
to break if the message with comma-separated references was a reply and
we hadn't yet synced the message directly in the In-Reply-To header.
2017-02-23 18:58:13 -08:00
Christine Spang 49103367c1 [client-sync] Update comment about Gmail inbox prioritization 2017-02-23 15:36:35 -08:00
Christine Spang 4a1dde8830 [client-sync] Deduplicate UID downloads across sync loop interruptions
Summary:
Since we only persist updates to fetchedmin/fetchedmax at the end of a
batch, and a batch can contain many messages, if the sync loop is
getting interrupted often we will download the same messages over and
over again and not make much progress in downloading the message
backlog. This patch keeps a set of already downloaded messages in memory
for each batch and skips downloading UIDs we've processed in interrupted
sync loops.

Messages may still be redownloaded across app restarts.

Fixes T7798

Test Plan: manual

Reviewers: juan, mark

Reviewed By: juan, mark

Maniphest Tasks: T7798

Differential Revision: https://phab.nylas.com/D4040
2017-02-23 15:23:28 -08:00
Mark Hahnenberg da6bc473f8 [client-app] Limit search to focused perspective
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/D4032
2017-02-23 13:36:45 -08:00
Juan Tejada dced0aa99e [client-sync] 🎨 Remove unecessary use of PromiseUtils
Given that this packages lives inside the NylasMail app, it has access
to Bluebird Promises, which already define a .each method.

Using PromiseUtils here is unecessary
2017-02-22 18:30:34 -08:00
Mark Hahnenberg c77c8becb9 [client-sync] Use IMAPConnectionPool for IMAP search client
Summary: See title

Test Plan: Run locally, verify IMAP search still works

Reviewers: evan, spang, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4006
2017-02-22 10:57:42 -08:00
Mark Hahnenberg a912dc971c [client-sync] Use IMAPConnectionPool for downloading raw messages
Summary: See title

Test Plan: Run locally, verify we can download raw messages

Reviewers: evan, spang, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4005
2017-02-22 10:55:08 -08:00
Mark Hahnenberg c4ccd8aac4 [client-sync] Use IMAPConnectionPool for downloading files
Summary: See title

Test Plan: Run locally, verify downloading files works

Reviewers: evan, spang, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D4004
2017-02-22 10:48:26 -08:00
Christine Spang 84a3b20839 [client-sync] Refresh SMTP client when auth credentials change
Summary:
Previously, we would create a nodemailer SMTP transport object when the
sync worker booted up. The transport object would be passed the account
SMTP credentials at the time of object creation. If the Google auth
token later expired, we would continue to try to send mail using the
expired token, resulting in "Invalid login" failures.

This patch makes it so we refresh the transport object if the auth token
changes, and also turns on SMTP connection pooling to limit simultaneous
SMTP connections (& maybe make sending multiple messages faster).

Fixes T7891

Test Plan: manual

Reviewers: juan, halla

Reviewed By: juan, halla

Subscribers: mark

Maniphest Tasks: T7891

Differential Revision: https://phab.nylas.com/D3997
2017-02-21 16:24:24 -08:00
Juan Tejada a29dfcf56c [client-sync] Fix sending per recipient
Summary: This was broken, oh so broken

Test Plan: manual

Reviewers: halla, spang, evan, mark

Reviewed By: mark

Differential Revision: https://phab.nylas.com/D4001
2017-02-21 15:24:21 -08:00
Juan Tejada 9f78574c3d [*] metrics(Part 6) MetricsReporter.reportEvent now requires a nylasId
Summary:
This will help us aggregate metrics by user. This also makes it so we
don't report events in dev mode

Test Plan: manual

Reviewers: spang, evan

Reviewed By: spang, evan

Differential Revision: https://phab.nylas.com/D3981
2017-02-21 11:48:45 -08:00
Juan Tejada 301766722a [local-sync] metrics(Part 2) Move MetricsReporter to isomorphic-core
Summary:
Renamed it from SyncMetricsReporter to MetricsReporter and moved it to
iso-core.

The new metrics reporter can now be called from any environment and will
correctly report the metrics.

Test Plan: manual

Reviewers: mark, spang, evan

Reviewed By: spang, evan

Differential Revision: https://phab.nylas.com/D3967
2017-02-21 11:44:21 -08:00
Evan Morikawa 27c08d86e7 [*] update and add READMEs to each package
Summary:
Adding READMEs for easy and helpful browsing on GitHub.
Also add missing script and `--interpreter` flag

Test Plan: Run new launch commands

Reviewers: mark, spang, juan, halla

Reviewed By: spang, juan, halla

Differential Revision: https://phab.nylas.com/D3971
2017-02-17 17:28:09 -08:00
Halla Moore 57ff111925 [client-app, client-sync] Save imap folder names in the client-app
Summary:
Client-sync has the full imap folder names, but used to only pass the display
name to the application. The application needs the full imap names so that it
can pass them via metadata to cloud-workers that need to open imap boxes.

Test Plan: manual

Reviewers: evan, juan

Reviewed By: evan, juan

Subscribers: juan

Differential Revision: https://phab.nylas.com/D3951
2017-02-17 14:47:10 -08:00
Christine Spang faf44193a7 [local-sync] Differentiate sync loop & other errors by additional fingerprint info
Summary:
If an exception has the same stack trace, by default Sentry will always group
it together in the same event. We don't want to do that for sync loop
errors---e.g. 'Invalid credentials' errors should not be grouped together with
stuff like 'Too many simultaneous connections'. Creating more unique groups
will allow us to better evaluate the effect of sync & other bugfixes.

Test Plan: writing unit test right now

Reviewers: juan, mark

Subscribers:

Differential Revision: https://phab.nylas.com/D3915
2017-02-17 14:03:24 -08:00
Evan Morikawa 918fa0b6dd [*] move to monorepo
[*] update babel

[client-app] remove flow-typed

[client-app] Move build/package.json to main package.json

[client-app] remove spec_integration

[client-app] fix babel support

Add client-private-plugins package.json

[client-app] add node_modules to global path for private-plugins

Move client-sync dependencies to client-app root

fix electron rebuild

[*] moved to monorepo

Summary: App now runs in monorepo

Test Plan: npm test

Reviewers: juan, mark, khamidou, halla, spang

Differential Revision: https://phab.nylas.com/D3947
2017-02-16 18:46:26 -08:00
Evan Morikawa 9407533f51 [client-*] Rename packages folders and update readme 2017-02-16 13:31:37 -08:00