Summary:
refactor multi-send
This diff started off by fixing sending with attachments.
The issue is that our `FileUploadStore` listened for
`Actions.sendDraftSuccess` as its signal to remove the files from the
.nylas temp directory. Unfortunately, the old MultiSend tasks, after
delivery of the message, would try and put the base message in the sent
folder. Since we already deleted the file from our local temp dir,
creating the base message for the sent folder would fail.
This exposed a much bigger issue which is that we don't consistently
distinguish between "delivery" of a message and any post-processing we do
(like filling the sent folder). This was leading to a variety of other
subtle issues.
For example, N1 assumes that if the SendMessage task fails, then we pop
the draft back up and ask the user to try again. Unfortunately, since we
were combining "delivery" and "post processing" it was possible for the
message to actually deliver, but fail when stuffing the sent folder, or
fail due to some other random bug. This would cause the user to send the
message twice.
To help us ensure we never "deliver" twice and handle errors more
intuitively, I separated out the two concepts.
Now there are "send" set of tasks and endpoints, and a
"EnsureMessageInSentFolder" set of tasks and endpoint (the latter used to
be ambiguously known as ReconcileMultiSend, whatever that meant)
The logic for send hasn't changed. This is mostly a renaming and moving
files around.
Test Plan: manual :(
Reviewers: jackie, juan, halla
Reviewed By: juan, halla
Differential Revision: https://phab.nylas.com/D3577
Contrary to what you might think, a message can have both an empty From: header
and multiple From: headers / multiple addresses in a From header. In that case,
we must save all of them and let the client decide how to display.
Fixes: T7370
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
The 'encoding' library transparently upgrades to using iconv instead of
iconv-lite, if available. This allows us to support more encodings in
emails, such as ISO-2022-JP.
Fixes: T7358
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: We were creating duplicate `Message` objects because the formatting for the date was different between `buildForSend` and `parseFromImap`. Now, we create the initial hash using the same format that `buildmail` uses to ensure that we generate the same IDs.
Test Plan: Tested locally.
Reviewers: evan, juan, spang
Reviewed By: spang
Differential Revision: https://phab.nylas.com/D3559
Summary:
Because of the way we were attempting to parse contacts from
From/To/Cc/Bcc headers by converting them to JSON with a regex, we were
erroneously breaking contacts that contained commas in quoted names into
multiple contacts. This could result in things like parsing multiple
addresses for the From: header, incorrectly!
To resolve the problem, replace our homegrown logic with mimelib's
seemingly excellent parseAddresses(), which handles this and a myriad of
other cases correctly.
Fixes: T7370
Test Plan: unit tests included
Reviewers: mark
Reviewed By: mark
Differential Revision: https://phab.nylas.com/D3565
This doesn't do anything with sqlite, and just generates the following
warning in the logs:
>> WARNING: SQLite does not support TEXT with options. Plain `TEXT` will be used instead.
>> Check: https://www.sqlite.org/datatype3.html
Will fix these once I've finished up the current slew of bugfixes I'm
working on---kind of a pain to ensure they're passing in all
intermediate states.
Summary:
Similar to the fix in D3555, concurrent message processing may cause insert
races for folder and label thread associations. If the row is already present,
we can simply do nothing.
Test Plan: manual for now
Reviewers: jackie
Reviewed By: jackie
Differential Revision: https://phab.nylas.com/D3557
Summary:
When the Date: header is not present, use the INTERNALDATE from the IMAP server
instead.
Test Plan: manual for now - will add a regression test for this though
Reviewers: juan, jackie
Reviewed By: jackie
Differential Revision: https://phab.nylas.com/D3556
Summary:
Because of JavaScript's asynchronous nature, it is possible that we will
be processing several downloaded messages concurrently. This can lead to
calling extractContacts() in an interleaved fashion, which it was not
designed to handle. It looks up which contacts are already in the
database and then performs inserts or updates accordingly, assuming
nothing has changed in the contacts table in between---which is not
true! If several messages have similar contacts, an insert race can
cause one of the inserts to throw an unhandled exception.
We fix this by simply catching the unique constraint error, and falling
back to an update instead. (There's not really a better way to deal
with write races other than to enforce that we process contacts from
messages serially, as transactions are of no help here.)
This commit also removes extractContacts()'s return value, which is not
currently used and I found confusing.
Test Plan: manual
Reviewers: juan, evan, mark, jackie
Reviewed By: jackie
Differential Revision: https://phab.nylas.com/D3555
Summary:
This was leading us to put funny things like 'Nylas !' in some snippets that used
tags like <i> and <b> for text formatting. This is probs a teeny little bit slower
than the previous version since it invokes a callback on a lot more nodes, but we
can't really fix this issue without knowledge of the preceding tag name.
Test Plan: unit test included!!
Reviewers: evan, jackie
Reviewed By: jackie
Differential Revision: https://phab.nylas.com/D3553
Summary:
I have quite a few emails in my mailbox that have both multiple Reply-To
addresses. This is perfectly OK by the spec.
Fixes: T7369
Test Plan:
regression test coming - making a list and planning to update all the tests once I've hammered out the current crop of fixes I've identified
I also tested and made sure that N1 does the right thing in this case...
multiple Reply-To addresses are displayed correctly, and when you hit "Reply" a
new draft is started with both in the To: field. Makes sense given this is
something the Python sync engine supported too.
Reviewers: jackie
Reviewed By: jackie
Differential Revision: https://phab.nylas.com/D3558
Using node-imap's parseHeader function to parse headers was resulting in
a huge number of message parse failures on Office365 accounts, because
the results contained unicode control character 9 and we'd then feed that
string to JSON.parse when extracting contacts, which would throw an
exception.
Using mimelib's header parsing function eliminates these errors.
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:
We were seeing JS blocking in snippet extraction of up to 2k ms. This
is because we were walking the entire DOM of a message and extracting
all text, regardless of message size---and using our own homegrown
DOM walker function.
To remedy this, use the standard TreeWalker from the Chrome browser
APIs (which in benchmarks looks 2-4x faster) and also exit out of
the DOM walking process once we've accumulated enough text to create
a snippet. Informal eyeballing of timing metrics for this function suggests
the new implementation is something like 10-100x faster for some messages.
As a bonus, we get to delete some code and end up with a cleaner
implementation!
Test Plan: old unit tests yaay
Reviewers: juan
Reviewed By: juan
Subscribers: evan
Differential Revision: https://phab.nylas.com/D3543
Summary:
Save metadata correctly by reassigning an object to value.
Since account IDs are different between N1 and N1 Cloud, use just the message ID, which should be unique.
Test Plan: Tested locally.
Reviewers: evan, juan
Reviewed By: evan, juan
Differential Revision: https://phab.nylas.com/D3524
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:
- Don't fail if there's no subject, just set it to `(no subject)`
- Support "BINARY" content-transfer-encoding. (This really means
that there is no encoding, so it's simple to add support for it)
Test Plan: tested locally
Reviewers: jackie
Reviewed By: jackie
Differential Revision: https://phab.nylas.com/D3528
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:
Add internal state to the sync worker to allow for it to be interrupted
and restarted.
The concept in this commit is that if we've tried to trigger a sync enough times
while its already in progress, bail and start over.
Usually, we manually trigger sync loops when we queue a new SyncbackTasks,
so that the newly queued task gets executed. This is necessary because the only
way to run SyncbackTasks is via the sync loop, for consistency and simplicity
reasons.
For example, we might run into a case where we queue a SendMessage task,
and we want it to be executed ASAP, but if we're in the middle of a
syncing a mailbox with a ton of folders, we wont get to the SendMessage
task after some considerable time.
Specifically this commit makes it so:
- If the number of sync attempts while in progress is > 0, make sure we schedule the next sync immediately
- If we reach a threshold of sync attempts while in progress, interrupt sync and restart
Test Plan: todo :(
Reviewers: mark, spang, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3520
