Asynchronous Promise Cancellation with Cancellable Promises, AbortController and Generic Timer

[CMCDragonkai]

Specification

There are a number of places where we have asynchronous operations where there is a timeout applied. When timedout, certain cancellation operations must be done. Sometimes this means cancelling a promise which may involve IO, and other cases it means breaking a while (true) loop.

These places are:

• src/utils.ts - the poll utility function is being used by a number of tests to testing if an asynchronous function returns what we want using the condition lambda, this is inherently loopy due to its naming of poll, this means cancellation occurs by breaking the loop, and no setTimeout is being used here, when the loop is boken, an exception is thrown as ErrorUtilsPollTimeout
• src/network/ForwardProxy.ts and src/network/ReverseProxy.ts - these make use of the Timer object type, that is used to allow timer inheritance. Unlike poll, here we are using Promise.race to cancel awaiting for a promise that will never resolve. In this situation, the problem is that we want to cancel waiting for a promise, the promise operation itself does not need to be cancelled, see Promise.race usage in src/network/ConnectionForward.ts and src/network/ConnectionReverse.ts
• src/grpc/GRCPServer.ts same situation as the proxies, here it's a promise we want to stop waiting for, the promise operation itself does not need to be cancelled.
• src/identities/providers/github/GitHubProvider.ts - this is the oldest code making use of polling, as such it has not been using the poll or timerStart and timerStop utilities, it should be brought into the fold
• src/status/Status.ts - this uses polling in waitFor
• src/discovery/Discovery.ts - we need to be able to abort the current task (discovery of a node/identity) rather than awaiting for it to finish when we stop the discovery domain CLI and Client & Agent Service test splitting #311 (comment)

Note that "timer" inheritance needs to be considered here.

So there are properties we need to consider when it comes to universal asynchronous "cancellation" due to timeout:

1. The ability to inherit a "timer" object that is used by the networking proxies, this is because a single timeout timer may need to be used by multiple asynchronous operations
2. The ability to cancel asynchronous operations in 3 ways:
   1. Breaking a while (true) loop
   2. Racing a promise and then dropping the awaiting of it
   3. Actually cancelling the promise operation - this is not currently done at all
3. An undefined version of the timeout, perhaps like an infinite version, this is so that function signatures can take timer object, which can defaulted to new Timer(Infinity)

My initial idea for this would be flesh out the Timer type that we are using, include methods that allow one to start and stop.

For 2.3, we actually have to integrate AbortController. This will be important for cancelling async IO operations. We can then integrate this into our timeout system itself.

Imagine:

const timer = new Timer(timeout);

timer.run(
  async (abortSignal) => { 
    // use the abort signal
    // fetch has a signal option
    // it can take the abortSignal
  }
);

In a way it can be an abstraction on top of the AbortController.

General usage summary of the abort controller.

// Creating an abort controller.
const abortController = new AbortController();

// it is pretty standard to pass an abort controller to methods in this way.
async function someFunction(arg1: any, arg2: any, options?: {signal?: AbortSignal}){
  // short way of getting the optional signal
  const { signal } = { ...options };

  for await (const item of longRunningGenerator()) {
    // Doing things here
    if (signal?.aborted){
      // Aborted functions/methods need to throw some kind of abort error to end.
      throw Error('AbortError')
    }
    // passing the signal down
    someOtherFunction(arg2, {signal});
    // Or we can use an event
    signal?.addEventListener('abort', async () => {
      // Do something to abort
    })
  }
}

// using a function that supports the abort signal
const prom = someFunction(1,2, {signal: abortController.signal});
// aborting
abortController.abort();
// will throw the abort error.
await prom;

More prototyping has to be done here.

For while loops, we can check if the signal is true, and if so cancel the loop.

---

There's 3 portions to this issue:

1. The timer/timeout problem
2. The cancellable wrapper higher order function/decorator
3. Refactoring everywhere where we have a "cancellable" asynchronous thing to use the cancellable wrapper and optionally the timer/timeout mechanism

Timer/Timeout problem

Many third party libraries and core core has a timeout convenience parameter that allows an asynchronous function to automatically cancel once a timeout has finished.

The core problem is that timeout is not easily "composable".

This is why the Timer abstraction was created.

/**
 * Use Timer to control timers
 */
type Timer = {
  timer: ReturnType<typeof setTimeout>;
  timedOut: boolean;
  timerP: Promise<void>;
};

This allowed functions to propagate the timer object down to low level functions while also reacting to to this timer. This means each function can figure out if their operations have exceeded the timer. But this is of course advisory, there's no way to force timeout/cancel a particular asynchronous function execution unlike OS schedulers that can force SIGTERM/SIGKILL processes/threads.

This is fine, but the major problem here is how to interact with functions that only support a timeout parameter. This could be solved with an additional method to the Timer object that gives you the remaining time at the point the method is called.

For example:

