Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Task.halt() always succeeds as long as teardown succeeds. #837

Merged
merged 1 commit into from
Nov 29, 2023

Conversation

cowboyd
Copy link
Member

@cowboyd cowboyd commented Nov 29, 2023

Motivation

There are two distinct ways in which a task can fail. It can straight up fail if there is an error during its execution, or it can fail after it has already completed its execution and somewhere in the teardown process an error occurs. So for example, a task whose execution succeeds but whose teardown fails will be considered a failure.

It is also possible to have a task whose execution fails, but whose teardown succeeds. However, we don't represent this when evaluating the Task.halt() operation. Instead, we always fail if the task execution has failed. This is a not ideal because if a user awaits or yields to Task.halt() they haven't asked for the result of the task itself, only for the result of halting it. This makes meta-level apis like wait groups more difficult because you have to have separate logic for errored tasks and for successful tasks, when really in some cases, you want to treat them all as something that just needs to be halted.

current behavior

let task = run(function*() { throw new Error('boom!'); });
await task.halt(); //=> Error boom!

desired behavior

let task = run(function*() { throw new Error('boom!'); });
await task.halt(); //=> void

Approach

Luckily, we are already tracking the success or failure of frame destruction internally, so this just adds that information to the FrameResult, so now with every frame, you can not only read both how its execution went (error, success, abort), but also how went its destruction.

@cowboyd cowboyd requested a review from neurosnap November 29, 2023 14:33
There are two distinct ways in which a task can fail. It can straight
up fail if there is an error during its execution, or it can fail after it has
already completed its execution and somewhere in the teardown process
an error occurs. So for example, a task whose execution succeeds but
whose teardown fails will be considered a failure.

It is also possible to have a task whose execution fails, but whose
teardown succeeds. However, we don't represent this when evaluating
the `Task.halt()` operation. Instead, we always fail if the task
execution has failed. This is a not ideal because if a user awaits or
yields to `Task.halt()` they haven't asked for the result of the task
itself, only for the result of _halting_ it. This makes meta-level
apis like wait groups more difficult because you have to have separate
logic for errored tasks and for successful tasks, when really in some
cases, you want to treat them all as something that just needs to be
halted.

Luckily, we are already tracking the success or failure of frame
destruction internally, so this just adds that information to the
`FrameResult`, so now with every frame, you can not only read both how
its execution went (error, success, abort), but also how went its
destruction.
@cowboyd cowboyd force-pushed the good-halt-on-bad-task branch from 632478d to 5fec9c2 Compare November 29, 2023 14:46
@cowboyd cowboyd merged commit b0f8f38 into v3 Nov 29, 2023
1 check passed
@cowboyd cowboyd deleted the good-halt-on-bad-task branch November 29, 2023 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants