Summary: Using `await` instead of `advanceClock()` fixes all the things!
Test Plan: Ran the spec file
Reviewers: evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4248
Summary:
This fixes T7995.
Previously, attempting to log out from your NylasID would just
automatically sign you back in with the same NylasID because the webview
session was preserved.
Now, instead of relaunching the windows, we restart the app to clear the
webview session
Test Plan: manual
Reviewers: halla, evan
Reviewed By: evan
Maniphest Tasks: T7995
Differential Revision: https://phab.nylas.com/D4224
Summary:
Previously, the logic for deciding wether to report an APIError lived
inside NylasAPIRequest and depended on an array of ignorable
statusCodes. However, when handling APIErrors, NylasAPIRequest would
convert special error codes (for offline errors) into a statusCode of 0,
which would automatically be ignored.
Given that the delta streaming connection didn't convert status codes to
0, it wouldn't actually ignore the error and end up reporting it,
flooding sentry.
This commit makes it so that that logic lives inside APIError, and
anyone who handles APIErrors can check wether to report them or not,
including the delta streaming connection
Test Plan: manual :(
Reviewers: spang, evan, halla
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4235
Summary:
Currently, when we auth an account for the first time in Nylas Mail (or
we blow away the database), the app is going to request transactions
since cursor `null` from the /delta/streaming endpoint and from the local-sync
delta observable, instead of requesting transactions since cursor `0`
This is due to a subtle bug with the use of default values when
destructuring an object. Our coded did the following:
```
const {cursor = 0} = this._state
```
Which at a glance seems correct. However, this will only work as
expected if `this._state` has the following shape:
```
{cursor: undefined}
```
And unfortunately, our `this._state` looked like this when authing an
account for the first time:
```
{cursor: null}
```
Which would make `cursor === null` instead of `0`.
This is because when using default values, null is considered an
intentional argument/value, as opposed to not passing any argument/value
(which will mean that the argument is undefined).
This was a regression introduced in d60a23c and 8bc2ec5
Test Plan: manual, will add regression test in upcoming diff
Reviewers: evan, spang, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4243
Summary:
Previously, after creating a new folder, the UI would indicate that the new
folder had children, even though it didn't. This was caused by duplicate models
in our `MutableQueryResultSet` for the user's categories. Basically, we would
sync the server version of the folder before the `SyncbackTask` for the new
folder returned its `serverId`. Without the `serverId`, the synced version of
the folder couldn't yet be tied to the optimistic folder, so a second row was
created in the database. This second row is removed when the `syncbackTask`
does return the `serverId`, because we persist the optimistic folder with a
`REPLACE INTO` query. (This deletes other rows with the same id.) However,
since this was done inside a `persist` change with the `serverId` and no
`unpersist` was ever recorded for the `clientId`, our `MutableQueryResultSet`
never removed the `clientId` model.
To address this, this diff adds a check in `updateModel` to see if the
`serverId` is being added. If it is, and both the `serverId` and `clientId`
exist in the `_ids` list, we remove the `clientId`.
The children indicator does still briefly show up while there are still two
separate rows for that folder in the database. If we want to get rid of this
completely, we would have to ensure that we do not sync the folder before the
`syncbackTask` returns the `serverId`. However, this would probably be pretty
involved, and for not much gain. This fix is much simpler and reduces most of
the issue.
Test Plan: manual
Reviewers: juan, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D4228
Summary:
The ignore list was very old. It included several dozen MB of docs_src and
other crap in our builds
Test Plan: manual
Reviewers: halla, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D4213
Summary: We need to upload the nupkg for the Windows autoupdater to work
Test Plan: manual
Reviewers: juan, spang, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4219
Summary:
This is going to be a diff way shorter than the previous one! Basically, it adds a new endpoint, `/blobs` to our API to store send later attachments. When a user schedules a draft to be sent, we send all attachments to this endpoint. Separately, we store the rest of the message as metadata.
When it's time to send the message, we fetch the attachments from S3, fetch the metadata and merge them together to get a message we can send.
Test Plan: Tested manually. Will make a final QA pass before landing.
Reviewers: juan, halla, evan
Reviewed By: halla, evan
Differential Revision: https://phab.nylas.com/D4196
Summary:
On windows running `git rev-parse --short HEAD` does in fact now give you
9 characters instead of 7 like it does on Mac. This will ensure that
builds get uploaded to the same folder and help ensure we don't post a
version that doesn't exist on the release page.
Test Plan: Manual
Reviewers: juan, halla, spang
Reviewed By: spang
Differential Revision: https://phab.nylas.com/D4217
Summary:
Use electron's `powerMonitor` module to detect when the computer resumes
from sleep, and restart the sync loop when that happens in order to
sync the inbox immediately, in case we received any new mail events
while the computer was asleep
Test Plan: manual
Reviewers: evan, spang, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D4216
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:
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:
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:
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 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:
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:
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:
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:
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
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:
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:
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