function f (timer?: Timer | number) {
  // Checks if already Timer, if not, will construct a timer with timeout being undefined or a number
  // An `undefined` should result in some default timeout
  timer = timer instanceof Timer ?? new Timer(timer);

  // This takes some time
  await doSomething();

  // This can inherit the timer
  await doAnotherThing(timer);

  // This uses timeout number soley, and `timer.getTimeout()` gets the remaining time
  // Which can be 0 if the timeout has run out
  await doLegacyThing(timer.getTimeout());

  while (true) {
    if (timer.timedOut) throw new ErrorTimedOut();
    await doSomething();
    break;
  }

}

It is up the function internally to deal with the timer running out and throwing whatever function is appropriate at that point. Remember it's purely advisory.

All we have to do here is to create a new class Timer that supports relevant methods:

class Timer {

  protected timer: ReturnType<typeof setTimeout>;
  public readonly timerP: Promise<void>;
  protected _timedOut: boolean;

  public constructor(time?: number) {
    // construct the timer
  }

  get timedOut(): boolean {
    return this._timedOut;
  }

  public start() {
  }

  public stop() {
  }

  public getTimeout(): number {
    // subtract elapsed time, from total timeout
    // return in ms how much time is left
  }

}

This would replace the timerStart and timerStop functions that we currently have in our src/utils.

Functions that take a Timer should also support a number as well as convenience, and this can be made more convenient just be enabling the time?: number parameter above. Note that by default if there's no time set for the Timer, we can either default in 2 ways:

1. 0 meaning the timer immediately times out
2. Infinity meaning the timer never times out

Certain functions may want to default to infinity, certain functions may wish to default to 0. In most cases defaulting to Infinity is probably the correct behaviour. However we would need to decide what getTimeout() means then, probably return the largest possible number in JS.

Because this is fairly general, if we want to apply this to our other libraries like js-async-locks, then this would need to be a separate package like js-timer that can be imported.

Cancellable Wrapper HOF and/or Decorator

The timer idea is just a specific notion of a general concept of asynchronous cancellation. In fact it should be possible to impement timers on top of general asynchronous cancellation.

There may be a need to cancel things based on business logic, and not just due to the expiry of some time.

In these cases, we want to make use of 2 things: AbortController and CancellablePromise.

The AbortController gives us an EventTarget that we can use to signal to asynchronous operations that we want them to cancel. Just like the timer, it's also advisory.

The CancellablePromise gives us a way to cancel promises without having to keep around a separate AbortController object. The same concept can apply to AsyncGenerator as well, so one may also have CancellableAsyncGenerator. Note that AsyncGenerator already has things like return and throw methods that stop async generator iteration, however they are not capable of terminating any asynchronous side-effects, so they are not usable in this situation.

The idea is to create a cancellable higher order function wrapper that works similar to promisify and callbackify.

The expectation is that a lower order function takes in options?: { signal?: AbortSignal } as the last parameter, and cancellable will automate the usage the signal, and return a new function that returns CancellablePromise.

This higher order cancellable has to function has to:

1. Preserve the input/output types of the lower order function - see the new promisify how to do this (note that overloaded functions will need to be using conditional types to be supported)
2. If the wrapped function is given an abort signal, this signal is preserved and passed through to the lower order function.
3. If the wrapped function is not given an abort signal, the wrapper must create the AbortSignal and connect it to the cancel* methods of the returned promise.
4. The returned CancellablePromise must be infectious, any chained operations after that promise must still be a CancellablePromise. Basically pC.then(...).then(...) must still return a CancellablePromise, even if then chained function returns a normal Promise.
5. The CancellablePromise has 2 additional methods: cancel and cancelAsync, the difference between the 2 is equivalent to our EventBus.emit and EventBus.emitAsync. You would generally use await pC.cancelAsync().
6. Inside the lower order function, if the signal is emitted, a cancel exception must be thrown. This exception will be expected to be caught in the cancelAsync or cancel methods of the cancellable promise. If no exception is thrown, then the method is successful because it is assumed that the promise is already settled. If a different exeption is thrown, then it is considered an error and will be rethrown up.
7. What is the cancel exception? This should be specifiable with an exception array in the higher order function. A default cancel exception can be made available if none is specified.
8. Be usable as a decorator, in some cases, such as class methods, it is best to have this as a @cancellable wrapper similar to our @ready decorator used in js-async-init.
9. Be applicable to AsyncGenerator as well, in which case the @cancellable would return a CancellableAsyncGenerator instead.
10. Be composable with promisify and callbackify.

An example of this being used:

// Assume that cancellable can take a variable number of args first
// If you don't pass any exceptions, it should default on particular exception
const pC = cancellable(
  ErrorCancellation,
  async (options?: { signal?: AbortSignal }) => {
    // Once you wrap it in a signal, then the signal is always passed in
    // however you don't know this in the lower order function
    if (signal?.aborted === true) {
      throw new ErrorCancellation();
    }
  }
);

await pC.cancelAsync();

Or with a decorator:

class C {
  @cancellable(ErrorCancellation)
  public async (options?: { signal?: AbortSignal }) {
    // ...
  }
}

You should see that it would be possible to implement the timer using this concept.

const pC = cancellable(
  ErrorCancellation,
  async (options?: { signal?: AbortSignal }) => {
    // Once you wrap it in a signal, then the signal is always passed in
    // however you don't know this in the lower order function
    if (signal?.aborted === true) {
      throw new ErrorCancellation();
    }
  }
);

