matrixai / js-async-init Goto Github PK

Is your feature request related to a problem? Please describe.

The error stack trace is created when the error instance is constructed, not where it is thrown.

Atm, we fix this by overriding the error instance's stack trace at thrown time.

Describe the solution you'd like

If we take a class instead, we can just instantiate the class and then expect that the error instance is constructed. Even more general would be a function returning an error instance...

This will make the stack trace start from where it is thrown. But we may still need to "reset" it according to the decorated function, as that will give us a cleaner trace... but it assumes we don't need to know exactly where it is thrown. It's a bit more "magical".

I'm already trialling this out in the timed decorator and other context decorators.

Additional context

• MatrixAI/Polykey#297

Specification

The decorators currently take error objects instead of error classes.

This causes the stack trace to be set to when the error object is constructed. These stack traces are not useful for debugging.

We should replace the stack property with a stack that would be created when something goes wrong.

To do this, everywhere we are doing throw ...; we should replace it with:

errorDestroyed.stack = new Error().stack ?? '';
throw errorDestroyed;

We can wrap this with a utility function.

Additional context

Tasks

1. create utility function for updating the stack information in the error.
2. Update all @ready, @startstop, ect decorators to provide the useful stack information when throwing the error.
3. add test to check if the updated stack information is provided.

Describe the bug

When using CreateDestroy or CreateDestroyStartStop, it is necessary for the async creation function to use new this().

If you use new ClassName() then it will fail due to the constructor not referring to the decorated class constructor.

This only seems to occur with SWC, TSC works fine.

I have a memory of having reported this already upstream to SWC, but evidently it's not fixed and I cannot find the issue I reported or saw...

@tegefaulkes

To Reproduce

1. Create a prototype script with class X with CreateDestroyStartStop and a @ready method
2. Run it with npm run ts-node -- ./test.ts
3. See the result between swc enabled or disabled
4. Change to new this and it should work!

Expected behavior

The SWC should behave the same as TSC.

Specification

The ready decorator right now prevents execution unless the class is ready, that is after start is finished, and before stop is called.

The problem here is that there are many methods that need to be callable publically and therefore has ready applied, but also needs to be called in the start method.

Of course the reason for this is to prevent inadvertent usage of the public method when the side-effects of start hasn't completed. However in some situations, the programmer can be responsible for ordering the side-effects in the start method.

So the ready method should have an additional parameter to allow it to run in the other statuses other than _running.

Note that this parameter would make block not work. Because block ensures that the function would wait until the initLock is freed, as the ready methods acquire a read lock.

This parameter can be called:

// ready(errorNotRunning, block, allowedStatuses: Array<Status>)
ready(error, false, ['starting'])

Then you can allow a ready method to also be called when it is 'destroying', 'starting' or 'stopping' or null.

Note that null means that the none of the statuses are occurring. So generally speaking, allowing null means that it is also allowed when the class itself isn't starting, stopping or destroying, and whether or not is running.

Right now the methods are doing this:

if (this[initLock].isLocked()) {
  throw errorNotRunning;
}
if (!this[_running]) {
  throw errorNotRunning;
}

The above would need to change to allow the statuses as exceptions, even if the init lock is locked, or that running isn't true.

Additional context

• MatrixAI/Polykey#310 - affects syncNodeGraph, if this can be done, this call be part of the start() call of NodeConnectionManager rather than being a separate call.

Tasks

1. ...
2. ...
3. ...

Specification

Appears to be problems using async init decorators on abstract classes. Should investigate if this is the case and add tests.

Tasks

1. - Prototype the application of decorator on abstract classes
2. - Fix
3. - Add abstract class test including extended child class being able to use the decorated methods

Specification

While developing the timed decorator, we discovered that the decorator may decorate a regular function that also returns a promise, generator or async generator. In such a case, the same behaviour should be applied as if it was an async function, generator function or async generator function.

This just needs to be done in the last block:

      descriptor[kind] = function (...args) {
        if (allowedStatuses.includes(this[_status])) {
          return f.apply(this, args);
        }
        if (this[initLock].isLocked('write') || this[_destroyed]) {
          resetStackTrace(errorDestroyed, descriptor[kind]);
          throw errorDestroyed;
        }
        return f.apply(this, args);
      };

Where after doing f.apply(this, args) we should be doing:

        const result = f.apply(this, params);
        if (utils.isPromise(result)) {
          // do as a promise
        } else if (utils.isIterable(result)) {
          // do as a generator
        } else if (utils.isAsyncIterable(result)) {
          // do as an async generator
        } else {
          // as usual
        }

We will need the conditional checks like isPromise, isIterable, isAsyncIterable.

Consider that you may only need isPromiseLike, and not isPromise because you only need then then function.

It may require some refactoring of the code to avoid too much duplication.

This will allow us to do:

class C {
  @ready()
  f(): Promise<void> {
    // ...
  }

  @ready()
  async g(): Promise<void> {
    // ...
  }
}

The C.f and C.g should behave the same with respect to readiness.

Additional context

• MatrixAI/Polykey#297 (comment) - first applied here to timed decorator

Tasks

1. ...
2. ...
3. ...

Specification

We can use RWLock so that when running certain ready operations, they actually block the start, stop, destroy.

To do this, we need to use read locks on all other operations, and write locks in start, stop, destroy.

It would only apply to async functions and async generators.

Releasing the lock would only happen at the end of the async generator.

async function* asyncGenerator() {
  let i = 0;
  while (i < 3) {
    yield i++;
  }
}

async function* testthis () {
  try {
    yield * asyncGenerator();
  } finally {
    console.log('done');
  }
}

async function main () {
  for await (const a of testthis()) {
    console.log(a);
  }
}

main();

We would take the RWLock from js-polykey. However we need to upgrade it. We need to use the acquire and release API as the functional API doesn't always work. Such as for the async generators.

Additional context

• MatrixAI/Polykey#292 - The compose method can then use just use ready(..., true); to indicate blocking operation.

It seems by default blocking can be true, since this is usually what you want, but it does change the semantics of all ready uses. Maybe that's ok. This can be integration tested by using npm link.

Tasks

1. Upgrade RWLock with acquire and release API
2. Apply the block parameter instead of wait parameter
3. Test all decorators and decide whether it is block true or false by default