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

Pass parent's thrown error to a bond task[Symbol.dispose](err) #158

Closed
rixtox opened this issue Jun 2, 2023 · 2 comments
Closed

Pass parent's thrown error to a bond task[Symbol.dispose](err) #158

rixtox opened this issue Jun 2, 2023 · 2 comments

Comments

@rixtox
Copy link

rixtox commented Jun 2, 2023

I'm looking at this problem with the Cancellation API in mind. I know it's probably not the scope for this proposal but I think we should probably think ahead of possible interaction with a future Cancellation design.

This means a Disposable can not only be some kind of resource, but it can also be some kind of tasks. If it's a task, it would probably be interested in the reason for it to be disposed, which means to distinguish between disposal because of normal completion, and disposal because of abruption.

Consider the following example that leverage generator abruption as a cancellation mechanism.

function wrapGeneratorInDisposable(g) {
  return {
    [Symbol.dispose](err) { // current Disposal proposal doesn't pass in err thrown from parent
        if (err) {
          g.throw(err);
        } else {
          g.return();
        }
    }
  };
}

function * bar() {
  try {
    let count = 0;
    while(true) {
      yield count++;
    }
  } catch(err) {
    // error handling
  } finally {
    // normal cleanup
  }
}

function * foo() {
  using task = wrapGeneratorInDisposable(bar()); // bind a cancellable task to the current scope
  yield task.next();
  yield task.next();
  yield task.next();
} // task get disposed because of normal completion or abruption

{
  using g = foo();
  g.next();
  g.throw(err); // abrupt the parent
}

When the parent got abrupt with an error, the child task would got disposed by task[Symbol.dispose](). Notice that the task would have no way to distinguish it with a dispose call when foo() is returning normally without error.

A suggestion is to pass in the parent's thrown error to the bond task[Symbol.dispose](err) call, or undefined if it the parent is normally completed. However this would have a corner case if undefined was thrown. I think we should look into this use case anyway.

I think a comparable design is the Tomb Golang package that does tomb.kill(reason) and the reason would be propagated to bound cleanup receivers.

@rixtox
Copy link
Author

rixtox commented Jun 2, 2023

A solution probably being the least intrusive to the current design, is to introduce a TryResult type:

interface TryResult {
  hasError: boolean;
  error: any; // thrown value
}

interface Disposable {
  [Symbol.dispose](result: TryResult): void;
}

And the semantics becomes:

{
  const $$try = { stack: [], hasError: false, error: undefined };
  try {
    ... // (1)

    const x = expr1;
    if (x !== null && x !== undefined) {
      const $$dispose = x[Symbol.dispose];
      if (typeof $$dispose !== "function") {
        throw new TypeError();
      }
      $$try.stack.push({ value: x, dispose: $$dispose });
    }

    const y = expr2;
    if (y !== null && y !== undefined) {
      const $$dispose = y[Symbol.dispose];
      if (typeof $$dispose !== "function") {
        throw new TypeError();
      }
      $$try.stack.push({ value: y, dispose: $$dispose });
    }

    ... // (2)
  }
  catch ($$error) {
    $$try.error = $$error;
    $$try.hasError = true;
  }
  finally {
    while ($$try.stack.length) {
      const { value: $$expr, dispose: $$dispose } = $$try.stack.pop();
      try {
        $$dispose.call($$expr, { hasError: $$try.hasError, error: $$try.error });
      }
      catch ($$error) {
        $$try.error = $$try.hasError ? new SuppressedError($$error, $$try.error) : $$error;
        $$try.hasError = true;
      }
    }
    if ($$try.hasError) {
      throw $$try.error;
    }
  }
}

@rixtox
Copy link
Author

rixtox commented Jun 4, 2023

I withdraw my opinion of passing parent completion result to the dispose method. Exposing the dispose method made it tempting to use Disposable as a cancellation mechanism, while in principle it should not be entangled with the execution order and interruption of an execution scope.

Exposing the dispose method also exposed transferable ownership (or control) of the resource. In general, I don't think that's a good idea at all. Ownership should be declared, not assigned.

Cancellation on the other hand, is assigning a controlling signal to a running task. Cancellation cannot be declared.

The current design of the Disposable interface is mixing these concepts, which made it tempting for people to abuse it for things it shouldn't do.

I have made a counter-proposal in #159. Closing this issue.

@rixtox rixtox closed this as completed Jun 4, 2023
@ljharb ljharb closed this as not planned Won't fix, can't repro, duplicate, stale Jun 4, 2023
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

No branches or pull requests

2 participants