setTimeout(() => {
  pC.cancel();
}, 5000);

The timer parameter would just be a convenience parameter to automate a common usecase.

In some of our functions, I can imagine that end up supporting both timer and signal:

class C {
  @cancellable(ErrorCancellation)
  public async (timer?: Timer | number, options?: { signal?: AbortSignal }) {
    timer = timer instanceof Timer ?? new Timer(timer);
    if (timer.timedOut) {
      // Maybe you want to differentiate that this got cancelled by the timer and not by signal?
      throw new ErrorCancellation();
    }
  }
}

If a cancellation error is thrown without actually be cancelled, then this is just a normal exception.

Refactoring Everywhere

Lots of places to refactor. To spec out here TBD.

Additional context

• Create IPC locking library @matrixai/js-file-locks to introduce RWlocks in IPC #290 - Locking operations can make use of this
• Implementing BIP39 recovery, Unattended Bootstrap and Agent Start and Status #283 (comment) - discussion about poll usage and the difference between timers

Tasks

1. Create Timer class into our src/utils
2. Integrate AbortController into Timer
3. Prototype Timer integration into poll utility
4. Prototype Timer integration in network proxies
5. Prototype Timer integration into identities
6. Prototype Timer into our tests and ensure that it works with jest mock timers

[CMCDragonkai]

Another place where this can be applied is the authenticate AsyncGenerator. We can cancel async generators by doing https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Generator/return or https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Generator/throw.

But the underlying pollAccessToken() will not be cancelled immediately. Instead it will loop until one of the exit conditions which includes a timeout.

So having a consistent way of aborting these sorts of operations would be useful.

Being able to abort when we cancel CTRL + C the pk identities authenticate github.com joshuakarp call would be useful.

[CMCDragonkai]

Note that we have a usecase in js-async-init that could make use of a general way of aborting asynchronous operations. It should be optional/advisory so that decorated classes can choose to make use of the abort signal indicated by a stop or destroy operation. If they ignore, that's fine, but if they take it into account, they should then also stop what they are doing and cancel whatever async operation they are doing.

This can be quite useful for blocking async generator methods, so they can cancel their async generator context when the stop signal is raised.

[CMCDragonkai]

It could also be 2 separate libraries, one for in-memory ("in/intra-process") locking, another for IPC locking.

[CMCDragonkai]

• https://github.com/MatrixAI/js-async-locks
• https://github.com/MatrixAI/js-file-locks

js-async-locks
@matrixai/async-locks - async-mutex, RWLock, and variants on that

js-file-locks
@matrixai/file-locks - fd-lock (needs to use C++ and NAPI to get posix or whatever)

[CMCDragonkai]

Some more interesting libraries to try:

• https://github.com/alkemics/CancelablePromise
• https://github.com/srmagura/real-cancellable-promise
• https://github.com/DigitalBrainJS/c-promise
• https://github.com/DigitalBrainJS/use-async-effect

[CMCDragonkai]

It would worth considering getting this solved #307 before this issue so we can actually prove that we are properly cancelling promises.

[CMCDragonkai]

Based on this comment srmagura/real-cancellable-promise#1 it seems that this package is the one we should be using.

[CMCDragonkai]

GRPC calls have cancel operation.

Web requests fetch AND node FS have have AbortController and AbortSignal.

Combining it with that cancellable promise wrapper, we may finally have the ability to properly cancel our async operations which can impact our Discovery system, Node Manager queue, async locks... pretty much all promises that rely on underlying side-effects/IO and not just computational promises.

Almost all uses of promises are IO/side-effectful promises though!

[tegefaulkes]

I'm having a look at CancellablePromise and trying to see how I could apply it to an existing method such as getClosestGlobalNodes.

To create a new CancellablePromise you need to wrap an existing promise like below. We can also break out of the loops by throwing the Cancellation.

    let cancelledFlag = false;
    const cancel = () => {
      // This needs to cause prom to throw an `Cancellation()` error.
      cancelledFlag = true;
    }
    const prom = () => {
      // Doing things
      while(true) {
        // We can break out of the loop with an error
        if (cancelledFlag) throw new Cancellation();
        // Do our things
      }
    }

    const cancellablePromise = new CancellablePromise(prom(), cancel);

But we need a way to chain cancellation by having our cancel callback call cancellablePromise.cancel(). We could do this with a simple event emitter?

    const cancellable: Array<() => void> = []
    cancellable.push(() => thing1.cancel());
    cancellable.push(() => thing2.cancel());
    cancellable.push(() => thing3.cancel());
    cancellable.push(() => thing4.cancel());
    cancellable.push(() => thing5.cancel());
    const cancel = () => cancellable.forEach( thing => thing());

