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
Summary:
Node's native `Error` object does NOT implement toJSON. We attempt to call
toJSON when reporting errors. This wasn't noticed until now because
bunyan's pretty logger (which only run in dev mode) started JSONifying
errors
Test Plan:
Try and API auth with a bad username with local setup. See that it throws
toJSON error. After patch, error properly serializes
Reviewers: spang, halla, jerm, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D3975
Summary:
Some API Errors, like ECONNREFUSED, have no \.message.
Catch this in the error reporter
Test Plan: Manually create a non-message error and see better error message
Reviewers: spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D3973
Summary:
Adding READMEs for easy and helpful browsing on GitHub.
Also add missing script and `--interpreter` flag
Test Plan: Run new launch commands
Reviewers: mark, spang, juan, halla
Reviewed By: spang, juan, halla
Differential Revision: https://phab.nylas.com/D3971
Summary:
Client-sync has the full imap folder names, but used to only pass the display
name to the application. The application needs the full imap names so that it
can pass them via metadata to cloud-workers that need to open imap boxes.
Test Plan: manual
Reviewers: evan, juan
Reviewed By: evan, juan
Subscribers: juan
Differential Revision: https://phab.nylas.com/D3951
Summary: atob() is a global in browser environments, but needs to be imported otherwise.
Test Plan: manual
Reviewers: juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D3950
Summary: Moved a file and forgot to update this import :-/
Test Plan: Run locally
Reviewers: juan, evan, spang
Differential Revision: https://phab.nylas.com/D3970
Summary:
If an exception has the same stack trace, by default Sentry will always group
it together in the same event. We don't want to do that for sync loop
errors---e.g. 'Invalid credentials' errors should not be grouped together with
stuff like 'Too many simultaneous connections'. Creating more unique groups
will allow us to better evaluate the effect of sync & other bugfixes.
Test Plan: writing unit test right now
Reviewers: juan, mark
Subscribers:
Differential Revision: https://phab.nylas.com/D3915
Summary: See title
Test Plan: Run locally, verify that double clicking inline images opens them
Reviewers: evan, juan, spang
Reviewed By: juan, spang
Differential Revision: https://phab.nylas.com/D3963
Summary: This stuff doesn't seem to be used for anything anymore and it's cluttering up the client-app dir.
Test Plan: ran the app, also did `npm start` with no ~/.nylas-dev
Reviewers: evan, juan
Reviewed By: evan, juan
Differential Revision: https://phab.nylas.com/D3961
Summary:
This is a set of functions which will allow isomorphic-core to detect
which environment it is running on.
This will be useful for moving the metrics reporter to iso-core
Test Plan: manual
Reviewers: mark, spang
Reviewed By: spang
Differential Revision: https://phab.nylas.com/D3966
Summary:
We were doing some incorrect processing of args passed to the main
function which was causing us to think we were launching NM by passing a
file (which creates a new draft and tries to attach that file). Since
were trying to attach 'packages/client-app', this was causing an error
dialogue to appear indicating that it wasn't possible to attach a
directory.
Test Plan: Run locally, verify no dialogue
Reviewers: evan, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D3962
[*] update babel
[client-app] remove flow-typed
[client-app] Move build/package.json to main package.json
[client-app] remove spec_integration
[client-app] fix babel support
Add client-private-plugins package.json
[client-app] add node_modules to global path for private-plugins
Move client-sync dependencies to client-app root
fix electron rebuild
[*] moved to monorepo
Summary: App now runs in monorepo
Test Plan: npm test
Reviewers: juan, mark, khamidou, halla, spang
Differential Revision: https://phab.nylas.com/D3947
Summary:
When saving a thread, we weren't properly setting the `participants`
object. Since Sequelize has object properties under getters and setters,
doing `this.participants.push()` did nothing.
Since threads had no `participants`, the related threads widget, which
queries through there, did nothing.
Test Plan: Manually inspect DB and use the related threads widget
Reviewers: halla, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D3938
Summary:
This is the result of auto package.json fixing by lerna. Would be nice to
commit this so you can run script/bootstrap without it making local
changes.
I didn't manually bump any versions.
Test Plan: manual
Reviewers: mark, halla, spang, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D3934
Summary:
We don't need to check folders that often while on battery. Check every
5 minutes rather than every 10 seconds.
Test Plan: Run locally, verify the timeout is longer while on battery
Reviewers: evan, spang, juan
Reviewed By: spang, juan
Differential Revision: https://phab.nylas.com/D3940
Summary: See title
Test Plan: manual
Reviewers: evan, mark, spang
Reviewed By: spang
Subscribers: mark
Differential Revision: https://phab.nylas.com/D3932
Summary: They obscure the location of our logs in the dev tools.
Test Plan: Run locally, verify that logs link to proper place
Reviewers: evan, spang, juan
Reviewed By: spang, juan
Differential Revision: https://phab.nylas.com/D3939
Summary:
Previously, we would only refresh Google OAuth2 access tokens at the
beginning of the sync loop, and _only_ if the access token had already
expired. This meant that if an access token expired in the middle of a
sync loop iteration, the user would get prompted with the reauth red box
for their account and would have to either go through the oauth flow
again or restart the app for sync to continue.
This diff makes two changes:
1. Adds 5min of padding to the refresh window, so if a token will expire
in <5min, we'll go ahead and refresh the token. This will reduce the
possibility that an access token can expire during a sync loop
iteration.
2. Catches Invalid Credentials IMAPAuthenticationErrors for Gmail
accounts and forces a token refresh on the next sync loop.
These should prevent a user from _ever_ having to reauth their Gmail
account unless the refresh token is revoked, or we encounter some other
permanent error trying to refresh the token.
Fixes T7775 (at least some cases)
Test Plan: manual
Reviewers: khamidou, evan, juan
Reviewed By: juan
Maniphest Tasks: T7775, T7755
Differential Revision: https://phab.nylas.com/D3908
Summary:
Instead of re-implementing exponential backoff, throw the retryable
error so the sync loop handles it and backs-off
Test Plan: manual
Reviewers: spang, halla
Reviewed By: halla
Differential Revision: https://phab.nylas.com/D3914
Summary:
Update Salesforce to use the new search indexer
Depends on D3911
Test Plan: Manually bootup SFDC and ensure it launches and indexes models properly
Reviewers: mark, halla, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D3913
Summary: Should help a fair bit with our redis connection pileup.
Test Plan: Tested manually.
Reviewers: mark, spang, juan, evan
Reviewed By: mark, spang, evan
Differential Revision: https://phab.nylas.com/D3916
Summary:
This commit makes it so we always continue retrying syncback tasks as long as they
error with a retryable error. There's really no reason to not continue retrying syncback
tasks after an arbitrary number of retries (especially such a low one) if we
encounter a retryable error. Before this commit, if for example we got 2 random
network errors in a row, we would just mark the task as failed even
though it would eventually succeed in subsequent attempts.
Previously, when N1 synced against the cloud api, we would indefinitely
retry a Task if we continued getting retryable errors. This ensures that
the app can work correctly offline and prevents displaying unecessary errors to
the user, and having actions bouncing back or sending messages without putting them
in the sent folder.
Additionally, this commit ensures that when cleaning up messages without a
folderImapUID we don't delete messages that are currently being added to the sent
folder. This is relevant to this commit because given that we could retry the
EnsureMessageInSentFolder task indefinitely, we might end up deleting that
message because it wont have a uid until the task succeeds.
Depends on D3898
Test Plan: manual
Reviewers: mark, spang, evan, halla
Reviewed By: spang, evan, halla
Differential Revision: https://phab.nylas.com/D3900
Summary:
Since all Nylas Mail actions are thread-based except for sending,
we never use any of these. Make `git grep` less confusing by getting rid
of them.
Test Plan: use the app
Reviewers: evan, halla, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D3910
Summary:
Previously, if you were to close the app while you had any tasks queued,
these would be marked as failed the next time you open the app, showing
an annoying error message and reverting any optimisitic actions.
However, we don't need to be so defensive about retrying tasks because
the only tasks we can't retry are the Sending tasks. All of the other
tasks like moving or changing labels are fine to retry (trying move the same
set of uids twice wont cause an error)
This commit adds an extra status to syncback requests, "NOTRETRYABLE".
Only NOTRETRYABLE requests will be marked as failed at the beginning of
the sync loop, and any INPROGRESS tasks will be marked as NEW so they
can be retried
Depends on D3896
Test Plan: manual
Reviewers: mark, evan, spang, halla
Reviewed By: evan, halla
Differential Revision: https://phab.nylas.com/D3898
Summary:
Now that we don't run Send tasks outside the sync loop, we don't need
that awful hack wich required passing a `runTask` callback to
`runSyncbackTask` in order to customize how to run the task.
Instead, runSyncbackTask now knows 2 ways to run a task, either via imap, or
via smtp, depending on the resource declared by task to run. So now
SyncbackTasks declare a resource type they need to run, and that will be
passed as their second argument when running.
Depends D3894
Test Plan: manual
Reviewers: mark, halla, spang, evan
Reviewed By: halla, spang, evan
Differential Revision: https://phab.nylas.com/D3896
Summary:
We had previously ripped send tasks outside the sync loop to make them run faster,
but they run fast enough inside the loop.
This commit will also fix the scenario where if you closed the app in the
middle of a send task, the task would just hang forever and never succeed or
fail (T7818); given that it was excluded from the loop, we also had to exclude it
from the cleanup step to mark any INPROGRESS tasks as failed at the beginning
of each loop, which caused send tasks in progress to never get cleaned.
Putting them back inside the loop allows us to fix this without adding more messy
logic, and it cleans up ugly duplicated code. Additionally, it will prevent more
duplicated code for upcoming diffs that will improve syncback task reliability
when the app is closed or window is restarted in the middle of a task.
Depends on D3893
Test Plan:
manually test sending, it still works, it's still fast. Restarted
window in the middle of send task, task fails.
Reviewers: mark, spang, halla, evan
Reviewed By: spang, halla, evan
Differential Revision: https://phab.nylas.com/D3894
Summary:
[cloud-api]
Based on the passed in `messageIds`, it finds any existing thread
metadata that might be under a different thread id. If it realizes
there are actually multiple threads that should be the same thread,
(due to getting a missing message link), it reconciles all of them.
[local-sync]
Return `message_ids` in `Thread.toJSON()`
See D3879 for tests
N1 Pairing: D3875
Test Plan: unit tests, local testing soon
Reviewers: juan, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D3880
