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
Summary:
This commit modifies the api of NylasAPIRequest to /not/ take `success`
or `error` callback options at all, and only returns a Promise which you
can `then` and `catch` to handle the api response.
The fact that it returned a promise, and /also/ took `success` and
`error` callback options made it really confusing to use.
Additionaly, when using the callbacks intead of a promise, any errors
would be unhandled and reported to Sentry because even though the `error`
callback was being passed, the promise returned by `run()` still rejected and
no one was handling that reject, so it reached the `unhandledRejection` event
listener. This is undesirable because if you passed an `error` callback, it
means that you intended to handle it.
An example of this is calling the clearbit API, which will more often than
not return a 404, and even though we had an error handler which ignored the 404,
it still unecessarilly reported to Sentry, flooding it with events
Test Plan: manually check all updated codepaths still work
Reviewers: halla, spang, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3869
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:
During the _onDOMMutated callback, we disconnect the mutation observer,
call some other callbacks, and then reconnect the mutation observer. If
we threw an error during the callbacks before reconnect the mutation
observer we would never get any more callbacks when the user changed
things in the composer, causing us to stop saving updates to drafts
(among other things). The fix is to just make sure that we always
reconnect the mutation observer using a finally clause.
Test Plan:
Run locally, make sure drafts are no longer truncated after spelling
correction (which was triggering an error to be thrown)
Reviewers: juan, evan, spang
Reviewed By: spang
Differential Revision: https://phab.nylas.com/D3864
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()
Depends on D3861
Test Plan:
Download latest prod build from nylas.com
Remove both ~/.nylas-mail and ~/.nylas-dev
Start up downloaded Nylas Mail, connect accounts.
Run `npm start` (which enables --dev)
Connect account to dev mode
Ensure both clients are syncing and can send/receive mail
Reviewers: khamidou, halla, mark, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D3862
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:
Snooze is back in the mainline but not yet ready for primetime. We need to not show the popover if it's not enabled, which is what this diff does. It should be pretty simple to revert it once snooze has officially shipped.
Fixes T7791.
Test Plan: Tested manually.
Reviewers: juan, evan
Reviewed By: evan
Maniphest Tasks: T7791
Differential Revision: https://phab.nylas.com/D3855
Summary:
This adds the "You've reached max features" modal in N1.
http://g.recordit.co/9O7R0mLlXE.gif
Test Plan:
1. Pull latest nylas/cloud-core and start Billing site:
```
cd cloud-core
vagrant up
vagrant ssh
cd /vagrant
bin/setup-up-feature-usage
bin/launch
```
2. Blow away ~/.nylas-mail (err backup your old one first)
3. Restart N1
4. Before logging in, edit `~/.nylas-mail/config.json`
- set env to "local"
- remove `thread-snooze` from the list of `disabledPlugins`
5. `cd /nylas-mail/src/k2` and run `npm start`
6. Restart N1 and create accounts & log in
Reviewers: khamidou, juan, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D3846
Summary:
FTS tables don't support indices, so doing UPDATEs and DELETEs based on
the `content_id` was very slow on large FTS tables. Fortunately, it seems
that `UPDATE`s and `DELETE`s based on the `rowid` are much faster, so now we
store that info hanging off the searchable models. Also fixes a random bug
where after reaching the `MAX_INDEX_SIZE` we would clear the Thread search
index on startup.
Test Plan: Run locally, time how long it takes to delete when receiving new mail
Reviewers: spang, evan, juan
Reviewed By: evan, juan
Differential Revision: https://phab.nylas.com/D3847
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:
It's slow, so only do it if we've never seen the Account before. This
fixes jank that would happen somewhat randomly. It was especially noticeable
if the user had a lot of Accounts (and therefore a lot of passwords to
load).
Test Plan: Run locally, verify that we only load new Accounts
Reviewers: spang, evan, juan
Reviewed By: evan, juan
Maniphest Tasks: T7766
Differential Revision: https://phab.nylas.com/D3848
Summary:
We don't want to overwhelm a user with a bunch of bings and bongs when
they open their laptop after a long weekend. This diff takes a
relatively simple approach by debouncing the notification sounds every 5
seconds.
Test Plan: Run locally, make sure we still get notified but not too much.
Reviewers: juan, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3841
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:
This commit converts list-tabular to JS, and in the process re-adds shouldComponentUpdate which had been previously removed (D3837).
This time, shouldComponentUpdate will correctly check if actual data to render has changed, as opposed to checking if `itemPropsGenerator` had changed.
Test Plan: manual
Reviewers: halla, spang, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3839
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:
We recently added a `shouldComponentUpdate` method to ListTabular to
improve scrolling performance. However, this was preventing ListTabular
from updating the selected state of its items when we selected new
threads via cmd or shift keys.
This occurred because instead of passing data as props for each of ListTabular's
items, we are passing a function that calculates the props for each
item, so when we diffed old props and new props in shouldComponentUpdate,
given that the function reference never changes, we thought that the props
hadn't changed and prevented a re render, when in fact the props for each item
generated by that function might actually change.
Unfortunately, we can't use shouldComponentUpdate in this component
given how it is structured
Test Plan: manual
Reviewers: evan, spang, halla
Reviewed By: spang
Differential Revision: https://phab.nylas.com/D3837