so overall our cancellable methods would look something like this;

    const cancellable: Array<() => void> = [];
    let cancelledFlag = false;
    const cancel = () => {
      // This needs to cause prom to throw an `Cancellation()` error.
      cancelledFlag = true;
      cancellable.forEach( cancel => cancel())
    }
    const prom = () => {
      // Doing things
      while(true) {
        // We can break out of the loop with an error
        if (cancelledFlag) throw new Cancellation();
        // Do our things
        const cancellablePromise1 = thing1();
        cancellable.push(() => cancellablePromise1.cancel());
        await cancellablePromise1;
        const cancellablePromise2 = thing2();
        cancellable.push(() => cancellablePromise2.cancel());
        await cancellablePromise2;
      }
    }

    const cancellablePromise = new CancellablePromise(prom(), cancel);

[CMCDragonkai]

Just remember that to truely cancel the promise, you must also cancel the underlying side-effect.

The way to do it depends on what the final side-effectful op is. In most cases, they support an AbortSignal.

For example:

• https://developer.mozilla.org/en-US/docs/Web/API/fetch - AbortSignal
• https://nodejs.org/api/fs.html - AbortSignal
• GRPC client https://grpc.github.io/grpc/node/grpc-ClientWritableStream.html - use the cancel()
• For generators use the return() or throw() - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Generator/return (this "stops" the generator), the same thing happens for AsyncGenerator

But just because you stop the generator, doesn't mean you stop the underlying side-effect. We will need to propagate such calls to the underlying side-effect by calling cancel or something else.

[CMCDragonkai]

My thinking is that we should build in AbortSignal support into our specialised GRPC generator abstractions similar to https://github.com/alanshaw/abortable-iterator.

[CMCDragonkai]

Timer inheritance is technically a separate problem from having abortable asynchronous contexts. It's just often used together for our deadlines... etc.

In some cases I've supported passing a Timer object down thus preserving the same timer object. But this doesn't always work when the underlying system uses a timeout number. And it's not that portable to other libraries/packages. It can still be done with just a timeout number, you just have to subtract the time taken on each step. In many cases, I've wanted to enable the ability of functions to take Timer | number so that way you can pass a Timer object, or just a number to make it more convenient to use.

[tegefaulkes]

making things cancel-able seemts to be a pretty major change that will see us converting quite a few methods such as withConnF, withConnG, getConnection, establishConnection, GRPC client calls, ect. We will need to streamline the process. Possibly with a combination of a decorator and a cancel event emitter?

It could look something like;

@cancelable()
public async thing(arg1, arg2, cancelEventEmitter) {
    await otherThing(arg3, arg4, cancelEventEmitter);
    // Or
    const prom =  otherThing(arg3, arg4);
    cancelEventEmitter.on('cancel', prom.cancel());
    await prom;
}

It's easy enough to wrap a promise with CancellablePromise. It's just chaining the cancel event that is annoying. since the currently running promise has to be the thing to throw the Cancelled error.

[CMCDragonkai]

I'd prefer that cancellable promises are specifically implemented in only areas where we need it. Not whole-sale conversion.

async function doSomething() {
  await thingThatIsCancellable();
  await thingThatIsNot();
}

It would make sense that by using thingThatIsCancellable, it should propagate to doSomething itself being cancellable too.

Would it be done through an additional parameter like AbortSignal?

That's how alot of existing async methods do the job like fetch and fs.*.

In other cases like async generators, you would need integration of the AbortSignal as well.

[tegefaulkes]

We don't have to convert everything wholesale. But connection related procedures such as findNode, refreshBucket and possibly pingNode should be cancel-able. I suppose we don't have to go as deep as cancelling any active GRPC calls or connections currently being created. Just the worst offenders that are creating connections / preforming discovery using a loop.

As for how it's implemented. We would have to pass a cancel event emitter, AbortController or an AbortSignal as a parameter to any cancellable function. so long as we have a standard way of handling it we can wrap it in a CancellablePromise using a decorator.

Looks like using the AbortController/AbortSignal API is the best option but it doesn't seem to be fully supported by Node 14.x. We may have to update the node version if we want to go down that route. Or we could use a package for it. https://www.npmjs.com/package/node-abort-controller

[CMCDragonkai]

NixOS/nixpkgs#146440 this is the PR waiting for to upgrade our nodejs.

[CMCDragonkai]

AbortSignal is an extended class of EventTarget.

The EventTarget is the JS/Ecmascript version of EventEmitter that nodejs has. JS/Ecmascript doesn't have EventEmitter so they created their own EventTarget.

If we want to be browser-compatible, we should prefer to use EventTarget over EventEmitter (it does have some differences though).

So in order to do this, our cancellable asynchronous functions has to take an AbortSignal like this:

async f (p1, p2, { signal: AbortSignal }): Promise<void> {
  await g({ signal });
  while (true) {
    // do stuff while waiting on the abort event
  }
  await h({ signal });
}

The standardised interface is that the last parameter is an "options" object that has a signal property being AbortSignal. It has to be optional, both for the object and the signal itself.

options?: { signal?: AbortSignal }

Functions that can be aborted needs to listen on the abort event, that triggers a boolean switch in the computational context that causes it to throw an aborting exception.

It's the exception that bubbles up and cancels/stops all computation within an abort controller context.

The abort handler itself cannot be throwing an exception, as that exception is in its own context, and not in the call stack of the function(s) being aborted.

