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:
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:
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 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:
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:
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 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:
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:
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:
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
