Commit graph

845 commits

Author SHA1 Message Date
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
Christine Spang ee5abf22eb [iso-core] Fix Office365 sending from the mail app
Summary:
This fixes a regression introduced in D3980 that prevented sending from working
on Office365 and generic IMAP accounts.

Fixes T7892

Test Plan: manual - we could use unit tests for this but need to set up tests for iso-core first

Reviewers: juan, halla, evan

Reviewed By: halla, evan

Maniphest Tasks: T7892

Differential Revision: https://phab.nylas.com/D4003
2017-02-21 15:59:12 -08:00
Halla Moore 1105b2621f [*] Rename postinstall.js -> postinstall.es6
The n1Cloud machines try to compile this script, even though they
never run it, and they didn't recognize it was supposed to be es6
2017-02-21 15:55:08 -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 1722e8bb39 [*] Catch more invalid login errors when sending
Summary: See title

Test Plan: manual

Reviewers: evan, halla, spang

Reviewed By: halla, spang

Differential Revision: https://phab.nylas.com/D3994
2017-02-21 14:58:31 -08:00
Evan Morikawa 3a33b0ad64 [client-app] update build system 2017-02-21 17:32:22 -05:00
Mark Hahnenberg c634380ab6 [client-sync] Add per-Account IMAP connection pooling
Summary:
Prior to this diff it was easy for us to create too many IMAP connections (e.g.
by requesting many attachments at once), causing random failures when the
server would reject our connection attempts. This diff adds a per-Account IMAP
pooling mechanism so that we avoid these failures.

Test Plan:
Run locally with sync worker and several other clients using the
pool, verify correct behavior. Also added a few unit tests.

Reviewers: evan, spang, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3965
2017-02-21 14:00:08 -08:00
Juan Tejada 66873e3f9e bump(version) 1.0.29 2017-02-21 13:12:09 -08:00
Evan Morikawa 3071856a8e [client-app] symlink in babelrc and resolve links
Summary:
The prod build was failing because it couldn't find the .babelrc.

I decided to put the babelrc in the main nylas-mail repo with the
expectation that it'll need to be in the open source version too. To DRY
up this for us building, we sylink the root one to the client-app babelrc

Also since we now dereference symlinks we don't need to do the full copy
of nylas-private-resources so we don't have two error reporters floating
around

Test Plan: npm run build-client

Reviewers: juan, spang

Reviewed By: juan, spang

Differential Revision: https://phab.nylas.com/D3993
2017-02-21 16:08:59 -05:00
Christine Spang 776cf26d7a [client-app] Convert nylas-env.coffee to ES6 😓
Test Plan: unit tests + run the app

Reviewers: juan, halla, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3979
2017-02-21 12:38:36 -08:00
Juan Tejada 15bcf974d2 [client-app] For consistency, proxy marking unread thru ThreadListActionStore
Summary: Depends on D3990

Test Plan: manual

Reviewers: spang, halla, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3992
2017-02-21 12:31:07 -08:00
Juan Tejada 20f7ae47ca [client-app] For consistency, proxy starring thru ThreadListActionStore
Summary: See title

Test Plan: Depends on D3990

Reviewers: spang, halla, mark, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3991
2017-02-21 12:29:52 -08:00
Juan Tejada 19195e9686 [client-app] Measure and report times for trashing and marking as spam
Summary:
See title

Depends on D3989

Test Plan: manual

Reviewers: evan, spang, halla

Reviewed By: spang, halla

Differential Revision: https://phab.nylas.com/D3990
2017-02-21 12:26:58 -08:00
Juan Tejada 6dbbf1e094 [client-app] Measure and report times for removing labels from threads
Summary:
This commit adds new actions, `applyCategoryToThreads` and `removeCategoryFromThreads`
to be proxied and measured through the ThreadListActionsStore. These are called when
labels are added or removed via the category picker or by removing using
the label icon.

For now, we are only interested in timing actions that remove threads
from the inbox.

Test Plan: manual + reenabled unit tests for category-picker

Reviewers: spang, evan, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D3989
2017-02-21 12:13:17 -08:00
Juan Tejada fb85db0964 [client-app]: Measure and report time when dragging threads to sidebar
Summary:
This commit adds a new action, `moveThreadsToPerspective` to be proxied and
measured through the ThreadListActionsStore. This action is called when
a thread or group of threads is dragged and dropped into an item in the
sidebar.

For now, we are only interested in timing actions that remove threads
from the inbox

Depends on D3984

Test Plan: manual

Reviewers: halla, evan, spang

Reviewed By: evan, spang

Differential Revision: https://phab.nylas.com/D3985
2017-02-21 12:10:52 -08:00
Juan Tejada 90e49d31aa [client-app] Measure and report times for removing from inbox
Summary:
This commit adds a new action, `removeThreadsFromView` to be proxied and
measured through the ThreadListActionsStore

