Summary:
This is a critical patch that fixes two problems with the task queue:
1. Tasks in Status: Retry are retried the next time processQueue is run,
which could be pretty much immediately. Certain scenarios lead to tasks
running in a hard loop forever.
2. Returning Task.Status.Retry set the retry flags on the task but did not
schedule the queue to be processed again. So if only a single item in the
queue was present, it might never be retried again until the user performed
another action.
Test Plan: Where did the specs for TaskQueue go? There aren't many... need to write more.
Reviewers: evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D2762
Summary:
Fixes an issue with sending where certain conditions could result in a
duplicated message.
Fixes task dependency logic for draft syncback and send. Changes `createdAt`
on tasks to instead be `sequentialId`, assigned when the task is queued, to
track order of enqueueing. Renames `isDependentTask` => `isDependentOnTask`
and adds comments for clarity.
Test Plan:
Specs updated. Might be good to add some later to test this particular
edge case.
Reviewers: juan, evan, bengotow
Reviewed By: evan, bengotow
Differential Revision: https://phab.nylas.com/D2681
Summary:
This diff adds an "OutboxStore" which reflects the TaskQueue and
adds a progress bar / cancel button to drafts which are currently sending.
- Sending state is different from things like Send later because drafts
which are sending shouldn't be editable. You should have to stop them
from sending before editing. I think we can implement "Send Later"
indicators, etc. with a simple InjectedComponentSet on the draft list
rows, but the OutboxStore is woven into the DraftList query subscription
so every draft has a `uploadTaskId`.
- The TaskQueue now saves periodically (every one second) when there are
"Processing" tasks. This is not really necessary, but makes it super
easy for tasks to expose "progress", because they're essentially
serialized and propagated to all windows every one second with the
current progress value. Kind of questionable, but super convenient.
- I also cleaned up ListTabular and MultiselectList a bit because they
applied the className prop to an inner element and not the top one.
- If a DestroyDraft task is created for a draft without a server id, it
ends with Task.Status.Continue and not Failed.
- The SendDraftTask doesn't delete uploads until the send actually goes
through, in case the app crashes and it forgets the file IDs it created.
Test Plan: Tests coming soon
Reviewers: juan, evan
Reviewed By: evan
Differential Revision: https://phab.nylas.com/D2524
Summary:
Add concept of "final" to Query, clean up internals
Tiny bug fixes
RxJs Observables!
WIP
Test Plan: Run tests
Reviewers: evan, juan
Reviewed By: juan
Differential Revision: https://phab.nylas.com/D2319
Summary:
Fixes T4291
If I made a final edit to a pre-existing draft and sent, we'd queue a
`SyncbackDraftTask` before a `SendDraftTask`. This is important because
since we have a valid draft `server_id`, the `SendDraftTask` will send by
server_id, not by POSTing the whole body.
If the `SyncbackDraftTask` fails, then we had a very serious issue whereby
the `SendDraftTask` would keep on sending. Unfortunately the server never
got the latest changes and sent the wrong version of the draft. This
incorrect version would show up later when the `/send` endpoint returned
the message that got actually sent.
The solution was to make any queued `SendDraftTask` fail if a dependent
`SyncbackDraftTask` failed.
This meant we needed to make the requirements for `shouldWaitForTask`
stricter, and block if tasks failed.
Unfortunatley there was no infrastructure in place to do this.
The first change was to change `shouldWaitForTask` to `isDependentTask`.
If we're going to fail when a dependent task fails, I wanted the method
name to reflect this.
Now, if a dependent task fails, we recursively check the dependency tree
(and check for cycles) and `dequeue` anything that needed that to succeed.
I chose `dequeue` as the default action because it seemed as though all
current uses of `shouldWaitForTask` really should bail if their
dependencies fail. It's possible you don't want your task dequeued in this
dependency case. You can return the special `Task.DO_NOT_DEQUEUE_ME`
constant from the `onDependentTaskError` method.
When a task gets dequeued because of the reason above, the
`onDependentTaskError` callback gets fired. This gives tasks like the
`SendDraftTask` a chance to notify the user that it bailed. Not all tasks
need to notify.
The next big issue was a better way to determine if a task truely errored
to the point that we need to dequeue dependencies. In the Developer Status
area we were showing tasks that had errored as "Green" because we caught
the error and resolved with `Task.Status.Finished`. This used to be fine
since nothing life-or-death cared if a task errored or not. Now that it
might cause abortions down the line, we needed a more robust method then
this.
For one I changed `Task.Status.Finished` to a variety of finish types
including `Task.Status.Success`. The way you "error" out is to `throw` or
`Promise.reject` an `Error` object from the `performRemote` method. This
allows us to propagate API errors up, and acts as a safety net that can
catch any malformed code or unexpected responses.
The developer bar now shows a much richer set of statuses instead of a
binary one, which was REALLY helpful in debugging this. We also record
when a Task got dequeued because of the conditions introduced here.
Once all this was working we still had an issue of sending old drafts.
If after a `SyncbackDraftTask` failed, now we'd block the send and notify
the users as such. However, if we tried to send again, there was a
separate issue whereby we wouldn't queue another `SyncbackDraftTask` to
update the server with the latest information. Since our changes were
persisted to the DB, we thought we had no changes, and therefore didn't
need to queue a `SyncbackDraftTask`.
The fix to this is to always force the creation of a `SyncbackDraftTask`
before send regardless of the state of the `DraftStoreProxy`.
Test Plan: new tests. Lots of manual testing
Reviewers: bengotow
Reviewed By: bengotow
Subscribers: mg
Maniphest Tasks: T4291
Differential Revision: https://phab.nylas.com/D2156
