Summary:
This commit makes it so `resetEmailCache` works as expected, i.e. it
removes all databases, without forcing the user to re sign-in to their
accounts or NylasID
Previously, this method removed the database without removing the
accounts, left users in an un-authed state that was hard to recover
from. This was fixed in D4212 which makes sure that when we get a new
identity, sync and deltas are restarted
However, resetEmailCache would still force you to log in to yoru NylasID
because it was deleted from the database. However, if we reuse the
command `application:relaunch-to-initial-windows` instead of manually
deleting the database, we can relaunc the app while preserving the users
NylasID session, so they don't have to sign back in manually.
Test Plan: manual
Reviewers: evan, spang, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4215
Summary:
This prevents the app from entering a restart loop when there's no
identity.
Specifically, when a user logs out of their identity, or when she resets the
email cache, or any other scenario that leaves the app without an
identity but with accounts added, the sync loop (and deltas) will start
without an Identity.
This will cause NylasAPIRequest to throw an error that
forces the user to close the app. When the app restarts, sync will start
again without an identity, and the user will be forced to close the app
again, and so on and so forth for the rest of eternity
Relevant error:
https://github.com/nylas/nylas-mail-all/blob/master/packages/client-app/src/flux/nylas-api-request.es6#L165-L174
Additionaly, this makes sure that after resetting the email cache, the sync
process starts when the identity becomes available
This solves T7989
Test Plan: manual
Reviewers: evan, spang, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4212
Summary:
Previously, while resetting the email cache, we would try to stop sync
and just wait for an arbitraty amount of time for it to stop and the
proceed to blow away the database.
This commit makes it so we correctly wait for sync to stop, and then we
blow away the db. It adds a timeout anyway in case sync is stuck and we
can't stop it
Depends on D4207
Test Plan: manual
Reviewers: evan, spang, halla
Reviewed By: spang, halla
Differential Revision: https://phab.nylas.com/D4209
Summary:
We no longer keep `this._accounts` state, which was being accessed
inside `_resetEmailCache`
Test Plan: manual
Reviewers: evan, spang, halla
Reviewed By: spang, halla
Differential Revision: https://phab.nylas.com/D4207
Summary:
build-client, aka packaging, aka bundling is now separate from uploading.
This is both to compartmentalize our tasks a bit more and so we can add a
non-grunt windows task inbetween packaging and uploading.
No more heavily-overloaded PUBLISH_BUILD flag.
Added SIGN_BUILD flag instead.
No more TRAVIS and TRAVIS_PULL_REQUEST flag.
Test Plan: Manual
Reviewers: halla, juan, spang
Reviewed By: juan, spang
Differential Revision: https://phab.nylas.com/D4208
Summary:
This removes specs from the build production version.
One of the major issues with the windows build was that one of the package
specs included a .node compiled file that the codesigner attempte to sign,
but couldn't find for some reason.
In testing, removing the specs from the build prod version fixed this.
While we could just remove the one offending test, I think we should just
remove the whole suite from the build version. That'd save in the number
of files we have to ship, save download time, save build time, and have
less chance for some symlinked issue failling a build in the future.
Test Plan: Manual
Reviewers: khamidou, halla, juan, spang
Reviewed By: juan, spang
Differential Revision: https://phab.nylas.com/D4206
Summary:
It always drives me crazy when folks use "setup" (no space) as a verb,
because the verb version has a space in it.
Also remove the word "Setup" from the displayName for custom IMAP accounts
because it doesn't make sense for how we use this field, e.g.
a8f36b88d9/packages/client-app/internal_packages/onboarding/lib/page-account-onboarding-success.jsx (L27)
will display "Sucessfully connected to IMAP / SMTP Setup!" with the old
wording when you successfully auth a custom IMAP account.
Test Plan: manual
Reviewers: evan, juan, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4199
Summary:
Dunno if I'm the only one having this problem, but I spent at least an
hour over the last two days wondering why hapi just sits there and hangs
in the Joi validation step when I copy-paste calls to /auth from the
developer bar to my terminal. It turns out that if you fail to send the
correct Content-Type when sending a JSON payload, Joi just hangs
forever. (WTF!)
Test Plan: manual
Reviewers: halla, evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4204
Summary:
We were always incrementing the queue length and resolving the promise
in the constructor :-/
Test Plan: Run locally, make sure queue doesn't start rejecting messages
Reviewers: evan, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4198
Summary:
Previously, n1 cloud auth errors were stuffed into an account's
syncState. The problem with that is that the sync loop manages that
state, and as long as the sync loop is running it will set that state to
running. However, it might be the case that the sync loop is running but
we can't connect to n1Cloud, so even though we would set the `syncState`
to `n1_cloud_auth_failed`, the sync loop would just set it back to
'running', and the user wouldn't see the error notification indicating
that it can't connect to n1Cloud
This commit makes it so we keep tracj of the auth failure state for
n1Cloud in a separate field, and makes sure that the error notification
component shows that error.
Test Plan: manual
Reviewers: mark, spang, evan, halla
Reviewed By: evan, halla
Differential Revision: https://phab.nylas.com/D4172
Summary:
On rare occasions, the api error would not have a body, and we would try
to access the message prop on the body, which would in turn throw an
error.
This commit prevents this error, and adds better messaging to the sentry
report
Test Plan: manual
Reviewers: mark, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4194
Summary:
This commit adds a new perf metric for inline composer times.
Additionally, it consolidates the timer logic for all other types of draft
creation (mailto links, dropping a file in the app dock icon, etc).
Test Plan: manual
Reviewers: halla, evan, mark
Reviewed By: evan, mark
Differential Revision: https://phab.nylas.com/D4186
Summary:
Previously we limited it to 3. Gmail supports up to 15, many IMAP
servers support 10 by default. So let's bump it for those folks.
Test Plan: Run locally, verify things still work.
Reviewers: evan, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4175
Summary:
Errors in the MessageProcessor were causing sync to get stuck
occasionally. This diff refactors queueMessageForProcessing and friends
so that they're more robust to errors during promise construction.
Test Plan: Run locally
Reviewers: juan, spang, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4190
Summary:
Sequelize can sometimes return promises that will never resolve or
reject. We can wrap the promises we get back from sequelize in a
bluebird promise which gives us the ability to timeout these abandoned
promises. This way, we can track down issues in sequelize as well as
unblocking stuck sync loops.
Test Plan: Run locally, verify that timeouts occur
Reviewers: juan, evan, spang
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4192
Summary:
This commit adds error handling to the sync-loop's `onSyncError` and
`scheduleNextSync`.
These functions generally don't fail, as they are in the `catch` and
`finally` blocks respectively of the sync loop. But as we've seen in
D4152, the datbase can sometime error if it's in a bad state. If it
errors inside these functions, we will never schedule the next run of
the sync loop.
Depends on D4152
Test Plan: manual
Reviewers: evan, spang, mark, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4153
Summary:
Before this commit, we would call .close() onError and when the
connection closed. We also closed the connection onError, even though
NylasLongConnection had also closed it, so we ended up calling close a
bunch of times. This would cause us to set a bunch of timeouts to retry
unecessarilly.
This commit makes it so we ensure there's only one retry timeout and
consolidates the logic that calls .close() so we don't call it so many
times unnecessarily
Test Plan: manual
Reviewers: spang, mark, evan
Reviewed By: mark, evan
Differential Revision: https://phab.nylas.com/D4166
Summary:
This commit consolidates the `DeltaConnectionStatusStore` and the
`DeltaConnectionStore` which kept track of very similar state and made
sense to be the same store (as per feedback in D4118#77647)
Given that this state needs to be available app-wide for plugins to
query the status of delta connections, `internal_packages/deltas` was
removed (given that it only activated that store), in favor of having the
unified store inside `src/flux/stores` and available via `nylas-exports`
The `deltas` package also contained some contacts-ranking code, which is
no longer in use until we restore that fetaure, so I created a
`internal_packages/contact-rankings` which contains this unused code for
now.
Test Plan:
manually open, close, end delta connections, verify that I'm getting
correct results. unit tests to come
Reviewers: halla, spang, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4145
Summary:
This DRY's up code that use these status codes
Depends on D4130
Test Plan: manual
Reviewers: spang, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4140
Summary:
When reporting error inside `IdentityStore._fetchIdentity`, we would
pass null as the `extra` parameter for `reportError`. This would cause
us to try to assign something to null and throw an error while reporting
the error.
(Even though `extra` has a default value of `{}` (see
declaration of `NylasEnv.reportError`), passing null will be interpreted
as a valid arg and it wont get assigned the default value)
Thankfully, the second error /is/ reported to sentry: https://sentry.io/nylas/nylas-mail/issues/230589717/
Test Plan: manual
Reviewers: mark, halla, spang, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4150
Summary:
This error is can flood Sentry pretty badly: https://sentry.io/nylas/nylas-mail/issues/230629801/
My initial thought was to rate limit it, but rate limiting wouldn't
do any good because when we get that error we destroy the databases and
restart the app, and we will loose the in-memory rate limiting data.
The real fix, and reason why Sentry is being flooded with this error by
a single user, is that once you encounter this error the app will enter
a restart loop that constantly throws this error. The reason being that
we weren't properly awaiting for the K2 account databases to be dropped
before closing the app, so on restart, the database would still be
malformed.
The fix is to properly `await` for the database drops
Test Plan: manual
Reviewers: spang, mark, halla, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4127
Summary:
This is in preparation for removing the ability of the IMAP pool to
automatically handle timeout errors.
Test Plan: Run locally, verify file download still works
Reviewers: juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4167
Summary: In preparation for removing timeout handling from IMAPConnectionPool.
Test Plan: Run locally
Reviewers: juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4168
Summary:
We had this weird nesting where we would keep the IMAPConnection open
beyond the scope of the connection pool and return the created
Observable. This diff inverts that relationship and flattens out some of
the indentation, making the code easier to read. This is in preparation
for refactoring the IMAPConnectionPool to no longer automatically handle
timeouts.
Test Plan: Run locally, make sure search still works
Reviewers: juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4165
Summary:
This is not a bug. Maaaybe we want to report this to MixPanel for individual
user debugging (how do we do that?), but it doesn't make sense to report this
error to Sentry.
Example:
https://sentry.io/nylas/nylas-mail/issues/230530818/
Test Plan: yolo
Reviewers: mark, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4130
Summary: Misspelled INPROGRESS-NOTRETRYABLE :-(
Test Plan: Run locally, make sure things get cancelled
Reviewers: juan
Differential Revision: https://phab.nylas.com/D4163
Summary:
If you have the console open and are looking at the Source panel or
something, this will constantly bring you back to the Console panel. Won't
do that anymore with this chec
Test Plan: manual
Reviewers: spang, mark, halla, juan
Reviewed By: mark, halla, juan
Differential Revision: https://phab.nylas.com/D4162
Summary:
Errors weren't getting reported because of a circular reference causing
JSON.stringify to fail.
We can't add the whole API `response` object onto the error object,
because sometimes (like when the delta connection errors with a 401), the
`response` object is circular. We only use it to the extract the status
code anyway and it's a huge object. This diff excludes them from the
APIError objects.
We also have code to recover just in case JSON.stringify errors for some
other weird reason. It'll still attempt to report the error prefixed with
`Recovered Error`
Finally, we used to spit only the `error.stack` to the console. This meant
what we saw in the console didn't properly include the error messages
being incredibly not useful.
It's better to console.error the whole error object since that'll more
nicely display on the inspector console
Test Plan: manual
Reviewers: juan, spang, mark, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4154
Summary:
Previously if we got back a huge number of unknown UIDs from IMAP search
we would try to sync all of them at once. This could lead to hanging the sync
loop trying to download tons of messages. This diff limits the UIDs we're
willing to sync per task to 500 and splits each task up into chunks of 25
messages so that we don't try to download all of them at once. If we need
to sync more than 500 uids then at the end of the syncback task it will
queue another task to run the next time the sync loop rolls back around.
Test Plan:
Run locally, verify that we gracefully handle various situations
including cancelling during the syncback task, cancelling between syncback
tasks, huge numbers of results, etc
Reviewers: evan, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4142
Summary:
We don't want to constantly retry to sync a bunch of random UIDs if
we're not longer in the search perspective so make these syncback tasks
not retryable.
Test Plan: Run locally, verify that we don't retry
Reviewers: spang, evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4149
Summary:
This commit bumps the version of sequelize to point to the latest
version in our fork. This version bump also includes latest updates from
upstream sequelize since the last time we bumped the version.
Notably, it includes a critical fix that will prevent the sync
loop from getting stuck on rare occasions.
Specifically, sequelize's `Query.prototype.run` could in some cases return a
Promise that would /never/ resolve or reject, effectively halting the
execution of any code that was waiting for that promise. This was due to
a lack of error handling inside a the query's `afterExecute` function; if an
error was thrown there, the enclosing Promise would never reject, and
the error would just remain uncaught. An example of such an error that
could cause this scenario is: T7742 - https://sentry.io/nylas/nylas-mail/issues/230016155/
Sync getting halted in this way can produce a variety of user facing
bugs like not being able to send or it taking an absurd amount of time
(hours), tasks never finishing or taking an absurd amount of time to
complete, new mail not arriving, among others
The commit that fixes this is: 0deeda9e1a
The full set of changes introduced by this version bump are: https://github.com/nylas/sequelize/compare/nylas-3.30.0...nylas-3.40.0
**Note:**
It is important to note that sequelize might still halt the sync loop if
there are other places in the code that can return Promises that never
resolve or reject under certain circumstances. I can consistently reproduce this
scenario when an error is thrown inside `afterExecute`, and I've seen it
happen in the wild, but I've also ran into this type of sync loop halting (db
promises never resolve) without any sequelize errors being thrown, which
suggests that there are other places in the sequelize code that might end
up returning a Promise that will halt sync.
Unfortunately, we don't have a good way to detect and report when this
happens yet, but we are adding one in upcoming diffs in order have data
on how many people are running into this, and/or if this patch completely
fixes the issue. Otherwise, we'd need to audit sequelize's code.
Should resolve T7837 and T7767
Test Plan: manual
Reviewers: spang, mark, halla, khamidou, evan
Reviewed By: evan
Subscribers: mg, tomasz
Maniphest Tasks: T7767, T7837
Differential Revision: https://phab.nylas.com/D4152
Summary:
We want to enable code to control whether fetching is throttled or not.
We basically only want to throttle syncing the historical message
archive. New messages, initial inbox sync, and unknown search results
should not be throttled. Also some drive-by code refactoring.
Test Plan: Run locally, verify that things still work
Reviewers: spang, evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4126
Summary:
This commit splits apart the `AccountDeltaConnection` class, which was
in charge of listening to both cloud /and/ local deltas by way of an
artificial interface, `DeltaStreamingInMemoryConnection`.
Splitting this into 2 modules with separate responsibilities will hopefully
make this code easier to reason about and reduce some cruft and unnecessary
indirection.
Specifically, this commit makes it so:
- `DeltaConnectionManager` is only in charge of starting and ending `DeltaStreamingConnection`s, which are solely in charge of listening to deltas from the cloud api
- `LocalSyncDeltaEmitter` no longer unnecessarily emits events for the `deltas` package to listen to but rather directly processes and saves those deltas from the K2 db to edgehill.db
- `LocalSyncDeltaEmitter` is also in charge of keeping track of the latest received cursor, under its own JSONBlob key in edgehill.db. This migrates localSync cursors saved under the old key.
- `LocalSyncDeltaEmitter` is now instantiated and managed from within the `SyncProcessManager` as opposed to the `SyncWorker`. Apart from removing extra state from the `SyncWorker`, this removes dependencies on the client-app environment from the sync-worker.
- `DeltaStreamingInMemoryConnection` and `AccountDeltaConnection` are now gone
(Sorry for the big diff! This one was a little hard to split up without landing something broken)
Depends on D4121
Test Plan: manual + unit tests planned in upcoming diff
Reviewers: halla, mark, evan, spang
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4122
Summary:
This is to make it available to all plugins in the app
Depends on D4120
Test Plan: manual
Reviewers: halla, mark, spang, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4121
Summary:
This commit completely refactors `DeltaStreamingConnection`, notably
introducing the following changes:
- Right now, `AccountDeltaConnection` establishes both delta connections to the cloud api and to the `client-sync` database (K2). This class is meant to disapper in favor of splitting into two different classes meant for syncing with the n1Cloud api and the local database. Specifically, `DeltaStreamingConnection`'s only responsibility is now to connect to the n1Cloud API and establish an http streaming connection for metadata deltas, etc. This class no longer unecessarily inherits from `NylasLongConnection`, which removes a lot of unecessary callback indirection.
- The statuses of the n1Cloud delta streaming connections are now stored in as JSONBlobs in edgehill.db under new keys. This commit ensures that the data is correctly migrated from the old key (`NylasSyncWorker:<account_id>`).
- The `DeltaStreamingConnection` now correctly retries when closed or errors. This logic previously existed, but was removed during the K2 transition: https://github.com/nylas/nylas-mail/blob/n1-pro/internal_packages/worker-sync/lib/nylas-sync-worker.coffee#L67
- Delta connection retries now backoff using the `ExponentialBackoffScheduler`
- Attempt to restore the delta connection whenever the app comes back online
Depends on D4119
Test Plan: manual + planned unit tests in upcoming diff
Reviewers: halla, mark, evan, spang
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4120
Summary:
For legacy reasons, NylasLongConnection did not take the `onResults`
callback as an option, but rather it was a method you had to call on the
connection, which took the callback, and which you had to call after
instantiating the connection. This was an annoying and clunky interface.
This commit makes it so NylasLongConnection takes an `onResults` option
as one would expect it to, and updates any references to the old
interface
Depends on D4118
Test Plan: manual
Reviewers: halla, mark, spang, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4119
Summary:
Turns out, NylasSyncStatusStore doesn't need to concern itself with the
status of delta connections.
This is legacy cruft from the days when this store used to provide the
online status of the app based on the delta connections. Now we have an
OnlineStatusStore, so we don't need this at all
Instead of stuffing that state in that store, this commit adds a small
DeltaConnectionStatusStore to provide an easy way to listen to changes
on the status of delta connections. The only part of the app concerned
with that state is the DeveloperBarStore in order to render the green
circles for delta connections in the worker window.
Depends on D4117
Test Plan: manual
Reviewers: mark, halla, spang, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4118
Summary:
If an error is thrown while opening an imap box, we need to make sure to
set the `_isOpening` back to false
Test Plan: manual
Reviewers: mark
Reviewed By: mark
Differential Revision: https://phab.nylas.com/D4134
Summary:
In many cases when send fails, the server settings may be incorrect.
Send this data to Sentry with the original error in order to help us
diagnose.
I think the grouping will only contain settings for a single user
getting the same error, but should still be good enough to do further
investigation, e.g. https://sentry.io/nylas/nylas-mail/issues/230529222/
We can always go look up the requests in the n1Cloud logs if more info is
needed.
Test Plan: yolo
Reviewers: mark, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4131
Summary:
We have a bunch of different endpoints and it'd be helpful to know which
endpoints are failing, not just which status codes are being thrown. e.g. while
trying to reproduce send failures, I've been able to cause /auth to fail with a
504, but I couldn't tell which endpoint was failing with the 504 until I added
this extra differentiator.
Test Plan: manual
Reviewers: mark, juan
Reviewed By: mark, juan
Differential Revision: https://phab.nylas.com/D4128
Arc land messed up and landed a not fully merged branch. (Seriously – I
had merged a copy of my branch with master to see how easy it would be.
Because I didn't want to merge the whole thing, I blindly committed my
changes and switched back to my real branch). To my great surprise, arc
decided to use the wrong branch when landing it.
Original commit message:
Summary:
Finally, here it is! Send later, with support for open tracking but
without support for attachments yet. It took me some time to find the
right way to do things.
**The send later dilemna**
There's two ways we could handle send later:
1. do everything on the client
2. process the message in the cloud
1. is very tempting because it would make the cloud server very
simple. Unfortunately, it has some serious limitations, for example,
setting the "Date" message header. That's why I chose to go with 2. When
a user presses the "Send Later" button, we save the open/link tracking
metadata and fills in all the required fields. I added a custom endpoint
to the K2 API to do this, `/drafts/build`. After that, we save the JSON
contents of the message as metadata.
When we process metadata, we simply create a MIME message from the
JSON and send it.
**Limitations**
Right now, send later doesn't support and attachments. There's also
some minor code duplication which needs to be refactored away.
Test Plan: Tested manually. Checked that regular send still worked, too.
Reviewers: mark, spang, halla, juan, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4054
Summary:
Finally, here it is! Send later, with support for open tracking but without support for attachments yet. It took me some time to find the right way to do things.
**The send later dilemna**
There's two ways we could handle send later:
1. do everything on the client
2. process the message in the cloud
1. is very tempting because it would make the cloud server very simple. Unfortunately, it has some serious limitations, for example, setting the "Date" message header. That's why I chose to go with 2. When a user presses the "Send Later" button, we save the open/link tracking metadata and fills in all the required fields. I added a custom endpoint to the K2 API to do this, `/drafts/build`. After that, we save the JSON contents of the message as metadata.
When we process metadata, we simply create a MIME message from the JSON and send it.
**Limitations**
Right now, send later doesn't support and attachments. There's also some minor code duplication which needs to be refactored away.
Test Plan: Tested manually. Checked that regular send still worked, too.
Reviewers: mark, spang, halla, juan, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4054
Summary:
Keeping a sourceMapCache uses a great deal of memory. Don't do it if we
don't have to.
Test Plan: Run locally, verify that we no longer generate a sourceMapCache
Reviewers: evan, juan, spang
Reviewed By: evan, juan, spang
Differential Revision: https://phab.nylas.com/D3964
Summary:
We never want to ship the last release we'll ever ship.
This can happen if the new version's autoupdater is broken.
To test this we upload to a version n+1 higher than the current one.
Test Plan: manual
Reviewers: juan, spang, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4125
Summary:
This diff adds a timeout parameter to queueMessageForProcessing that can
be used to override the CPU-throttled-by-default timeout used by the
message processing queue. The FetchMessagesInFolder sync task now
overrides with a timeout of 0 when it is queuing specific UIDs that it
has downloaded. The thinking here is that if we've requested a specific
set of UIDs, it's very important to sync those quickly as this indicates
a higher priority (e.g. initial inbox sync, search result syncing, etc).
Test Plan: Run locally, verify that initial sync and search syncing run faster
Reviewers: spang, evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4114
Summary:
When searching using IMAP/Gmail commands we sometimes get back UIDs for
messages that we have yet to sync. Previously we would just ignore these
results, which would decrease the quality search results for quite some
time during initial sync. This diff enables us to eagerly sync the unknown
messages we get back from the provider by creating a syncback task which
interrupts the sync loop and runs a sync task for the unknown UIDs.
Test Plan: Run locally, verify that we sync unknown messages
Reviewers: spang, evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4101
Summary:
This commit adds a new option to the `reportError` interface to allow
to rate limit specific kinds of errors that are known to be reported a
lot if they fall in a loop for example.
For now, it only allows to impose a fixed reporting rate per hour for a specific
error, and will not report any errors past that rate before an hour has
passed.
Depends on D4099
Test Plan:
manually check that it reports and does not report errors based on the rate
limit data passed
Reviewers: khamidou, mark, evan, halla, spang
Reviewed By: evan, halla, spang
Differential Revision: https://phab.nylas.com/D4100
Summary:
Add better messaging and logging to the console.
Depends on D4098
Test Plan: manual
Reviewers: evan, khamidou, spang
Reviewed By: spang
Differential Revision: https://phab.nylas.com/D4099
Summary:
Given that NylasAPIRequest reports all APIErrors for requests that
return 5xx, we were double reporting access token errors, which caused 2
groups in sentry and makes it difficult to track.
This commit removes the extra reporting for that error in the sync loop
Test Plan: manual
Reviewers: evan, khamidou, spang
Reviewed By: spang
Differential Revision: https://phab.nylas.com/D4098
Summary:
Given that we backoff exponentially in the sync loop when we encounter
RetryableErrors (e.g. network errors), if the app goes offline and
reaches the maximum retry backoff of 5 minutes, and if then the app comes
back online, in the worst case, the sync loop will idle for 5 minutes before trying to
sync, which is undesireable.
To mitigate this, everytime we come online we can wake all of the sync
workers. This shouldn't have any adverse effects
Test Plan: manual
Reviewers: evan, mark, spang
Reviewed By: spang
Differential Revision: https://phab.nylas.com/D4097
Summary:
This is mostly to clean up the logic here a little bit.
Specifically:
- The backoff when encountering retryable errors is now truly exponential
- When encountering a permanent error, we back off for a minute
- If we don't encounter consecutive RetryableErrors, we clear the exponential backoff. Previously, we would just clear it when sync completed without errors.
Test Plan: manual
Reviewers: khamidou, spang, evan, mark
Reviewed By: mark
Differential Revision: https://phab.nylas.com/D4086
Summary:
This commit makes it so we /always/ catch errors thrown in the request
established inside `NylasLongConnection`. Specifically, it catches an
uncaught `socket hang up` error which is flooding Sentry unnecessarily
(https://sentry.io/nylas/nylas-mail-old-1030/issues/213178758/).
This error is thrown when the long connection is manually ended before
any data was received from the request (which happens most times during
search when you clear the search query).
The problem was that when we end the long connection, we remove all event listeners
from the request, and /then/ the error is thrown, at which point there is no error
handler to catch it. The solution is simply to always have an error handler attached to
the request object.
This commit also adds extra error handling that seems to have been missing.
Test Plan:
manually check that delta connection is still working, and that error
is no longer thrown when you search and then immediately clear the search
Reviewers: mark, evan, spang
Reviewed By: spang
Differential Revision: https://phab.nylas.com/D4096
Summary:
We want to listen to both `abort` and `aborted`
- `abort` happens when we manually abort a request client side, so we don't want to report this to sentry
- `aborted` happens when the server aborts the request, so we /do/ want to report in this case
Test Plan: manual
Reviewers: spang, mark, evan, halla
Reviewed By: evan, halla
Differential Revision: https://phab.nylas.com/D4073
Summary:
Previously we used the Gmail REST API to get search results. This API
returns the X-GM-MSGID which is different from the folder UID. Gmail also
offers the X-GM-RAW extension for IMAP's UID SEARCH command which allows
us to pass through the raw search query. This enables us to vastly
simplify the GmailSearchClient by having it subclass ImapSearchClient
and override a few select methods. Yay!
Test Plan: Run locally, verify that Gmail search queries still work
Reviewers: evan, spang, juan
Reviewed By: spang, juan
Differential Revision: https://phab.nylas.com/D4094
Summary:
Previously for IMAP we just grabbed the search text and fed it into a TEXT
query. Now we have a proper backend that generates the appropriate
search criteria according to the IMAP spec. Important to note that we
don't support 'in:' yet, which is complicated due to the way that IMAP
search is scoped to the currently selected folder.
Test Plan: Run tests, run locally and verify IMAP search still works.
Reviewers: evan, juan, spang
Reviewed By: juan, spang
Differential Revision: https://phab.nylas.com/D4071
Summary:
- Add an icon to threads that have reminders set
- Show the pending reminder message header in all perspectives, rather than
just the Reminders perspective
- Add the ability to clear the reminder from the message header
- Condense the message header text
Test Plan: manual
Reviewers: evan, juan
Reviewed By: evan, juan
Subscribers: sdw
Differential Revision: https://phab.nylas.com/D4052
Summary:
Gleb mentioned that we need the sample size of search events to be
bigger, so let's report all search events
Test Plan: manual
Reviewers: evan, gleb, mark
Reviewed By: mark
Differential Revision: https://phab.nylas.com/D4077
Summary:
If it's expensive for us to SELECT a folder (which happens a lot for
large gmail accounts), we should try to sync more messages so that we
don't waste a ton of time SELECT-ing during initial sync. This speeds up
initial sync quite a bit.
Test Plan: Run locally, verify batches are properly computed
Reviewers: evan, juan, spang
Reviewed By: juan, spang
Differential Revision: https://phab.nylas.com/D4050
Summary:
This changes the scheduler from `setImmediate` to `setTimeout`. The HUGE
difference is that Chrome's async stack traces works with `setTimeout` but
does NOT work with `setImmediate`. These two functions are adjacent to
each other in the Node event loop, so nothing should depend on the change
in ordering.
Test Plan: manual
Reviewers: mark, spang, halla, juan
Reviewed By: spang, halla
Differential Revision: https://phab.nylas.com/D4068
Summary: Bluebird changed the way you call `longStackTraces`. This uses the latest
Test Plan: manual
Reviewers: spang, halla, juan
Reviewed By: spang, halla
Differential Revision: https://phab.nylas.com/D4067
Summary:
We no longer optimistically fetch messages when we load their bodies. We
always get the full message with bodies returned via the in memory delta
stream
Test Plan: manual
Reviewers: spang, halla, juan
Reviewed By: spang, halla, juan
Differential Revision: https://phab.nylas.com/D4066
Summary:
We no longer need to use the `returnsModel` param since we get all of our
models through the in memory delta stream. There were a ton of places
unnecessarily passing `returnsModel: false` when it defaults to false in
the first place
Depends on D4057
Test Plan: manual
Reviewers: halla, spang, juan
Reviewed By: spang, juan
Differential Revision: https://phab.nylas.com/D4065
Summary:
We no longer use the `ensureOnce` bit.
Depends on D4057
Test Plan: manual
Reviewers: spang, halla, juan
Reviewed By: halla, juan
Differential Revision: https://phab.nylas.com/D4064
Summary:
This removes the last reference to a now unused `beforeProcessing` feature
of the Nylas API Request.
Depends on D4057
Test Plan: manual
Reviewers: spang, halla, juan
Reviewed By: halla, juan
Differential Revision: https://phab.nylas.com/D4063
Summary:
The old NylasAPIRequest helper class used to do all sorts of run-time
requiring to attempt to notify of auth errors when 401s and 403s came up
in a request. We now move that logic to the AccountStore where it belongs.
Depends on D4057
Test Plan: Manual
Reviewers: spang, halla, juan
Reviewed By: halla, juan
Differential Revision: https://phab.nylas.com/D4060
Summary:
This greatly refactors the NylasAPIRequest object to be more single
purpose, easier to read, async-ified.
It also fixes issues where long stack traces weren't being reported bcause
`reportError` was being called before the error had time to make it
through Bluebird's rejection handler (which adds back in the long stack
traces).
This also fixes an issue where ignorable errors (like 404s,
ESOCKETTIMEDOUT) were being reporting to Sentry.
This also fixes subtle conditions where the request run would throw mixed
error types (sometimes regular `Error`s, sometimes `APIError`s).
The old class used to handle logging people out on 401s. This is now moved
to the AccountStore in a different diff.
We've also deprecated the `returnsModel` param in a separate diff.
This deprecates the `PriorityUICoordinator` since we no longer need to
worry about frequently making expensive API requests to pull down data
like we used to in the old cloud-based API.
This deprecates `ensureOnce` param in a separate diff.
Test Plan:
Manually boot app. Check regular 404s don't throw. Check requests get
through. Check special ESOCKETTIMEDOUT type errors are properly handled
Reviewers: juan, spang, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4057
Summary:
Previously we were using the raw visitors that were confined to the flux
attributes directory. We're going to add more search query backends, so this
is mostly just moving things to a new, more general place.
Test Plan:
Run locally, verify parser specs still work, verify in-app search
still works.
Reviewers: spang, evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4053
Summary:
Isomorphic-core should be functional outside of the client environment
Depends on D4056
Test Plan: Run the test suite
Reviewers: evan, spang, juan
Reviewed By: spang, juan
Differential Revision: https://phab.nylas.com/D4062
Summary:
Move the base Jasmine spec runner into isomorphic-core to prevent
code duplication. Jasmine will look for the config file relative to
the directory it's being run in though, so we need to symlink the
config file into each package that will need it.
Test Plan: Run tests once the suites are integrated
Reviewers: evan, spang, juan
Reviewed By: spang, juan
Differential Revision: https://phab.nylas.com/D4056
Summary:
Convert the isomorphic-core specs to Jasmine 1 and symlink them into
client-app/internal_packages so they are run as part of the client
test suite.
Test Plan: Ran the suite with fdescribes in iso-core to ensure they were being called properly
Reviewers: evan, spang, juan
Reviewed By: spang, juan
Differential Revision: https://phab.nylas.com/D4055
Summary:
Disabling verbose logging in production builds just makes it harder to help our
users help us fix their problems.
Test Plan: run app without --dev
Reviewers: mark, evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4070
Summary:
We detect when a thread is removed from the thread-list by listening to
`Actions.threadListDidUpdate`. However, we were not firing the action at
the correct moment.
Before this commit, we fired the action on the root `ThreadList`'s
`componentDidUpdate`, however did this not actually fire when the
child list actually updated/removed threads from the list.
This sort of used to work because before 396a027bcb
the root ThreadList component re-rendered all the time, so it fired the
action, but after initial sync, we would never actually report any
thread-list action metrics at all
Test Plan: manual
Reviewers: spang, halla, evan
Reviewed By: halla, evan
Differential Revision: https://phab.nylas.com/D4051
The previous fix only worked by coincidence. It turns out that we don't have
the right data to associate draftClientId with ChangeMailTask on task
creation, so we have to either look up this data inside the dependency
function or make the dependency check looser.
This check gets run all the time, so we chose to make the check looser
rather than putting a db call in here and changing the interface to support
an async version of isDependentOnTask().
This patch also makes it so that the fix works properly when Undo Send
is enabled.
Fixes T7889
Because ChangeMailTask only depended on EnsureMessageInSentFolder task,
if a user sent a message & then archived it before the send completed,
the ChangeMailTask could run in between the SendDraftTask and
EnsureMessageInSentFolderTask and fail because it's
EnsureMessageInSentFolderTask that detects the new UID for the message.
Fixes T7889
Temporarily reverting this commit for 1.0.30 release because it is causing
performance issues: https://phab.nylas.com/T7910
This reverts commit 5e35d39eb2.
Summary:
This commit adds a `shouldComponentUpdate` to thread-list.cjsx so that
the thread list doesn't unnecessarily /try/ to re-render when state or
props haven't actually changed.
I noticed this because the thread list was constantly calling `render`
even though it didn't render any changes to the DOM. This was caused
because it listens to `NylasSyncStatusStore` which constantly triggers,
even though the piece of state the list is interested in rarely changes,
causing unnecessary calls to the `render` loop.
Test Plan: manual
Reviewers: halla, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4047
Summary:
This commit is an attempt to cleanup duplicated code and crufty code
inside IMAPConnection and IMAPBox
Specifically:
- It replaces `_createConnectionPromise` with a more aptly named (imo) `_withPreparedConnection` helper, which provides the user a node-imap connection that will correctly time out and handle `error` and `end` events. Most of these changes are just changing existing code to use the new interface.
- Adds a subtle change to how we handle `end` and `error` events on the connections. Previously, we manually called `this.end()` on `error`, but not on `end`. From what I could gather from the old comment documenting `_createConnectionPromise`, we should /also/ call `this.end()` on `end` because node-imap doesn't clean up correctly and can leave the connection hanging (taking care not to introduce a recursion loop by `end`ing on `end`). Additionally, it no longer listens to the events via `once` but via `on`, which should be okay given that the listeners get cleared at the end.
This /might/ fix some instances of sync freezing up (T7837).
Depends on D4035
Test Plan: manual -- this really needs some unit tests 😢
Reviewers: spang, halla, mark, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4036
Summary:
Remember how we made it so imap socket timeouts would increment up to 10m if
we constantly got timeout errors? Remember how we told everyone in
https://github.com/nylas/nylas-mail/issues/3232 that we'd fixed their problems, but they weren't actually fixed?
Well, we weren't /actually/ applying the correct timeout value. 😢
Depends on D4033
Test Plan: manual
Reviewers: spang, halla, mark, evan
Reviewed By: mark, evan
Differential Revision: https://phab.nylas.com/D4035
Summary:
The way it was set up was a little smelly.
- `this._settings` were /not/ the actual settings. Now, internally we have access to `this._resolvedSettings`
- `this.resolvedSettings` was being set kind of out of nowhere (gmail-oauth-helpers needs it, hence adding a getter instead)
Test Plan: manual
Reviewers: spang, mark, evan, halla
Reviewed By: mark, evan, halla
Subscribers: khamidou
Differential Revision: https://phab.nylas.com/D4033
Summary:
This commit fixes 2 issues with signatures in the preferences:
- Creating a signature (via any of the create buttons) would create 2 signatures
- Trying to select accounts to associate with a signature in the preferences would not work (the account would not be selected)
This was a regression introduced in e638e94084 (diff-4f38fd25aa24b48a309354be643165d3R26)
3111c16 made it so we attempt to `activate` any Stores that are registered with
the StoreRegistry. When adding stores to `nylas-exports`, they are
automatically registered in the StoreRegistry. Given that the
`SignatureStore` is in `nylas-exports`, and consequently is registered in
the StoreRegistry, it would be `activate`d upon window boot.
However, we were also manually activating it inside `internal_packages/composer-signatures/lib/main.es6`.
This caused it to register listeners for every action **twice**. For
this reason, 2 signatures would be created when trying to create 1, and
an account would be immediately unselected after being selected int he
signatures dropdown in preferences.
(Other changes in this are just stylistic)
Test Plan: manual
Reviewers: spang, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4043
Summary: See title
Test Plan: Run locally, make sure sync still works
Reviewers: evan, spang, juan
Reviewed By: spang, juan
Differential Revision: https://phab.nylas.com/D3998
Summary:
I did a grep for `Message.find.*` to see if there were any other places
we could limit the attributes returned
There were.
This used to be INSANELY wasteful to return all bodies when we only needed
the ids.
Test Plan: Manually bootup app and ensure search still works
Reviewers: mark, juan, spang
Reviewed By: mark, juan, spang
Differential Revision: https://phab.nylas.com/D4042
Summary:
Fixes T7783
This unbounded query, plus a yet-to-be-diagnosed bug causing large numbers
of messages with no folderIds, led to frequent crashes of the worker
window at the end of every sync loop.
This took a fair amount to diagnose. Notes on that are here:
https://paper.dropbox.com/doc/Diagnosing-Crash-X329VTxevrqtIMESBtHNV
Huge thanks to @mark.
Test Plan: Reboot app with patch, no more crashes!
Reviewers: spang, halla, mark, juan
Reviewed By: mark, juan
Subscribers: mark
Maniphest Tasks: T7783
Differential Revision: https://phab.nylas.com/D4039
This goes against the spec, but we've seen emails which use comma-separated
References instead of space-separated references. This could cause threading
to break if the message with comma-separated references was a reply and
we hadn't yet synced the message directly in the In-Reply-To header.
Summary:
When we detect that threads in the database have changed, if they've already
been indexed, mark them as unindexed so that the indexer picks them up and
re-indexes them with the updated data
Test Plan: manual
Reviewers: evan, spang, mark
Reviewed By: mark
Differential Revision: https://phab.nylas.com/D4023
Summary:
If for whatever reason we passed an error with a `source` property
we weren't accounting for in `convertIMAPError`, we would return an
`undefined` error. Bad!
Instead, just return the original error
Test Plan: manual
Reviewers: spang, halla, evan, mark
Reviewed By: evan, mark
Differential Revision: https://phab.nylas.com/D4037
Summary:
When reporting different times to mixpanel other than `actionTimeMs` via
recordPerfMetric, we also want to clip those times to a range to have
good data in mixpanel
This commit adds an extra option to recordPerfMetrics to be able to clip
data other than the default `actionTimeMs`, and uses the new option to
report search metrics
Test Plan: manual
Reviewers: spang, evan, halla, mark
Reviewed By: mark
Differential Revision: https://phab.nylas.com/D4038
Summary:
I'm making these up based on gut feeling, so feel free to completely change
these.
Test Plan: manual
Reviewers: spang, mark, evan
Reviewed By: mark, evan
Differential Revision: https://phab.nylas.com/D4026
Summary:
Since we only persist updates to fetchedmin/fetchedmax at the end of a
batch, and a batch can contain many messages, if the sync loop is
getting interrupted often we will download the same messages over and
over again and not make much progress in downloading the message
backlog. This patch keeps a set of already downloaded messages in memory
for each batch and skips downloading UIDs we've processed in interrupted
sync loops.
Messages may still be redownloaded across app restarts.
Fixes T7798
Test Plan: manual
Reviewers: juan, mark
Reviewed By: juan, mark
Maniphest Tasks: T7798
Differential Revision: https://phab.nylas.com/D4040
Summary:
Previously we would always search all mail. Now, if the user has focused
a particular folder we will limit our search to that folder. The inbox
is an exception--it will always search all mail unless the user
explicitly uses an "in:" clause.
Test Plan: Run locally, verify that searching folders returns the correct results.
Reviewers: evan, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4032
This is a temporary commit to prevent connections from being closed
while we release logic to re-establish delta connections after they are closed.
Our infrastructure should be able to support current usage of delta
connections after our fixes to offline notification and infrastructure
This reverts commit 78f67d4a76.
Given that this packages lives inside the NylasMail app, it has access
to Bluebird Promises, which already define a .each method.
Using PromiseUtils here is unecessary
Summary:
We were only detecting and classifying IMAP errors as retryable and non
retryable in a few places, but errors thrown in any of our imap
operations were not being properly passed through our `convertImapErrors` :(
This was broken, oh so broken
Also loosen condition to detect System Errors
Test Plan: manual
Reviewers: spang, mark, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4022
Summary:
Fix errors and inconsistencies in MetricsReporter code like:
- We were modifying the passed in data object in place
- We were accessing NylasEnv without knowing we were in client env to
get the version
- We were creating an incorrect logger instance
Test Plan: manual
Reviewers: spang, halla, evan
Reviewed By: halla, evan
Differential Revision: https://phab.nylas.com/D4010
Summary:
This action will report perf metrics to honeycomb /and/ mixpanel. This
commit also fixes up Analytics Store a little bit
Test Plan: manual
Reviewers: spang, halla, evan
Reviewed By: halla, evan
Subscribers: jerm
Differential Revision: https://phab.nylas.com/D4012
Summary:
Also see:
3a33b0ad64
which was hot-pushed to master in order to get Travis building.
We now have two travis files:
1. /.travis.yml
2. /packages/client-app/travis.yml
The first one is alwas in the private repo and runs `npm install && npm
run build-client`. This decrypts our keys and signs, builds, and uploads
to S3.
The second one is designed to live in our yet-to-be public mirror. It will
basically just run `npm install && npm test`.
That way the public one should just about ALWAYS pass (YAY!) except of
course when you break the tests or something in the installer!
Test Plan: Run on new https://travis-ci.com/nylas/nylas-mail-all
Reviewers: jerm, spang, juan
Reviewed By: spang, juan
Differential Revision: https://phab.nylas.com/D3999
Summary: See title
Test Plan: Run locally, verify IMAP search still works
Reviewers: evan, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4006
Summary: See title
Test Plan: Run locally, verify we can download raw messages
Reviewers: evan, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4005
Summary: See title
Test Plan: Run locally, verify downloading files works
Reviewers: evan, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4004
Summary:
We did this to see if we should clear the index, which no longer makes
sense because the index never exceeds our threshold. Additionally, none
of the other search indexes do this. This was causing us to spend a ton
of time scanning the ThreadSearch table at startup.
Test Plan: Run locally, make sure things are fine
Reviewers: evan, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4007
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
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
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
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
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
Summary: See title
Test Plan: Depends on D3990
Reviewers: spang, halla, mark, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3991
Summary:
See title
Depends on D3989
Test Plan: manual
Reviewers: evan, spang, halla
Reviewed By: spang, halla
Differential Revision: https://phab.nylas.com/D3990
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