To integrate with the cancellable promise, one has to create a higher order function like this:

const pc: CancellablePromise = wrap(f.bind(context))(...params)

The wrap function must take f, make sure that it is bound to the right context first, and will create an abort controller and inject the abort signal into functions that have a standardised interface. Then give back a function that takes the internal function's parameters.

The wrap is a higher order function that partially applies the last parameter by setting the signal property.

To be flexible it also needs to do 3 things:

1. Be able to propagate a signal from an outside context to maintain the same abort controller context.
2. Be able to chain the abort event if they are separate abort controller contexts
3. Be able to merge options passed in from the returned function to the { signal } that it is supplying

The result is a cancellable promise which is somewhat easier to use than having to manage a bunch of AbortController objects around. Basically the CancellablePromise just works like a normal Promise, but with an additional method enabling you abort it.

It's important to note that aborting is asynchronous, calling abortController.abort() doesn't immediately abort. But potentially we could have an "await" for abort on our cancellable promise. Like await promiseCancellable.cancel().

This is similar to our events system with emit and emitAsync. Imagine:

promiseCancellable.cancel()
await promiseCancelable.cancelAsync()

[CMCDragonkai]

One way to implement cancelAsync would be:

  async cancelAsync(): Promise<void> {
    this.cancel();
    try {
      await this;
    } catch (e) {
      if (e instanceof AbortionError) return;
      // do we want to do this? What does `emitAsync` do?
      throw e;
    }
    // If already finished, and no exception, then just return
    // Or throw an error indicating that it is already finished (can't cancel what is already finished)
    return;
  }

[CMCDragonkai]

As for the wrap function:

type F = (...params: [...Array<unknown>, { signal: number }] | [...Array<unknown>]) => unknown;

function wrap<T extends F>(f: T): T {
  throw new Error();
}

function f1(a: number, options: { signal?: number, x: number } = { x: 4 }): number {
  return options.signal ?? 123;
}

function f2(a: number, options: { signal?: number, x: number }): number {
  return options.signal ?? 123;
}

function f3(a: number, options: { signal: number }): number {
  return options.signal;
}

function f4(a: number, options: { signal: number } = { signal: 4 }): number {
  return options.signal;
}

const g1 = wrap(f1);

const g2 = wrap(f2);

const g3 = wrap(f3);

const g4 = wrap(f4);

Remember this means the resulting function can still pass signal down. What does this mean though? Does it override the signal used internally by wrap? Is it how we "inherit" signals? Is it how do we chaining...?

[CMCDragonkai]

I'm testing explicit signal cancellation during the timed decorator usage. And I want to figure out if this means that the timer should be cancelled if already signalled to abort, or if during abort signal if it should immediately cancel the timer and not wait for the function to finish? This might clean up resources quicker.

[CMCDragonkai]

This would only apply to situations where the signal is passed in from the outside. The signal may already be aborted, and therefore the timer should not be constructed, or even just cancelled straight away.

We can make use of a special symbol as the reason for explicit cancellation and ignore such reasons when given.

[CMCDragonkai]

Currently timer.cancel() causes an unhandled promise rejection.

We should make timer.cancel() similar to the PromiseCancellable so that cancel does not result in an unhandled promise rejection.

This also makes it closer to how we expect clearTimeout to work. Simply cancelling a timer should not be considered a promise rejection.

Promise rejection can still happen but only to things chained after the timer promise (which then has to handle a rejection).

Making this would simplify the timed decorator in not having to catch and ignore explicit timer cancellations.

[CMCDragonkai]

Actually to be precise, this also causes an unhandled promise rejection:

    const pC = new PromiseCancellable<void>((resolve, reject) => {
      setTimeout(() => {
        resolve();
      }, 10);
    });
    pC.cancel('some reason');

It behaves the same as a regular Promise in this regard:

    const pA = new Promise((resolve, reject) => {
      reject('abc');
    });

Both are unhandled rejections. I think this is fine for PC.

But for Timer, I would want cancellations to not be unhandled rejections just to make it easier to use it like a regular setTimeout.

[CMCDragonkai]

This is now done by doing: void this.p.catch(() => {}); before this.p.cancel(reason);

  /**
   * Cancels the timer
   * Unlike `PromiseCancellable`, canceling the timer will not result
   * in an unhandled promise rejection, all promise rejections are ignored
   */
  public cancel(reason?: any): void {
    void this.p.catch(() => {});
    this.p.cancel(reason);
  }

Note that this means when cancelling the timer, all resolutions and rejections are ignored. We don't distinguish explicit cancellation from other kinds of errors that may occur. This is may be a controversial decision, but I tried to distinguish explicit cancellation, but it doesn't work because the catch handler will always get actual rejection which is supposed to be the user-passed reason.

During multiple cancellation, other reasons may be supplied, and we don't want to make those fail just because the reasons are different.

Therefore timer.cancel() applies to both eager and lazy timers. Rejections are just ignored.

Finally this doesn't apply to PromiseCancellable itself. The cancel() would result in a promise rejection that is unhandled if there's no handler for it. This aligns with Promise behaviour. This seems ok simply because if you're directly using PromiseCancellable, you're likely awaiting on the behaviour somewhere and not just orphaning promises.

