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
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
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
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
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
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.
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
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
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
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
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
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
Summary:
This diff adds support for database migration to our cloud API. It's partially inspired by Halla's local-sync migration diff (D3809). You can run a migration by calling "node-babel scripts/migrate-db up|down" or by calling "npm script upgrade-db|downgrade-db".
Note that for simplicity reasons we assume that we're only writing migrations for our MySQL database – people developing locally may have to blow up there dbs whenever there's a schema change, though in practice `ALTER TABLE ADD COLUMN`statements work the same on both dbs.
Test Plan: Tested locally. Will run the metadata migration on staging.
Reviewers: evan, spang, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D3840
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
Summary:
`isSending` was an artifact from the cloud sync engine that was used to
double check that the sending process for a multi-send draft had been
initiated. I don't believe the intermediate steps are API calls anymore,
and we've had the relevant code commented out for awhile. Time to kill it!
I've revived the double-sending tests in N1 to ensure we still have sufficient
checks against sending the draft again while the first call is still sending.
See D3834
Test Plan: N1 unit tests
Reviewers: juan, evan
Reviewed By: juan, evan
Differential Revision: https://phab.nylas.com/D3835
Summary:
This code is dead and has confused me grepping around the codebase before.
If for some unexpected reason we need these routes back in the future, we
can always extract them from version control. For now the routes we aren't
using are a distraction.
Test Plan: been using Nylas Mail with this local patch all week
Reviewers: evan, halla, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D3833
Summary:
We can't try to set or remove the sent label on gmail accounts because
the operation will silently fail and cause the threads to later bounce
back.
This occurred when trying to delete or archive a thread that contained a
sent message, and we incorrectly tried to overwrite or remove all of the
labels on messages, without regard for sent.
This was causing https://github.com/nylas/nylas-mail/issues/2706 and
sending and archiving to immediately bounce back.
Addresses T7757
Test Plan: unit tests
Reviewers: halla, evan, spang
Reviewed By: halla, evan, spang
Differential Revision: https://phab.nylas.com/D3829
Summary:
Previously, we attempted to immeditely retry syncback tasks when a
RetryableError was encountered. However, if the error was an
IMAPConnection error, we would keep retrying with a broken connection,
which would keep failing.
The correct way to retry is to wait for the next sync loop, since at the
beginning of each loop we ensure that we are correctly connected to
imap.
To achieve this this commit simply marks a failed task as NEW if it
encoutners a RetryableError and we haven't retried too many times. To
keep track of the number of retries, we save a new field in the `props`
field of the request.
Test Plan: manual
Reviewers: evan, halla, mark, spang
Reviewed By: spang
Subscribers: khamidou
Differential Revision: https://phab.nylas.com/D3831
Summary:
I've decided to break my snooze patches in multiple parts to make it easier to review. This diff contains all the code related to running workers in the cloud. Cloud workers all inherit from the `Worker` class which defines a bunch of useful things like error handling.
What's left to do:
- spawn workers based on the plugin type
- add monitoring (I'm going to add a simple HTTP endpoint for that)
- writing a migration for the local sync db and the prod metadata db.
Test Plan: Tested manually.
Reviewers: halla, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3814
Summary: Make them map the same directory structure of `src`
Test Plan: unit
Reviewers: evan, halla, spang
Reviewed By: halla, spang
Differential Revision: https://phab.nylas.com/D3826
Summary:
syncState on folders may contain arbitrarily long arrays of UIDs
(particularly, failedUIDs). If we serialize this JSON column to
edgehill.db, we can end up serializing very large objects when
persisting the local task queue. When the queue contains many tasks,
this can balloon the JSON blob to megabytes, causing the main window and
the worker window to become unresponsive.
The UI doesn't need to know about IMAP bookkeeping internals, so
serialize the sync progress instead of the sync state. This has the
advantages that (1) we don't need to worry about future keys added
to the syncState being large and (2) when we add Exchange support
we already have an abstraction for sync progress.
Test Plan: manual
Reviewers: juan, mark, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D3817
Summary:
This will prevent us from showing error messages to the user when we
can automatically recover from the error
Test Plan: manual-- throw error from syncback task, check expected results
Reviewers: evan, mark, spang
Reviewed By: spang
Differential Revision: https://phab.nylas.com/D3812
Summary:
There are 2 types of IMAP errors that need to be treated as retryable. See code
comments as to why.
Test Plan: manual
Reviewers: khamidou, evan, spang
Reviewed By: spang
Differential Revision: https://phab.nylas.com/D3811
We added new presets and plugins to the k2 .babelrc but didn't add the
respective dependencies in our package.json, so the build was failing.
Add the required dependencies, and make sure that `build-n1-cloud.js`
uses the correct babel presets and plugins
Summary: This was broken because babel could not compile the `super` keyword
Test Plan: manual
Reviewers: evan, mark, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D3804
Summary:
EnsureMessageInSentFolder also needs to sync the sent folder to fetch
the uid of the newly moved message (like `MoveThreadToFolder` does)
Test Plan: manual
Reviewers: halla, mark, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3803
Summary:
In `FetchNewMessagesInFolder`, sometimes we haven't synced anything in the folder
we are trying to fetch new messages in. Previously this would just throw
an error, now we properly check if we have a fetchedmax, and if not just
run a normal fetch.
Also, when the target folder box was already open, we were not fetching the /latest/ box status to check the latest uidnext value, so we would skip fetching new messages when in fact there were new messages to fetch
(This can happen for example when moving a sent message to the Sent
folder before we've started syncing the Sent folder)
Test Plan: manual
Reviewers: halla, mark, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3802
Summary: see title
Test Plan: manual, deploy to staging, check that it works
Reviewers: evan, spang, tomasz, khamidou
Reviewed By: tomasz
Differential Revision: https://phab.nylas.com/D3800
