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:
Given that `lerna bootstrap` does not install optional dependencies, we
need to manually run `npm install` inside `client-app` so
`node-mac-notifier` get's correctly installed and included in the build
See https://github.com/lerna/lerna/issues/121
Fixes T7887
Test Plan: manual
Reviewers: halla, evan
Reviewed By: evan
Maniphest Tasks: T7887
Differential Revision: https://phab.nylas.com/D4076
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:
Put them in the client-app/spec folder instead of
client-app/internal_packages
Test Plan: Ran the tests
Reviewers: juan, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4080
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