I guess this problem comes down to whether PromiseCancellable should work like a "fire and forget" asynchronous side effect. Atm, it's not fire and forget due to the unhandled promise rejection error. And therefore acts the same as Promise.

[CMCDragonkai]

Ok so with the new exception class creation, we get something like:

Error
    at Timer.handler (/home/cmcdragonkai/Projects/Polykey/src/contexts/decorators/timed.ts:49:40)
    at Timer.fulfill (/home/cmcdragonkai/Projects/Polykey/src/timer/Timer.ts:247:35)
    at Timeout.<anonymous> (/home/cmcdragonkai/Projects/Polykey/src/timer/Timer.ts:144:52)
    at listOnTimeout (node:internal/timers:559:17)
    at processTimers (node:internal/timers:502:7)

As you can see, this stack trace is pretty useless, doesn't even tell us which decorated function actually throw the error. So we're going to need to redo the stack trace anyway.

[CMCDragonkai]

Ok we have a problem with resetting the stack trace. The issue is that the when the error occurs due to a timeout, it is occuring inside a process timer stack. At that point it's not possible to reset the stack trace according to the decorated function. This is because the stack information no longer exists while in scope of the timers.

The alternative is to construct the exception before hand, and then throw it inside the timer.

This ensures that the stack trace now looks like:

ErrorCustom
    at setupContext (/home/cmcdragonkai/Projects/Polykey/src/contexts/decorators/timed.ts:50:15)
    at C.f (/home/cmcdragonkai/Projects/Polykey/src/contexts/decorators/timed.ts:162:33)
    at main (/home/cmcdragonkai/Projects/Polykey/test-expiry.ts:29:13)
    at Object.<anonymous> (/home/cmcdragonkai/Projects/Polykey/test-expiry.ts:36:6)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Module.m._compile (/home/cmcdragonkai/Projects/Polykey/node_modules/ts-node/src/index.ts:1618:23)
    at Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Object.require.extensions.<computed> [as .ts] (/home/cmcdragonkai/Projects/Polykey/node_modules/ts-node/src/index.ts:1621:12)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)

This gives us more useful information as it is now telling us where exactly this exception was triggered, and specifically at:

    at main (/home/cmcdragonkai/Projects/Polykey/test-expiry.ts:29:13)

However this is exposing a bit too much useless information like:

    at setupContext (/home/cmcdragonkai/Projects/Polykey/src/contexts/decorators/timed.ts:50:15)
    at C.f (/home/cmcdragonkai/Projects/Polykey/src/contexts/decorators/timed.ts:162:33)

Comparing this to the async-init @ready. There are possible places to put it:

1. When it is placed at the decorator declaration like @timed(new Error()). This tells us which part of the code it is declared, but not where it is called.
2. When it is placed inside the decorator - now it shows where it is instantiated and the trace shows where it is called, but it has too much detail.
3. With a reset of the stack trace, we can get it showing exactly where it was called rather than where it was instantiated. This requires using the decorated function, not the target function, that is descriptor['value'] not just f.

I think the main issue is that doing the resetStackTrace is slow. Usually the stack trace is only created when the stack property is accessed. So doing it on every call ahead of time is not good.

So the only good solution is the middle solution, instantiate the exception as part of the decorator, but not as part of the the timer. And if I find a good way to reset the stack trace but only when throwing it....

[CMCDragonkai]

Actually it appears captureStackTrace does have good performance. I don't think it actually slows down, until you were to capture the stack information itself. So we can optionally make use of it here if it is available.

[CMCDragonkai]

So basically with:

        const errorTimeout = new errorTimeoutConstructor();
        if (utils.hasCaptureStackTrace) {
          Error.captureStackTrace(errorTimeout, descriptor['value']);
        }

And putting that inside each decorated descriptor['value'] function, we're able to get:

/home/cmcdragonkai/Projects/Polykey/test-expiry.ts:30
    await c.f();
            ^
ErrorContextsTimedExpiry
    at main (/home/cmcdragonkai/Projects/Polykey/test-expiry.ts:30:13)
    at Object.<anonymous> (/home/cmcdragonkai/Projects/Polykey/test-expiry.ts:36:6)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Module.m._compile (/home/cmcdragonkai/Projects/Polykey/node_modules/ts-node/src/index.ts:1618:23)
    at Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Object.require.extensions.<computed> [as .ts] (/home/cmcdragonkai/Projects/Polykey/node_modules/ts-node/src/index.ts:1621:12)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:77:12)
    at phase4 (/home/cmcdragonkai/Projects/Polykey/node_modules/ts-node/src/bin.ts:649:14) {
  timestamp: 2022-09-01T11:11:30.802Z,
  data: {},
  cause: undefined,
  exitCode: 69
}

And this matches how the @ready shows up atm.

[CMCDragonkai]

Actually I'm going to remove the captureStackTrace. I actually prefer more detail here atm. It will aid in debugging. So we're sticking with:

/home/cmcdragonkai/Projects/Polykey/src/contexts/decorators/timed.ts:49
    const e = new errorTimeoutConstructor();
              ^