This action can encompass many different actions, e.g.:
- unstarring in starred view
- changing unread in unread view
- Moving to inbox from trash
- archiving a search result (which won't actually remove it from the thread-list)

However, for now, we are only interested in timing actions that remove threads
from the inbox

Depends on D3983

Test Plan: manual

Reviewers: halla, spang, evan

Reviewed By: spang, evan

Differential Revision: https://phab.nylas.com/D3984
2017-02-21 11:53:38 -08:00
Juan Tejada a8fbcb0c93 [client-app] Measure and report archiving times
Summary:
This commit makes so it we report perf metrics for archive actions.
To achieve this, I added a new `ThreadListActionsStore` which serves as
a proxy for thread actions, which allow us to time them.

The new store is in charge of listening to thread list actions, creating and
queueing  the appropriate tasks for any given action, and timing and
reporting action times to our MetricsReporter.

This commit only times archiving actions, and subsequent diffs will time
other relevant thread list actions.

Test Plan: manual

Reviewers: halla, spang, evan

Reviewed By: spang, evan

Differential Revision: https://phab.nylas.com/D3983
2017-02-21 11:50:55 -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 39c6c04bc4 [client-sync] metrics(Part 5) Rename PerformanceMonitor to GlobalTimer
Summary:
This global module wasn't really related to performance, but rather with
timing things across different processes in the app. I believe this name
is more appropriate.

Test Plan: I can still use NylasEnv.timer (instead of NylasEnv.perf)

Reviewers: spang, evan

Reviewed By: spang, evan

Differential Revision: https://phab.nylas.com/D3972
2017-02-21 11:47:33 -08:00
Juan Tejada 8afb14c4c6 [*] metrics(Part 4): Make /ingest-metrics use MetricsReporter
Summary:
This ensures that the Libhoney instance is a singleton in cloud
processes.

Test Plan: manual

Reviewers: mark, spang

Reviewed By: spang

Differential Revision: https://phab.nylas.com/D3969
2017-02-21 11:46:15 -08:00
Juan Tejada dc2f032325 [*] metrics(Part 3) Don't report CPU usage on every reportEvent
Summary: See title

Test Plan: Manual

Reviewers: spang

Reviewed By: spang

Differential Revision: https://phab.nylas.com/D3968
2017-02-21 11:45:29 -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 fd838a4280 [client-app] resolve symlinks before copying files
Summary:
In client-app/node_modules Lerna symlinks isomorphic-core to '../../'.
When we copy everything over to our tmp directory when building, we copy
over the relative symlink! Damn you lerna.

We get around this by resolving the symlinks BEFORE copying and caching
them in the installer task. Then the file copy always works.

Also, for some reason the glob {absolute} param doesn't seem to work in
the latest version. I'm manually creating an absolute path for the compile
target since it's a bit more transparent what's happening anyway.

Test Plan: `npm run build-client`

Reviewers: spang, jerm, juan, halla

Reviewed By: juan, halla

Differential Revision: https://phab.nylas.com/D3988
2017-02-21 14:36:18 -05:00
Evan Morikawa a7686bb35b [client-app] build tasks now use absolute paths
Summary:
Grunt is hardcoded to use paths relative to wherever the Gruntfile is
located. Unfortunately it also expects the grunt packages to be siblings
of that gruntfile. We can get around this by changing the relative base
path, but then the cwd is different for each tasks. This is okay as long
as we use absolute paths for various files in each of our tasks. This
updates our grunt tasks to use absolute paths

Test Plan: `npm run build-client`

Reviewers: spang, halla, jerm, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3987
2017-02-21 14:35:51 -05:00
Juan Tejada 7fa5e2fb3f [client-sync] Make imap ended error retryable
Summary: This error should really be retryable, and will prevent more red boxes

Test Plan: manual

Reviewers: evan, spang

Reviewed By: evan, spang

Differential Revision: https://phab.nylas.com/D3982
2017-02-21 11:08:47 -08:00
Halla Moore d415e7792e [isomorphic-core] Ensure smtp config exists
Summary:
We previously weren't saving the smtp settings for cloud gmail accounts,
and even though we fixed that, we still need to be able to handle the accounts
that were authed before that fix went out. This diff changes `smtpConfig()` to
always call `credentialsForProvider` instead of depending on what was saved
in the database.

Test Plan: manual

Reviewers: juan, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3980
2017-02-21 11:06:07 -08:00
Juan Tejada fad1671d6e [client-app] Fix delta-processor tests 2017-02-21 08:56:47 -08:00
Halla Moore a9f3033907 [cloud-workers, isomorphic-core] Implement the SendRemindersWorker
Summary:
Process reminders metadata and send the reminder email if there
haven't been any replies. Checking for replies involves checking
each of the folders in `folderImapNames` for messages that are in
reply to the messages in `messageIdHeaders`.

Test Plan: manual

Reviewers: khamidou, juan, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3958
2017-02-17 19:08:20 -08:00
Halla Moore 1e0ae4fb03 [cloud-core, cloud-workers, isomorphic-core] Fix sending from cloud-workers
Summary:
The SendmailClient is in isomorphic-core, but wasn't working properly when
called from cloud-workers. This diff includes a variety of fixes to get it
working:

  - Add smtp config to connection settings in cloud-core
  - Don't use `PromiseUtils.promisify` for a function that needs `this` to
    be bound properly
  - Don't reference `NylasEnv` (That error gets caught and reported
    elsewhere anyway, we don't need to report it here)
  - Default `message.uploads` to `[]` to keep iterator happy
  - Add Gmail environment variables to cloud-worker app

Test Plan: manual

Reviewers: juan, khamidou, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3954
2017-02-17 19:05:36 -08:00
Halla Moore 302f3b6215 [client-app, client-private-plugins] Update the reminders metadata structure
Summary:
The new send-reminders cloud-worker needs different information to check for
replies now that the sync engine is local. Make sure we save all of the
required information to the metadata. The current values are:

  - expiration: the time the reminder should be sent, if no new replies
  - folderImapNames: the folders to check for replies
  - messageIdHeaders: the Message-Id headers of messages in the thread
    (we search for In-Reply-To headers that match these values, and also
     use them to decide whether a message is a new reply or an old reply)
  - replyTo: the message to reply to when sending the reminder email
  - subject: the subject to use when sending the reminder email

Test Plan: manual

Reviewers: juan, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3952
2017-02-17 19:03:07 -08:00
Evan Morikawa 12236753d5 [client-app] only show cloud deltas in the developer bar
Summary:
This removes client-sync deltas from the developerbar delta list.

We ONLY show cloud deltas now.

The connection between client-sync is no longer a network delta stream,
it's a direct function call. It makes no sense to show its status.

This now shows a single dot representing the state of the cloud delta
stream.

Test Plan: Manually connect and disconnect local cloud API and see icon change

Reviewers: halla, spang, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3977
2017-02-17 17:33:18 -08:00
Evan Morikawa d1b5cb9952 [client-app] allow returning of message from DOM
Summary:
SFDC's task to upload an email to salesforce needs the stripped DOM of a
Message object to call `innerText`. The API was changed to return a string
instead of the DOM. This adds a flag to request the DOM instead of a
string.

Test Plan:
Manually assert `EnsureMessageOnSalesforceTask` properly can add the plain
text to the Salesforce Task object

Reviewers: halla, mark, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3976
2017-02-17 17:32:45 -08:00
Evan Morikawa 5577d1b4d3 [iso-core] fix error toJSON in node environments
Summary:
Node's native `Error` object does NOT implement toJSON. We attempt to call
toJSON when reporting errors. This wasn't noticed until now because
bunyan's pretty logger (which only run in dev mode) started JSONifying
errors

Test Plan:
Try and API auth with a bad username with local setup. See that it throws
toJSON error. After patch, error properly serializes

Reviewers: spang, halla, jerm, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3975
2017-02-17 17:31:36 -08:00
Evan Morikawa c149e9171d [client-app] fix errors with no message
Summary:
Some API Errors, like ECONNREFUSED, have no \.message.
Catch this in the error reporter

Test Plan: Manually create a non-message error and see better error message

Reviewers: spang, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3973
2017-02-17 17:29:33 -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
Evan Morikawa 9994b2b3d7 [client-app] remove rouge-committed packages folder 2017-02-17 15:08:26 -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
Halla Moore 393474a4d8 [isomorphic-core] Allow use of atob() in cloud environments
Summary: atob() is a global in browser environments, but needs to be imported otherwise.

Test Plan: manual

Reviewers: juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3950
2017-02-17 14:46:07 -08:00
Evan Morikawa ad65b7d77a [client-app] convert FocusedContactStore to es6 2017-02-17 14:17:30 -08:00
Mark Hahnenberg ebb19b4cd8 [client-app] Fix stale import
Summary: Moved a file and forgot to update this import :-/

Test Plan: Run locally

Reviewers: juan, evan, spang

Differential Revision: https://phab.nylas.com/D3970
2017-02-17 14:14:17 -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
Mark Hahnenberg 00299e446d [message-list] Add support for double-clicking inline images to open
Summary: See title

Test Plan: Run locally, verify that double clicking inline images opens them

Reviewers: evan, juan, spang

Reviewed By: juan, spang

Differential Revision: https://phab.nylas.com/D3963
2017-02-17 14:01:07 -08:00
Christine Spang 9cf703c6cd [*] Clean up some cruft
Summary: This stuff doesn't seem to be used for anything anymore and it's cluttering up the client-app dir.

Test Plan: ran the app, also did `npm start` with no ~/.nylas-dev

Reviewers: evan, juan

Reviewed By: evan, juan

Differential Revision: https://phab.nylas.com/D3961
2017-02-17 13:51:27 -08:00
Juan Tejada 4d5e69da7d [*] metrics(Part 1) Add env-helpers to iso core
Summary:
This is a set of functions which will allow isomorphic-core to detect
which environment it is running on.

This will be useful for moving the metrics reporter to iso-core

Test Plan: manual

Reviewers: mark, spang

Reviewed By: spang

Differential Revision: https://phab.nylas.com/D3966
2017-02-17 13:34:07 -08:00
Mark Hahnenberg a95ca8b07a [*] Fix electron launch args
Summary:
We were doing some incorrect processing of args passed to the main
function which was causing us to think we were launching NM by passing a
file (which creates a new draft and tries to attach that file). Since
were trying to attach 'packages/client-app', this was causing an error
dialogue to appear indicating that it wasn't possible to attach a
directory.

Test Plan: Run locally, verify no dialogue

Reviewers: evan, spang, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3962
2017-02-17 13:03:20 -08:00
Evan Morikawa c4a26e1dcd [*] convert everyting to rx-lite 2017-02-17 12:15:23 -08:00
Mark Hahnenberg 8267a14966 [iso-core] Add BackoffScheduler to exports
Summary: See title

Test Plan: Make sure NM loads

Reviewers: evan, spang, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3959
2017-02-17 11:03:26 -08:00
Juan Tejada 5a3a105e9d [*] Move BackoffSchedulers into isomorphic-core 2017-02-17 10:11:35 -08:00
Juan Tejada d385350808 [client-app] Update changelog 2017-02-17 10:11:35 -08:00
Evan Morikawa 3113fba25b [client-app] fix module spec
This changed because we now globally require node_modules
2017-02-17 10:00:43 -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
Christine Spang 25d88589ba fix(linux-build): Update rpm spec to use new package name 2017-02-16 16:58:27 -08:00
Christine Spang 2bc6b7ac0f fix(linux-build): Change package name to Nylas Mail
Need this in order to allow parallel installation of Nylas Pro and Nylas
Mail.
2017-02-16 16:07:36 -08:00
Christine Spang 15697a9064 fix(cli): s/n1/nylas-mail/ 2017-02-16 16:02:08 -08:00
Evan Morikawa 9407533f51 [client-*] Rename packages folders and update readme 2017-02-16 13:31:37 -08:00
Evan Morikawa 84e2c75ce9 Merge remote-tracking branch 'k2/master' 2017-02-16 13:20:20 -08:00
Evan Morikawa 9f6889b03f [nylas-mail] move nylas-mail into /packages 2017-02-16 13:15:10 -08:00
Evan Morikawa 41bc1e5f5c [local-sync] fix related threads
Summary:
When saving a thread, we weren't properly setting the `participants`
object. Since Sequelize has object properties under getters and setters,
doing `this.participants.push()` did nothing.

Since threads had no `participants`, the related threads widget, which
queries through there, did nothing.

Test Plan: Manually inspect DB and use the related threads widget

Reviewers: halla, spang, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3938
2017-02-16 12:33:52 -08:00
Evan Morikawa 9932fa4882 [*] update package.json to lerna specs
Summary:
This is the result of auto package.json fixing by lerna. Would be nice to
commit this so you can run script/bootstrap without it making local
changes.

I didn't manually bump any versions.

Test Plan: manual

Reviewers: mark, halla, spang, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3934
2017-02-16 12:32:07 -08:00
Mark Hahnenberg d654e80a96 [local-sync] Use new BatteryStatusManager to compute sync loop delay
Summary:
We don't need to check folders that often while on battery. Check every
5 minutes rather than every 10 seconds.

Test Plan: Run locally, verify the timeout is longer while on battery

Reviewers: evan, spang, juan

Reviewed By: spang, juan

Differential Revision: https://phab.nylas.com/D3940
2017-02-16 10:35:24 -08:00
Juan Tejada e15079874b [local-sync] 🎨 sync loop error handler
Summary: See title

Test Plan: manual

Reviewers: evan, mark, spang

Reviewed By: spang

Subscribers: mark

Differential Revision: https://phab.nylas.com/D3932
2017-02-15 18:46:41 -08:00
Mark Hahnenberg 5f4e5cbbdf [sentry] Don't use breadcrumbs in dev mode
Summary: They obscure the location of our logs in the dev tools.

Test Plan: Run locally, verify that logs link to proper place

Reviewers: evan, spang, juan

Reviewed By: spang, juan

Differential Revision: https://phab.nylas.com/D3939
2017-02-15 18:38:55 -08:00
Christine Spang dce9743283 [local-sync] Refresh Google OAuth2 tokens when Invalid Credentials occurs in sync loop
Summary:
Previously, we would only refresh Google OAuth2 access tokens at the
beginning of the sync loop, and _only_ if the access token had already
expired. This meant that if an access token expired in the middle of a
sync loop iteration, the user would get prompted with the reauth red box
for their account and would have to either go through the oauth flow
again or restart the app for sync to continue.

This diff makes two changes:

1. Adds 5min of padding to the refresh window, so if a token will expire
in <5min, we'll go ahead and refresh the token. This will reduce the
possibility that an access token can expire during a sync loop
iteration.

2. Catches Invalid Credentials IMAPAuthenticationErrors for Gmail
accounts and forces a token refresh on the next sync loop.

These should prevent a user from _ever_ having to reauth their Gmail
account unless the refresh token is revoked, or we encounter some other
permanent error trying to refresh the token.

Fixes T7775 (at least some cases)

Test Plan: manual

Reviewers: khamidou, evan, juan

Reviewed By: juan

Maniphest Tasks: T7775, T7755

Differential Revision: https://phab.nylas.com/D3908
2017-02-15 14:37:31 -08:00
Christine Spang 56422eee10 Add TODOs about retries in sending 2017-02-15 14:23:38 -08:00
Juan Tejada d3f0847f79 [local-sync] Add exponential backoff when retrying syncback tasks
Summary:
Instead of re-implementing exponential backoff, throw the retryable
error so the sync loop handles it and backs-off

Test Plan: manual

Reviewers: spang, halla

Reviewed By: halla

Differential Revision: https://phab.nylas.com/D3914
2017-02-15 12:31:43 -08:00
Evan Morikawa f409bf8be1 [SFDC] Update SalesforceSearchIndexer for new search indexing
Summary:
Update Salesforce to use the new search indexer

Depends on D3911

Test Plan: Manually bootup SFDC and ensure it launches and indexes models properly

Reviewers: mark, halla, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3913
2017-02-15 11:33:28 -08:00
Christine Spang 04478f81c8 [cloud-api,cloud-workers,local-sync] Bump hapi version
We were two major versions behind. I upgraded and everything is working fine
in dev.
2017-02-15 07:55:48 -08:00
Juan Tejada 78f67d4a76 [cloud-api] KEEP Timeout streaming API connections every 15 minutes
This reverts commit a1b997f350.
This is actually working correctly to reduce REDIS connections
2017-02-14 18:28:07 -08:00
Evan Morikawa a1b997f350 Revert "[cloud-api] Timeout streaming API connections every 15 minutes"
This reverts commit 6e43c86a95.
2017-02-14 17:55:08 -08:00
Juan Tejada d1bd77d11b [cloud-*] Properly listen to stream disconnect events to close redis connections
See the following for why we need to set up the listeners on the raw
stream.
http://stackoverflow.com/questions/26221000/detecting-when-a-long-request-has-ended-in-nodejs-express
https://github.com/hapijs/discuss/issues/322#issuecomment-235999544

Hapi's disconnect event only fires on error or unexpected aborts: https://hapijs.com/api#response-events
2017-02-14 17:02:31 -08:00
Karim Hamidou 6e43c86a95 [cloud-api] Timeout streaming API connections every 15 minutes
Summary: Should help a fair bit with our redis connection pileup.

Test Plan: Tested manually.

Reviewers: mark, spang, juan, evan

Reviewed By: mark, spang, evan

Differential Revision: https://phab.nylas.com/D3916
2017-02-14 14:43:23 -08:00
Evan Morikawa 4245ec7aa5 [isomorphic-core] add accountId index definition to Transaction table 2017-02-14 10:48:12 -08:00
Juan Tejada fe57a5884b [local-sync] 🎨 comment 2017-02-13 21:52:21 -08:00
Juan Tejada e441553b50 [local-sync] syncback(Part 5): Always keep retrying tasks if error is retryable
Summary:
This commit makes it so we always continue retrying syncback tasks as long as they
error with a retryable error. There's really no reason to not continue retrying syncback
tasks after an arbitrary number of retries (especially such a low one) if we
encounter a retryable error. Before this commit, if for example we got 2 random
network errors in a row, we would just mark the task as failed even
though it would eventually succeed in subsequent attempts.

Previously, when N1 synced against the cloud api, we would indefinitely
retry a Task if we continued getting retryable errors. This ensures that
the app can work correctly offline and prevents displaying unecessary errors to
the user, and having actions bouncing back or sending messages without putting them
in the sent folder.

Additionally, this commit ensures that when cleaning up messages without a
folderImapUID we don't delete messages that are currently being added to the sent
folder. This is relevant to this commit because given that we could retry the
EnsureMessageInSentFolder task indefinitely, we might end up deleting that
message because it wont have a uid until the task succeeds.

Depends on D3898

Test Plan: manual

Reviewers: mark, spang, evan, halla

Reviewed By: spang, evan, halla

Differential Revision: https://phab.nylas.com/D3900
2017-02-13 21:50:02 -08:00
Christine Spang 3854051350 [local-sync] 🔥 Message syncback tasks
Summary:
Since all Nylas Mail actions are thread-based except for sending,
we never use any of these. Make `git grep` less confusing by getting rid
of them.

Test Plan: use the app

Reviewers: evan, halla, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3910
2017-02-13 18:27:18 -08:00
Juan Tejada 964bc3208f [local-sync] syncback(Part 4): Don't always mark INPROGRESS tasks as failed at beginning of sync
Summary:
Previously, if you were to close the app while you had any tasks queued,
these would be marked as failed the next time you open the app, showing
an annoying error message and reverting any optimisitic actions.

However, we don't need to be so defensive about retrying tasks because
the only tasks we can't retry are the Sending tasks. All of the other
tasks like moving or changing labels are fine to retry (trying  move the same
set of uids twice wont cause an error)

This commit adds an extra status to syncback requests, "NOTRETRYABLE".
Only NOTRETRYABLE requests will be marked as failed at the beginning of
the sync loop, and any INPROGRESS tasks will be marked as NEW so they
can be retried

Depends on D3896

Test Plan: manual

Reviewers: mark, evan, spang, halla

Reviewed By: evan, halla

Differential Revision: https://phab.nylas.com/D3898
2017-02-13 13:48:29 -08:00
Juan Tejada 4e85993957 [local-sync] syncback(Part 3): Fixup runSyncbackTasks
Summary:
Now that we don't run Send tasks outside the sync loop, we don't need
that awful hack wich required passing a `runTask` callback to
`runSyncbackTask` in order to customize how to run the task.

Instead, runSyncbackTask now knows 2 ways to run a task, either via imap, or
via smtp, depending on the resource declared by task to run. So now
SyncbackTasks declare a resource type they need to run, and that will be
passed as their second argument when running.

Depends D3894

Test Plan: manual

Reviewers: mark, halla, spang, evan

Reviewed By: halla, spang, evan

Differential Revision: https://phab.nylas.com/D3896
2017-02-13 13:07:20 -08:00
Juan Tejada c1ecd045d7 [local-sync] syncback(Part 2): Reinstate send tasks back into the sync loop
Summary:
We had previously ripped send tasks outside the sync loop to make them run faster,
but they run fast enough inside the loop.

This commit will also fix the scenario where if you closed the app in the
middle of a send task, the task would just hang forever and never succeed or
fail (T7818); given that it was excluded from the loop, we also had to exclude it
from the cleanup step to mark any INPROGRESS tasks as failed at the beginning
of each loop, which caused send tasks in progress to never get cleaned.

Putting them back inside the loop allows us to  fix this without adding more messy
logic, and it cleans up ugly duplicated code. Additionally, it will prevent more
duplicated code for upcoming diffs that will improve syncback task reliability
when the app is closed or window is restarted in the middle of a task.

Depends on D3893

Test Plan:
manually test sending, it still works, it's still fast. Restarted
window in the middle of send task, task fails.

Reviewers: mark, spang, halla, evan

Reviewed By: spang, halla, evan

Differential Revision: https://phab.nylas.com/D3894
2017-02-13 13:05:11 -08:00
Halla Moore 8287f4116f [local-sync, cloud-api] Add logic to handle thread metadata
Summary:
[cloud-api]
Based on the passed in `messageIds`, it finds any existing thread
metadata that might be under a different thread id. If it realizes
there are actually multiple threads that should be the same thread,
(due to getting a missing message link), it reconciles all of them.

[local-sync]
Return `message_ids` in `Thread.toJSON()`

See D3879 for tests

N1 Pairing: D3875

Test Plan: unit tests, local testing soon

Reviewers: juan, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3880
2017-02-13 13:02:40 -08:00
Juan Tejada 1e74be3b94 [local-sync] syncback(Part 1): Refactor syncback-task-helpers
Summary:
Instead of exposing helper functions, make this a class to hold the
shared state of the db, account, and logger required to run any syncback
inside an account sync loop.

Test Plan: manual

Reviewers: mark, spang, halla, evan

Reviewed By: spang, halla, evan

Differential Revision: https://phab.nylas.com/D3893
2017-02-13 13:01:28 -08:00
Juan Tejada dbb404ccba [iso-core] Detect more offline errors when sending
Summary:
See title.
I really wish we could clean up this error handling a bit better, but I don't
think its super important right now.

Test Plan: manual

Reviewers: spang, evan, halla

Reviewed By: evan, halla

Differential Revision: https://phab.nylas.com/D3903
2017-02-13 12:18:45 -08:00
Juan Tejada 85dc9f319e [local-sync] Add a better reason when waking sync for syncback
Summary: see title

Test Plan: nil

Reviewers: mark

Reviewed By: mark

Differential Revision: https://phab.nylas.com/D3892
2017-02-13 12:00:13 -08:00
Juan Tejada 3a2f2ec6fc [local-sync] More retryable IMAP errors
Summary:
I've encountered random imap errors that we mark as permanent, but that
contain try again in the error message. We should check for that

Test Plan: manual

Reviewers: spang, evan, mark

Differential Revision: https://phab.nylas.com/D3899
2017-02-11 12:56:35 -08:00
Halla Moore 2d3bb52bc8 [local-sync] Properly clean-up in-memory test database
Summary: There's no file to unlink, we just need to drop the tables.

Test Plan: manual

Reviewers: evan, juan

Reviewed By: evan, juan

Differential Revision: https://phab.nylas.com/D3878
2017-02-10 15:37:58 -08:00
Evan Morikawa e646d56bf8 [local-sync] fix sync when no messages in inbox in gmail
Summary:
If you have no messages in your Gmail Inbox (Yay Inbox Zero!) and you
connect your account and do first sync, then we get an error where we try
and fetch a range from null to -1.

This was due to a logical error in the first sync fetch code.

This diff fixes this bug and renames some variables to make it clearer
what's going on

Fixes T7842

{F11176}

Test Plan:
1. Bring Gmail to Inbox Zero
2. Connect account
3. Verify first sync works

Reviewers: spang, halla, juan

Reviewed By: juan

Maniphest Tasks: T7842

Differential Revision: https://phab.nylas.com/D3889
2017-02-10 18:20:53 -05:00
Mark Hahnenberg 8f08328329 [files] Add retry with exp backoff to IMAP connections for file download requests
Summary: See diff title

Test Plan: Run locally, make sure we backoff

Reviewers: juan, spang, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3886
2017-02-10 13:29:12 -08:00
Evan Morikawa e7ffb974e4 [local-sync] rename spec fixture folder to be correct 2017-02-09 19:05:04 -05:00
Juan Tejada 6856a2ccf7 [local-sync] When replying to a thread, properly add it to Sent folder
Summary:
Previously, when processing messages during folder sync, if the message already existed, and it belonged to a thread, we would update the message, but forget to update its thread with any changes that new
message would produce on the thread (e.g. updating the threads folders or labels).

One obvious manifestation of this was when replying to a thread: the EnsureMessageInSentFolderTask would create the new message, and then attempt to sync the sent folder to fetch the newly created message. When processing this message during sync, we would update the message but not update its thread, so the thread would not be associated to the sent folder, and it wouldn't show up in your sent items list in the UI.

Test Plan: manually verify that it works

Reviewers: evan, mark, spang

Reviewed By: mark, spang

Differential Revision: https://phab.nylas.com/D3872
2017-02-09 15:04:12 -08:00
Juan Tejada cec55f7188 [local-sync] Fix logger for local requests
Also log send task correctly
2017-02-09 10:25:08 -08:00
Juan Tejada 600be97324 [local-sync] Don't crash app when reporting error and id not available
Summary:
Our sentry reporter tries to fetch the nylas identity from the database,
and access properties on it. However, if you are in a state where there
is no identity available (like having logged out, or just starting the
app), and encoutnered an error that would be reported to sentry, we
would throw an error while reporting and that would crash the app

Also, fix lint errors and some really janky code

This fixes T7810

Test Plan: manual

Reviewers: halla, spang, evan

Reviewed By: spang, evan

Maniphest Tasks: T7810

Differential Revision: https://phab.nylas.com/D3867
2017-02-09 09:27:36 -08:00
Juan Tejada e6afea45a0 [local-sync] Prefer promises over success option for api requests
Summary: Companion D3869

Test Plan: manual

Reviewers: halla, spang, evan

Reviewed By: evan

Differential Revision: https://phab.nylas.com/D3868
2017-02-09 09:19:19 -08:00
Juan Tejada 9a3470bacb [local-sync] 🎨 logger
If the first argument to our local-sync logger is an object
(this is bunyan's api, and it's how we log from isomorphic-core and cloud-* packages
in order to have structured json logs for logstash), make sure we log
the object last and the string that comes as the second argument first.
2017-02-08 18:27:09 -08:00
Christine Spang 2eeae66b2d [local-sync] Fix threading bug with open and link tracking enabled
Summary: Fixes T7649

Test Plan: FML writing unit tests now

Reviewers: evan, mark, juan

Reviewed By: mark, juan

Maniphest Tasks: T7649

Differential Revision: https://phab.nylas.com/D3863
2017-02-08 17:37:59 -08:00
Juan Tejada 8f805df476 [local-sync] Fix Mailbox does not exist error during sync
Summary:
This error ocurred, to the best of our knowledge, on iCloud accounts
that had been linked to other clients like Airmail.

On such accounts, node-imap would incorrectly parse the mailbox list
from imap, and return an `Airmail` folder which did not exist, causing
us to try to sync that nonexistent folder and error in the sync loop.

This error is amongst the most frequent we've seen in Sentry and
Support: https://sentry.io/nylas/nylas-mail/issues/213158962/events/4897450600/

The fix es detailed in the PR to node-imap: https://github.com/mscdex/node-imap/pull/594/files
This commit only points the node-imap dependency to our fork for now

Test Plan: manual and unit tests in node-imap

Reviewers: mark, khamidou, spang

Reviewed By: spang

Differential Revision: https://phab.nylas.com/D3860
2017-02-08 17:23:17 -08:00
Evan Morikawa bf99b7862c [local-sync] use different port in dev mode
Summary:
This diff (and the K2 counterpart diff) allow us to run dev-mode Nylas
Mail side-by-side with prod Nylas Mail.

There were 4 things that needed to change:

1. Use different config dir
2. Use different keychain name
3. Use different localhost port
4. Prevent Electron's `app.makeSingleInstance` from killing our app

All of these are activated through `NylasEnv.inDevMode()`.

Test Plan: Manual

Reviewers: halla, mark, spang, khamidou, juan

Reviewed By: juan

Differential Revision: https://phab.nylas.com/D3861
2017-02-08 18:16:32 -05:00
Juan Tejada c3873c16d6 [local-sync] Fix local-sync logs relayed to main (browser) process
Summary:
In electron, the --enable-logging flag makes it so the main browser
process logs to stdout all of the logs generated from within the renderer
processes.

Unfortunately, the main process will only log out the first argument passed to
`console.log` from within a renderer process (see https://github.com/electron/electron/issues/7061)

This commit makes it so that the local sync logger logs most of the log line in the first
argument passed to `console.log`

Test Plan: manual

Reviewers: evan, mark

Reviewed By: mark

Differential Revision: https://phab.nylas.com/D3852
2017-02-07 12:01:16 -08:00
Juan Tejada a5d2a92a61 [local-sync] Fix SyncMetricsCollector logger 2017-02-07 11:19:31 -08:00
Juan Tejada 00552469b3 [local-sync] Log account info in all logs during local sync + color code
Summary:
When multiple accounts are syncing, it's very hard to scan the local
sync logs because it is unclear to which account the logs belong to,
and it makes debugging hard.

This commit makes it so that all logs from local-sync include the
account info, with the account email prefixed at the beginning of each
log line (this allows filtering), and color coded by account.

Test Plan: manual

Reviewers: mark, spang, khamidou, evan, halla

Reviewed By: evan, halla

Differential Revision: https://phab.nylas.com/D3851
2017-02-07 11:04:10 -08:00
Juan Tejada ab95b9a612 [local-sync] Continously increment timeout for imap connection if we see timeout errors
Summary:
On each sync loop, we increment the socketTimeout based on how many times we've
seen socket timeouts in a row. The max socket timeout is 10m

Test Plan: manual

Reviewers: evan, spang, mark

Reviewed By: mark

Differential Revision: https://phab.nylas.com/D3843
2017-02-06 14:09:34 -08:00
Christine Spang a22b3a1fc0 [local-sync] Download message batches newest first
Summary:
In most cases (and especially so on Gmail and in the inbox on generic
IMAP), messages with higher UIDs are newer---and even if they aren't the
newest possible messages in other generic IMAP folders, they are the
most recent messages that have been moved to that folder.

Our previous batching strategy unfortunately resulted in us downloading
the lowest UID in each batch first, which was especially confusing when
connecting a new account and having the first message pop up on the
screen be a message from hours or days ago.

This patch changes the batching strategy in three ways:

1. Within a batch, we process downloaded messages from highest UID to
lowest UID.

2. We download batches in order of the ones containing the highest UIDs
first.

3. We group together more UIDs within a single batch by ignoring charset
and transfer-encoding on parts and grouping only by MIME part IDs (which
is the only thing you have to pass to the IMAP FETCH command---no idea
why we included this extraneous part data before, probably just
convenience.)

Example old grouping:

  batch key: '[{"id":"2","transferEncoding":"QUOTED-PRINTABLE","charset":"UTF-8","mimeType":"text/html"}]'
  batch UIDs: [356416,356418,356420,356423,356432,356433,356435,356436,356437,356442,356444]

  batch key: '[{"id":"2","transferEncoding":"QUOTED-PRINTABLE","charset":"Windows-1252","mimeType":"text/html"}]'
  batch UIDs: [353777]

In the new strategy, all of these messages will be downloaded with the
same FETCH command, reducing IMAP round trips before message processing
begins.

Fixes T7770

Test Plan: manual - connect a new account and see that most recent message downloads first

Reviewers: mark, evan, juan

Reviewed By: juan

Maniphest Tasks: T7770

Differential Revision: https://phab.nylas.com/D3838
2017-02-06 10:30:02 -08:00