Summary:
We are getting to many imap timeout connection errors because the
authTimeout was just 5 secs
Test Plan: manual
Reviewers: khamidou, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3733
Summary:
Specifically, these imap connections have been known to hang when you close your laptop or go offline/online, even though we are passing a `socketTimeout` to node-imap. When they hang, everything freezes because the promise waiting for the result never resolves.
`_createConnectionPromise` wraps the operations with a timeout we implemented ourselves, and correctly rejects on timeout.
This commit wraps other imap operations that were missing. (I notices because I encountered the hanging on one of these operations)
Test Plan: manual
Reviewers: evan, mark, spang
Reviewed By: spang
Differential Revision: https://phab.nylas.com/D3720
Summary: Was missing from in envelope when sending custom message
Test Plan: manual, tested sending from Gmail, Office365, Yahoo, all work
Reviewers: evan, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D3700
Summary:
Don't show the attachment icon on threads that only have inline
images. We do this by assuming that inline images have a contentID,
and regular attachments do not. Also updates the way we send
attachments in order to adhere to this standard.
Test Plan: tested manually
Reviewers: spang
Reviewed By: spang
Differential Revision: https://phab.nylas.com/D3696
Summary:
When syncing folders, we check if the folder needs syncing by checking if it has any new messages via the STATUS command (STATUS returns uidnext, highestmodseq among others, and is cheaper than SELECT)
However, we can't issue a STATUS on a box that is already selected. Previously, if the box was already selected, we would just return it, but this was incorrect because we wouldn't get the latest box values (e.g. uidnext), causing us to think that there were no updates available, and skip syncing folders that actually needed to be synced.
Now, if the box is already selected when getting the status, we have to re select it to refresh the latest values
Test Plan: manual
Reviewers: evan, khamidou, spang
Reviewed By: spang
Differential Revision: https://phab.nylas.com/D3697
Summary:
Consolidating provider checks to use the same source of truth.
Fixes send issues with some provider types.
Test Plan: tested locally
Reviewers: tomasz
Reviewed By: tomasz
Differential Revision: https://phab.nylas.com/D3694
Summary:
This patch changes the sync worker to back off exponentially when there is an issue syncing an account. This has two goals:
- first, it's a bit dangerous to retry immediately. We don't want hundreds of thousands of machines trying to refresh tokens unsuccessfully because our service is struggling.
- second, it's nicer on the CPU to wait a bit between retries.
Currently, we sleep for at most 2 minutes, with some random jitter added.
Test Plan: Tested manually, stared at the code a long time.
Reviewers: evan, juan
Reviewed By: evan, juan
Differential Revision: https://phab.nylas.com/D3684
Summary: Treat any that aren't gmail or office365 as standard imap
Test Plan: manual
Reviewers: juan, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3686
Summary:
Before trying to sync a folder, check if we actually need to do so. This will prevent us from doing unnecessary work that slows down the sync loop (like performing SELECT commands)
We will perform a folder sync if any of the following are true
- The folder hasn't been completely synced
- There are new messages (using imap STATUS command)
- There are attribute changes indicated via highestmodseq (using imap STATUS command)
- If server doesn't support highestmodseq, it has passed enough time since we last ran an attribute scan on the folder.
Addresses T7513
Test Plan: manual
Reviewers: evan, halla, spang
Reviewed By: halla, spang
Differential Revision: https://phab.nylas.com/D3675
Summary:
This swaps out our generic IMAP threading mechanism to use the threading
headers on the message instead of the prior way of grouping by subject
and then differentiating based on participants, as that design was
somewhat driven by what we could accomplish easily given legacy data
schema decisions and has serious caveats, such as different threads between
the same people with the same subject being misthreaded together. With K2, we
have free reign to change the data format, so we can do it right.
The algorithm is super simple:
- Define "references" as the union of the Message-Id, In-Reply-To, and
References headers on a message, filtered for valid RFC2822 Message-IDs
- On message sync, if any element of the new message's references
matches any element of an existing message's references, thread them
together
In order to accomplish this, we need to store References in a way that
allows each element to be indexed for fast lookup. That meant either
using the sqlite JSON1 extension + expression-based indices, or creating
a new table. I chose the latter as a time-tested and simple solution,
since we don't need the flexibility of JSON here.
Test Plan: manual - unit tests coming
Reviewers: khamidou, evan, juan
Reviewed By: evan, juan
Differential Revision: https://phab.nylas.com/D3651
Examined the headers on a message we sent and found this:
> X-Mailer: nodemailer (2.5.0; +http://nodemailer.com/; SMTP/2.6.0[client:2.8.0])
No need to plaster which sendmail library we're using all over every
email our users send. Turn it off!
Summary:
Previously we would unconditionally issue a SELECT when openBox was
called. Now we check if the currently open box is the one we want first and
return immediately if it is, avoiding the unnecessary SELECT (which can be
quite expensive on large folders like INBOX). We were also calling closeBox
after iterating all the messages in a thread to mark them as read/unread.
This was unnecessary and was causing extra SELECTs to be issued. Now we don't!
This diff is a 5x speedup over the old behavior when marking lots of
threads in the same folder as read all at once.
Test Plan: Run locally, measure perf with log statements
Reviewers: evan, juan
Reviewed By: evan, juan
Differential Revision: https://phab.nylas.com/D3654
Summary:
Add a new dedicated imap connection to listen for any updates or new mail on the inbox.
Previously, we wouldn't be able to receive new mail events on the inbox during the sync loop
because other mailboxes would be open while we sync them. This would cause big
delays in receiving new mail, especially if you have a lot of folders
Test Plan: manual
Reviewers: spang, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3650
Summary:
We don't want to inflate delete Transactions, but we do still want
to pass the delta itself along.
Test Plan: tested locally
Reviewers: evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3647
Summary:
This commit passes down the `socketTimeout` option to node-imap. However, just
passing this option doesn't seem to work reliably, so this commit manually implements
the socketTimeout option for our IMAPConnection.
How it works is that basically every operation is wrapped with a timeout by
augmenting the `createConnectionPromise` construct that already existed.
Test Plan:
Locally, tested by sleeping computer and turning off wifi. The
connection will successfully error and be restarted. It will reconnect when the
network is available again
Reviewers: khamidou, halla, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3642
Summary:
If we inflate delete deltas, the object we're trying to find won't exist
anymore (we just deleted it!). This is likely causing the `While inflating
${sourceName} transactions, we couldn't find models for some ${modelName}
IDs` error.
Fixes T7436
Test Plan: manual
Reviewers: spang, juan, halla
Reviewed By: halla
Maniphest Tasks: T7436
Differential Revision: https://phab.nylas.com/D3621
Summary:
Delete associated children when a parent is deleted to prevent foreign
key constraint errors. Also make sure any child hooks are run.
Test Plan: tested locally
Reviewers: evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3620
This reverts commit 0b3e3d2f39.
Interrupting sync by closing connection causes errors downstream when
`syncNow` is called elsewhere. Instead of interrupting by closing the
connection, we will post a patch to interrupt the sync loop properly
Summary:
- Ensure delete deltas make it through to N1
- Don't fail if we can't find a category that needs to be deleted
Test Plan: local
Reviewers: juan, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3593
Summary:
- Adds `File` objects onto the `Message`s so N1 sees attachments
- Ensures `File` is eagerly loaded for all messages
- Base 64 streams attachments through the local /download endpoint
- ExtractFile only uses disposition type attachment when extracting
attachments
- Makes sure we save existing messages when processing them
Test Plan: manual :(
Reviewers: juan, spang
Reviewed By: spang
Differential Revision: https://phab.nylas.com/D3595
Summary: I've found a pretty annoying bug --- N1 would stop syncing all accounts after the Internet connection dropped. It seems that deep inside node-imap or NodeJS itself, connections aren't timing out the right way. To work around this, this diff unilaterally restarts the sync every `nextSyncIn` milliseconds.
Test Plan: Tested manually by cutting internet access and checking that K2 recovered.
Reviewers: evan, juan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3573
Summary:
If you sent an email to yourself it would not show up in your inbox. This
is because sent messages would never get a lastMessageReceived timestamp.
Since we order the inbox by lastMessageReceived, setting that to null to
on sent mail would mean it never shows up in the thread list.
Also fixed an assertion bug in SFDC that requires transactions to return a
promise.
Finally added extra debug interfaces that will show more info if the delta
stream detects an inconsistency
Test Plan: manual
Reviewers: juan, halla, jackie
Reviewed By: jackie
Differential Revision: https://phab.nylas.com/D3552
Summary:
This replaces the API delta stream with a direct in-memory one
Addresses T7300
Test Plan: manual
Reviewers: jackie, halla, juan
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D3548
Summary:
Associated N1 Diff: D3545
This commit ensures that authh notifications are showed when the
underlying sync worker fails and are cleared when an account is
successfully reconnected
To achieve this, we manually keep track and update syncStates where
appropriate via `Actions.updateAccount`, given that we have access to
N1's version of the account directly from local-sync.
Initially I was considering account delta stream to the cloud-api and the local-api, but that
just complicated things more than it helped.
This commit also fixes a bug with refreshing the gmail token in which we
we were only attempting a token refresh upon restarting the app
This addresses: T7346, T7305, T7335
Test Plan: Manual
Reviewers: halla, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3544
Summary:
This is a refactor of the auth APIs to use async/await. Gmail Auth is
pretty confusing and I wanted to make it cleaner to read and easier to
use. This is also part of the general API upgrade to modern ES6
This also fixes the Gmail auth error we saw at showcase
Test Plan: manual
Reviewers: halla, jackie, mark, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D3535
Summary:
See https://github.com/mscdex/node-imap/issues/585 for details.
This issue was causing us constantly run the sync loop without pauses,
i.e. every next sync loop was scheduled immediately.
Currently, when we receive a new `'mail'`event, we trigger a new sync loop. Previously, when this happened while a sync loop was already in progress we would just ignore the event. However, my recent patch keeps track of how many times we tried to start a sync loop while one was already in progress. If the number of times this happens is > 0, it will schedule the next sync loop immediately (as opposed to waiting a constant amount seconds before the next loop).
The problem is that this new logic is making the sync worker always schedule the next sync loop immediately (without pausing for a few seconds). This is due to the following chain of events (assume we are just syncing `all` and `trash` folders):
This commit is a temporary workaround to this problem.
Test Plan: manual
Reviewers: evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3537
Summary:
Associated N1 Diff: D3530
This commit converts multi-send from a 3 step process into a 2 step
process
The first step creates the base message and sends a message per
recipient, each with its customized message body for tracking.
The second step reconciles all sent messages, specifically removing any
sent messages created by gmail, and saving the correct message to the
sent folder
This commit also ensures that we run the send tasks immediately by
ensuring we restart the sync loop if its already running
Test Plan: Manual
Reviewers: evan, jackie, halla
Reviewed By: jackie, halla
Differential Revision: https://phab.nylas.com/D3529
Summary:
Adds support for office 365
Depends on D3532
Test Plan: manual
Reviewers: jackie, halla, mark, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D3533
Summary:
This is a small patch but it's pretty complex, because of the numbers of moving parts. Gmail has two types of tokens, access and refresh tokens. Access tokens have a limited shelf life of one hour. After that they expire and you need to use your refresh token to get a new one.
We've decided to do the access token generation on the server, because we don't feel comfortable giving our users both our Google client id and secret. To do that, I've added an endpoint, `/gmail/auth/refresh` which returns a valid access token as well as an expiration date for the token.
The only place where we handle token expiration is in the sync workers. Before trying opening a new connection we check if our access token is expired. If yes, we get a new one from the API. If there's an issue doing this, we notify N1 using `NylasAPIHelpers.handleAuthenticationFailure`.
There's a second patch for N1 with tiny related fixes.
Test Plan: Tested manually. Will need to test more in the real world.
Reviewers: evan, jackie, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D3522
Summary:
Adds babel to K2
Creates a simple build script so it'll run on prod.
Test Plan: manual
Reviewers: jackie, halla, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D3527
Summary:
Move sendmail-client and errors to isomorphic-core, given that they will
probably be used by cloud-workers (plugin backends) and cloud-api
Depends on D3510
Test Plan: Manual
Reviewers: halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D3512
Summary:
This diff solves a few separate issues from T7313, T7316, T7282, and it refactors
the send code a little bit.
Initially, the problem that led to this diff was generating message ids that
wouldn't collide (which was causing errors in the message-processor). Collisions
in ids were being caused by messages that contained the exact same participants,
subject and date (most likely due bots or scripts sending emails in quick
succession)
To prevent collisions this commit adds the `message-id` header as part of the
database message id, and ensures that we set it correctly before sending, and
that it remains consistent through send, multi-send, and the sync loop.
During the refactor and review, I removed some code that assumed that we were
syncing drafts (which we aren't), and also fixes a few other known and
unknown issues around sending, message creation, and tracking, like assigning
the correct date header (we were previously assigning the draft creation date
from within N1), fixing the tracking regex, among other smaller bugs/typos.
Will address inline TODOs in a separate diff
Test Plan: TODO!!! I will add tests in another diff
Reviewers: evan, halla, jackie, khamidou
Reviewed By: halla, jackie
Differential Revision: https://phab.nylas.com/D3507
Summary: See title
Test Plan: Run locally
Reviewers: juan, evan
Reviewed By: juan, evan
Maniphest Tasks: T7281
Differential Revision: https://phab.nylas.com/D3498
Summary: See title
Test Plan: Ran it locally
Reviewers: khamidou, juan, evan
Reviewed By: juan, evan
Differential Revision: https://phab.nylas.com/D3496
Summary:
This commit fixes the following bugs in message parsing:
- we were unilaterally decoding MIME bodies as UTF-8; instead, decode according
to the charset data in the mimepart header
- '7bit' content-transfer-encoding means us-ascii, NOT utf-7
- only interpret valid content-transfer-encodings (previously we were trying
to treat various charsets as transfer-encodings)
- clearer naming: s/values/parsedMessage/
- unify snippet cleanup between plaintext & stripped HTML (merging
whitespace etc.)
Test Plan: units tests coming
Reviewers: juan
Differential Revision: https://phab.nylas.com/D3491