ErrorCustom
    at setupContext (/home/cmcdragonkai/Projects/Polykey/src/contexts/decorators/timed.ts:49:15)
    at C.f (/home/cmcdragonkai/Projects/Polykey/src/contexts/decorators/timed.ts:160:33)
    at main (/home/cmcdragonkai/Projects/Polykey/test-expiry.ts:30:13)
    at Object.<anonymous> (/home/cmcdragonkai/Projects/Polykey/test-expiry.ts:36:6)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Module.m._compile (/home/cmcdragonkai/Projects/Polykey/node_modules/ts-node/src/index.ts:1618:23)
    at Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Object.require.extensions.<computed> [as .ts] (/home/cmcdragonkai/Projects/Polykey/node_modules/ts-node/src/index.ts:1621:12)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)

[CMCDragonkai]

Note that import type { ContextTimed } from '@/contexts/types'; gets rewritten to import { ContextTimed } from '@/contexts/types';. This is intentional from eslint consistent type imports. Something about how TS uses types to change the runtime information. Atm, nothing can be done about this.

[CMCDragonkai]

The cancellable defaults to lazy false just like all the other things. So by default cancelling is eager, doesn't wait for the target function to actually settle.

Initial syntax tests are working.

One thing though, if the target function returns a PromiseCancellable, this is not special cased, and treated like any other promise. So @cancellable @cancellable would double wrap the promise. Kind of unnecessary though.

[CMCDragonkai]

For now the @cancellable will only work for PromiseLike functions unlike @timer which works for any function.

[CMCDragonkai]

The HOF variants will work like this:

// import cancellable from '@/contexts/decorators/cancellable'; // <- this is for decorators
import cancellable from '@/contexts/functions/cancellable';

const f = async (a, b, c, ctx): Promise => { };

// Without the `@context` param decorator, we need to have the `ctx` as the first parameter
const g = cancellable(
  (ctx, ...params) => f(...params, ctx)
);

// Maybe a type like this:
// <T extends Array<any>, V> (
//   (...params: [ContextCancellable, ...T]) => PromiseLike<V>) => 
//     (...params: [...T, ContextCancellable]) => PromiseCancellable<V>

g(a, b, c, ctx);

It will need to preserve the internal types of params.

Note that this would be complicated if the target function has overloaded types. It wouldn't really preserve it.

[CMCDragonkai]

This answer https://stackoverflow.com/a/52760599/582917 suggests providing the most general overload if this occurs. Seems like a good practice in case overloaded signatures exist. The implementation signature is ignored remember.

[CMCDragonkai]

Ok I was right, the types actually work for if I put ctx at the end, however due to the lack of RTTI, there's no way to know the right position to put the ctx, and thus the HOF variants lacking the parameter decorator have to have the ctx in front.

So types-wise it does work, the inference makes sense. But once you get the args the args[args.length - 1] does not guarantee that it's the last parameter for the function. It's just the last argument passed in.

Then f.length is not guaranteed to give you the length cause of rest parameters.

[CMCDragonkai]

So this is how you use it:

    const f = async function (
      ctx: ContextCancellable,
      a: number,
      b: number,
    ): Promise<number> {
      expect(ctx.signal).toBeInstanceOf(AbortSignal);
      return a + b;
    };
    const fCancellable = cancellable(f);
    const pC = fCancellable(undefined, 1, 2);
    await pC;

By having ctx as the first parameter, it does mean using undefined is necessary if you don't want to pass in a context.

[CMCDragonkai]

Timed and Cancellable HOF variants are done and tested. Moving to combinations of the 2 timedCancellable.

[CMCDragonkai]

As per the comment #438 (comment), the timedCancellable is the only combination worth having since it can simulate cancellableTimed, but cancellableTimed cannot simulate timedCancellable.

[CMCDragonkai]

The generic types of timed and cancellable has been reworked to make it composable. However it's not exactly perfect.

To ensure a standardised usage of timed and cancellable, it would be good to have @timedCancellable that should be used across the board.

It didn't actually apply to the TaskManager system at the end of the day, it was easier to use the underlying primitives directly. However for other domain methods, we should start applying @timed and @cancellable where it is applicable.

May also get @timedCancellable into #438 time permitting, since the goal is to the tasks system working, and then merge. If tasks system is already working, it would enough to merge without @timedCancellable.

[CMCDragonkai]

The js-timer now a separate library: https://github.com/MatrixAI/js-timer

CI/CD is setup for that https://gitlab.com/MatrixAI/open-source/js-timer

[CMCDragonkai]

It can be introduced to Polykey and eliminate the timer domain.

[CMCDragonkai]

Ok so I have a bit of a confusion regarding timed and timedCancellable.

The problem is that both of these decorators will use ContextTimed:

type ContextCancellable = {
  signal: AbortSignal;
};

type ContextTimed = ContextCancellable & {
  timer: Timer;
};

So there are 2 relationships here:

A. If timer times out, signal is aborted
B. If signal is aborted, then timer is cancelled

However we have 4 situations:

1. Nothing is inherited
2. Inherit signal
3. Inherit timer
4. Inherit timer and signal

This is already being done in the timed decorator. But I realise now that signal abortion is now also triggered by PromiseCancellable cancellation. So this has revealed to me I don't fully understand the implications here.

[CMCDragonkai]

So one thing, is that regardless of the 4 situations, A. relationship should old. If the timer times out, the signal to the target wrapped function should always be aborted. The abortion signal reason should new errorTimeoutConstructor() which is defaulted to contextErrors.ErrorContextsTimedTimeout.

Note that this does not mean that the promise is rejected. Whether the promise rejects or not depends on the actual function itself. Of course this is where the cancellable lazy option controls.

So in situation 1, relationship A will be setup by the decorator.

In situation 2 and 3, the relationship A will also be setup by the decorator. In 2, the new timer will abort a chained signal. In 3, a timer fulfilment will lead to signal abortion.

In situation 4, where both timer and signal are inherited. This is where things get complicated. Do we assume that the timer and signal have the relationship already setup? Because that assumption only holds if we expect the context object was passed in from an outer decorator.

But if the user just created a Timer and AbortSignal and passed it in, no relationship has been setup.

We could assume that if you did that, you did it intentionally for some reason, and the decorator won't do any magic. Alternatively the decorator can check if there is a relationship, however it's not really able to do this since the signal does not tell you if there are event handlers pointing to the timer. I think we have to assume that such an explicit passing means that you have already explicitly setup the relationship A. And if you did not, then it's a runtime error, and we cannot do anything here.

Furthermore... in situation 1, 2, 3, 4 a new downstream signal must be created due to cancellable. Otherwise there's no way for the PromiseCancellable to control the cancellation.

The next problem is relationship B. In some cases, I can see that if the signal aborts, this results in the timer being cancelled (not timed out!!). This is because if the signal is already aborted, there's no need to keep the timer running, since the timer will eventually just timeout and abort the signal. Relationship B should hold in: situation 1, situation 2.

But in situation 3, and 4, the timer is inherited, and now the problem is that with PromiseCancellable, this results in the possibility that cancelling a promise results in cancelling the timer that was inherited. Which may not be what the user expects, if that timer is going to be passed into another asynchronous operation.

Relationship B isn't actually that important. Once the signal is aborted, even if the timer times out and tries aborting the signal, its operation will be a noop. The reason won't change either. A signal can only be aborted once, subsequent aborts are ignored. And we know that the timer will eventually be cancelled once the async operation completes.

We can say that relationship B only holds if the decorator constructed the timer. If it he decorator did not construct the timer, then the signal does not try to cancel the timer. When the decorator constructs the timer, the timer is not exposed to the outside, and this is a safe operation.

Finally there's the issue of the function completion, when the function completes, it's necessary to cancel the timer to prevent it from continue to running. But we don't do this, if the timer was inherited. This is only done if the timer was constructed by the decorator itself and thus the decorator "owns" the lifecycle of that timer. This would only occur in situation 1 and 2.

So let's call this relationship C: if a timer is constructed by the decorator, then it must be cancelled at the end by the decorator

Consequently therefore:

1. Relationship A holds in all 4 situations: If the timer times out, the signal is aborted.
   • Caveat on situation 4, it is assumed that this relationship is already setup, and therefore no handler is registered to do this.
2. Relationship B holds only in situation 1 and 2: If the signal is aborted, the timer is cancelled early.
3. Relationship C holds only in situation 1 and 2: If the timer is constructed by the decorator, then it must be cancelled at the end of the asynchronous operation.

Separately whether the async op does an early rejection or not depends on the laziness.

[CMCDragonkai]

Currently relationship B is applied in situation 4. This is where things get a bit complicated.

I said that it should be expected that the caller has already setup the relationship. Where if the signal aborts, then therefore the timer should be cancelled.

But if that's the case, why is relationship A also not enforced? It's because it would require us to chain the signal to enforce that, and chaining the signal is what we do in situation 2.

Now is it actually a problem to readd these relationships? Yes it can be...

Consider if we created a nested series of async functions, each propagating the context down. That means everything except the outermost function is in situation 4. And if the relationships are added over and over, this will lead to multiple calls to signal abortion during a timeout, and also multiple calls to timer cancellation during signal abortion. Every function call will result in an additional 2 handlers being added.

This problem may be solved if we were able to know if the relationship exists or not instead of making assumptions. Unfortunately the signal event target does not give us a way to interrogate the event listeners. And the timeout while giving us a handler, doesn't really tell us what the handler is doing without doing meta work on the handler code.

Therefore relationship B should not be applied to situation 4. And the tests that rely on this should expect to fail because they are calling with a incorrectly setup context object.

[CMCDragonkai]

Async cancellation should be applied across the board:

1. Replace polling with Timer
2. Replace racing with Timer integration in network proxies
3. Timer integration into identities

For locking MatrixAI/js-async-locks#21 illustrates the problem is more complex.

Further R&D required to deal with cancellable async generators or generators... let's see how we go first.

[CMCDragonkai]

Issue description is still useful to indicate where changes still need to be applied.