WIP: Events Refactoring (integrating js-events)

Description
https://github.com/MatrixAI/js-events provides a Evented decorator and EventAny that allows one to depend on all relevant of a given object.
This results in:

No longer single inheritance of EventTarget, instead we use a mixin.
Classes aren't supposed to extend EventTarget anyway
Encapsulated evented systems can have all their events listened for by just doing addEventListener(callback) without any type key string.
The declaration of events should now be Event... basically starting with an Event prefix similar to Error... prefix for errors.
The names of errors should statically checked by relying on the EventClass.name static property, rather than having to make up event names

Issues Fixed

Related to https://github.com/MatrixAI/Polykey/pull/552
Related https://github.com/MatrixAI/Polykey/issues/444

Tasks

[x] 1. Replace all events with Event... prefixed class names.
[x] 2. ~Use @Evented() as class decorator for all things that use extends EventTarget~ - the js-async-init 1.9.x is now integrated with js-events
[x] 3. ~Use addEventListener(callback) instead of hardcoding all the relevant event names~ - Can use EventDefault.name instead.
[ ] 4. Encapsulated evented objects should have all their events captured and handled, the ones it cannot handle needs to be re-emitted
[x] 5. The events.ts isn't properly setting up the event class hierarchy, they should be extending the abstract classes for the grouped events.
[x] 6. Update to node v20 (including pkgs.nix, shell.nix, scripts for win and mac)

Final checklist

[ ] Domain specific tests
[ ] Full tests
[ ] Updated inline-comment documentation
[ ] Lint fixed
[ ] Squash and rebased
[ ] Sanity check the final build

If the QUICSocket is encapsulated in the QUICServer. Then stopping the QUICServer should stop the QUICSocket. And this will result in an emission of EventQUICSocketStop.
If QUICSocket is injected into QUICServer. Then stopping the QUICServer should not stop the QUICSocket. And this will not result in an emission of EventQUICSocketStop.
When EventQUICSocketStop is emitted. It has to be listened by QUICServer whether it is encapsulated or injected. This event could emit at any time due to any problem or user-initiated call. Therefore we must proceed to then stop the QUICServer.
One issue is that when stopping the QUICServer, we are also stopping the QUICSocket. Which results in an emission of EventQUICSocketStop. If we handle this to stop the QUICServer, don't we have an infinite loop?
Well atm, it's a no, because due to StartStop and associated decorators, we are using locks to block and furthermore conditions to coalesce these calls. So the infinite loop is terminated by the async-init decorators.
However as a general pattern, we shouldn't depend on the async-init in this situation. There could be other situations where an event is emitted from another object that we need to handle, which causes to execute a pull-method which triggers another event of the same type, thus creating an infinite loop.
Therefore, rather than simply relying on async-init to cancel the infiniteness (not only is this relying on implicit behaviour, but it's not even guaranteed in the future). We need something else as a general pattern to prevent such things.
Here are some possibilities.

We stop listening for the EventQUICSocketStop when we perform the quicSocket.stop(). - This is rather error prone, as we it requires us to keep track of this context every time we call quicSocket.stop(). And I err against the side of things that require too much context to keep track of.
Could the handler itself terminate with a base case? How would it decide? It cannot decide based on whether QUICSocket is injected or encapsulated, because in either case the EventQUICSocketStop could be emitted. Perhaps instead it can rely on a certain state-variable representing the execution of QUICServer.stop?
Other solutions?

Note that solution 2 means rather than just calling void this.stop(), it would rely on checking if the status of the system is stopping. Like if (this[stopping]) { ... }.
Is this actually any better than solution 1? Or just relying on void this.stop since the decorator manages it.
I think it is better than solution 1 as it centralises logic.
It's better than simply relying on this.stop from the decorator because it is explicit and gives a model for future operations.
The presence of infinite loops here is primarily due to this 1 reason: the inability to easily distinguish if an event happens due to something I'm already doing. The only good solution is to rely on status conditionals representing the state of doing that operation.

I think it ties into the idempotent nature of the stop method. In this case it's handled by the decorator. but as a general rule, for things like this, it can only really stopped or destroyed once. So the stop or destroy event can only be emitted once.
In the case of stop, it's emitted only once per state change since it could be started again. In that case it still prevents the cycle.
This might generalise as, any event that is the result of a state change, can only emit once per state change? we can refer to them as state events or... Once-lers? In any case, how does that sound for a general rule?

Now on the topic of encapsulated events. Where QUICServer encapsulates QUICSocket. Another design decision is about how encapsulated events should be emitted out of QUICServer.
Firstly there will be "relevant" and "irrelevant" events to QUICServer.
The "relevant" events are events that QUICServer must react to.
The "irrelevant" events are events that QUICSocket does not need to react to.
In both cases, there's a question about whether such events should be bubbled upwards outside of QUICServer.

Does an event matter to the outside world? Is QUICSocket an entirely encapsulated system?
If we do bubble it up, how do we do this? Currently Event is not allowed to be re-dispatched. We can only either dispatch a new event that wraps the original event by setting the original event as the detail property of the new event, or we redispatch the original event by instantiating a new class of that same event, and copy any detail to this new instance.

In terms of 1. even if we could argue that certain events shouldn't be relevant. Dispatching such an event is still useful for "observability". Where it can be useful to debug the internal state of the system, such as for tracing. The question is whether such tracing should make use of these events, or if a separate system should be used for such a thing. It also important to note that events could represent "pre-" or "post-" op events. However we defaulted to post- op events to match the convention of Node's event emitters and most other JavaScript evented systems.
In terms of 2., dispatching an event chain can help maintain a trace of these concurrent operations, similar to maintaining a trace of exceptions that was thrown, caught and rethrown. Exceptions are a bit easier since you can rethrow the same exception. I think that there are 2 equivalences:

Rethrowing an exception is equivalent to re-dispatching the same event class with the same details. (Since you cannot redispatch the exact same instance).
Throwing up a new exception chaining the old exception is equivalent to dispatching a new event wrapping around the old event.

Based on this https://gist.github.com/CMCDragonkai/08266b1463158f4156f66d4bf077add6 for exceptions, I think the same guidelines apply here.
# EXCEPTION RERAISE/RETHROW
# this reraises the same exception, in this case
# it just means you couldn't handle the exception
try:
    raise Exception
except Exception as e:
    raise e
    # raise # this is equivalent and you won't need to bind the exception

# EXCEPTION HANDLING EXCEPTION
# this raises another exception in the handling of the first exception
# python will recognise this and report it as so
# only do this if you really have an exception while handling an exception
try:
    raise Exception
except Exception as e:
    raise Exception

# EXCEPTION CHAIN
# this raises a new exception and chains the new exception with the old exception
# use this when your code represents an onion of different layers each capable of handling different exceptions
try:
    raise Exception
except Exception as e:
    raise Exception from e

I think both would be applicable depending on the situation.
In the case of QUICSocket and QUICServer, I think due to informational tracing, it would make more sense to just re-dispatch with the same class. This would be most of the events that we would want to redispatch. Then only in certain cases would we want to chain it for some reason.

I think it ties into the idempotent nature of the stop method. In this case it's handled by the decorator. but as a general rule, for things like this, it can only really stopped or destroyed once. So the stop or destroy event can only be emitted once.
In the case of stop, it's emitted only once per state change since it could be started again. In that case it still prevents the cycle.
This might generalise as, any event that is the result of a state change, can only emit once per state change? we can refer to them as state events or... Once-lers? In any case, how does that sound for a general rule?

I think using once: true would actually in fact make sense here if the event handler could be specialised to that specific event that could trigger an infinite loop.
In situations where you have a common event handler, then once won't work because there could be other events you want to listen and handle all the time repeatedly.

So it's possible to do this simultaneously:

Listen for "relevant" events, and handle it appropriately, as well as use once for things to terminate any infinite loops.
Listen for all events, and then the expectation is that all the events are "irrelevant", and thus can just be re-dispatched with the same class. The same-class redispatch can be done... but one then has to guarantee the signature of instantiating events. To do this, we can provide a clone method as part of a top-level root Event class. Similar to js-errors we can do AbstractEvent and provide a clone method.

I'm using this as scaffolding for the event with clone method for now. It will be replaced with your implementation.
type OptDetail<T> = T extends undefined ? {} : { detail: T };

class TempBaseEvent<T = undefined> extends Event {
  public readonly detail: T
  constructor(type: string, protected options?: EventInit & OptDetail<T>) {
    super(type, options);
    // @ts-ignore: Detail should be undefined here if it doesn't exist
    this.detail = options?.detail;
  }

  public clone(): this {
    // @ts-ignore: cheeky self prototype reference
    return new this.constructor(this.options);
  };
}

As per https://github.com/MatrixAI/js-events/pull/2, it should be easy to find and replace all uses to just AbstractEvent too. And due to the automation inside, most of the time you can just do:
class EventX extends AbstractEvent<SomeType> {}

And that's enough to create the relevant event.

Some notes on redispatch:
Injection
Don't redispatch relevant and irrelevant events. Only handle relevant events.
Encapsulation
Redispatch all events including relevant and irrelevant events
On Redispatch

redispatch same event - only do this for "irrelevant" events

an info event on quic socket, is not an info event of quic server

redispatch chain event - only do this for "relevant" events

an error in quic socket, is an error of the quic server

Do 2. whenever there's an "equivalent" event on the domain model.
Do 1. when there is not.
When doing 2, the chain event structure should be always have a base type.
Consider this:
class EventQUICServerError extends EventQUICServer<Error | EventQUICSocketError> {}

Notice that I'm wrapping Error | ErrorQUICSocketError under EventQUICServerError.

QUICServer might be encapsulating QUICSocket.
If QUICServer encapsulates QUICSocket, any error event of QUICSocket is a relevant event to QUICServer.
This is a potential infinite loop, so we use once to terminate the loop.
QUICServer redispatches QUICSocket using QUICServerError wrapping QUICSocketError because it's a relevant event and because a quic socket error is a quic server error.

This is not true for QUICConnectionError. While it is relevant to QUICServer for connection management, a quic connection error is not a quic server error. The quic server does not crash if there's a quic connection error. Thus redispatching upwards is purely informational. In this case, there is no need to wrap the event, and thus we redispatch the clone of it.
@tegefaulkes

Let's see how the above structure plays out in code before reviewing.

So one thing I've had to do is to introduce EventAll and EventDefault.
Furthermore my attempt at specialising the name property of AbstractError, EventAll and EventDefault fails to work because tsc won't accept the code even though it is now legal with native classes.
class Abc {
  // @ts-ignore
  public static name = `${this.name}`;
}

The above does not compile unfortunately. Even though it would be nice since we can continue using the same pattern like addEventListener(EventAll.name, f).
I used one before called EventAll.type, but then it would be different from the regular name property that would be used if people defined their own events.
addEventListener(EventCustom.name, f);
addEventListener(EventDefault.type, f);

Ok the change of nameclashes is quite low. So I'm just going to use EventAll.name and EventDefault.name.

The js-events https://github.com/MatrixAI/js-events/pull/5 has made this comment possible:
https://github.com/MatrixAI/js-quic/pull/53#issuecomment-1700334819
2 examples of this is actually in the js-events tests.
Can follow that as a guideline on what to do @amydevs @tegefaulkes.

Some notes on redispatch:
Injection
Don't redispatch relevant and irrelevant events. Only handle relevant events.
Encapsulation
Redispatch all events including relevant and irrelevant events
On Redispatch

redispatch same event - only do this for "irrelevant" events

an info event on quic socket, is not an info event of quic server

redispatch chain event - only do this for "relevant" events

an error in quic socket, is an error of the quic server

Do 2. whenever there's an "equivalent" event on the domain model.
Do 1. when there is not.
When doing 2, the chain event structure should be always have a base type.
Consider this:
class EventQUICServerError extends EventQUICServer<Error | EventQUICSocketError> {}

Notice that I'm wrapping Error | ErrorQUICSocketError under EventQUICServerError.

QUICServer might be encapsulating QUICSocket.
If QUICServer encapsulates QUICSocket, any error event of QUICSocket is a relevant event to QUICServer.
This is a potential infinite loop, so we use once to terminate the loop.
QUICServer redispatches QUICSocket using QUICServerError wrapping QUICSocketError because it's a relevant event and because a quic socket error is a quic server error.

This is not true for QUICConnectionError. While it is relevant to QUICServer for connection management, a quic connection error is not a quic server error. The quic server does not crash if there's a quic connection error. Thus redispatching upwards is purely informational. In this case, there is no need to wrap the event, and thus we redispatch the clone of it.
@tegefaulkes

So this has changed a bit. On injection, we may redispatch the relevant event, if the handling of the relevant event, is itself an event on the object receiving the event.

Rules for using js-events revised.

Dispatch event only after all state transition relevant to the event is done. This means events represent post-transition. This means subscribers always react to a post-transition state.
When an error event occurs in any given object X and is received by an outer object Y. Then it is Y responsibility to manage the lifecycle of X. But only if X is encapsulated within Y. Afterwards, Y will redispatch the X error event (whether encapsulated or not).

When handling events, if it is possible to hit an exception on the handler (both synchronous and asynchronous), it must be caught and redispatched as an error. Here's an example (notice the usage of AggregateError here, but when using js-errors, set the cause to be the AggregateError). Example:   protected handleEventXError = async (evt: EventXError) => {
     try {
       // Transition Y to errored state...
       if (this.isXEncapsulated) {
         // Only if it is encapsulated.
         await this.x.stop();
       }
       this.dispatchEvent(
         new EventYError({
           detail: evt,
         }),
       );
     } catch (err) {
       // Unexpected error is also dispatched
       this.dispatchEvent(
         new EventYError({
           // Aggregate error because it's actually 2 errors
           // If you use `js-errors`, then try making the cause an array
           // The array order should be
           detail: new AggregateError([err, evt.detail]),
         }),
       );
     }
   };

Say X emits error, then Y handles and tries to stop X then redispatches, then Z handles and tries to stop Y... etc all the way up to the root of the program P. It is important that at P, event handlers are added, otherwise errors will disappear to the ether.
Say X emits error, then Y handles and tries to stop X, but it fails at this point with a thrown exception. Then Y dispatches a new aggregate error. Z will handle this and try to stop Y. It may fail to stop Y too, in which case the same process repeats upwards.
Objects never listen on their own events.
Upon reacting to an event, mutations should occur before any event emission.
Event dispatch is always synchronous.
Event handlers which may cause errors should wrap the entire thing in a try catch that dispatches their domain error as it would be considered unexpected. That would be equivalent to calling await this.stop() so one can catch the exception.
The default handler should be used to redispatch the same instance of all encapsulated events.

In pull, handlers don't know who called them. In push, handlers always know who called them.
The usage of the event behaviour is tricky and not type-safe. Migrating to observables later in the future would enable these patterns in a more type-safe manner and allow us to automate this with higher order functions. But for libraries we have to stick with EventTarget.

@tegefaulkes brought up a situation like this:
If an object has a pull-method that could result in an error, and the object itself is evented, does that mean:

Throw up the exception, but don't dispatch.
Dispatch an event error, but don't throw up the exception.
Dispatch an event error AND throw up the exception.

I think all 3 are possibly valid depending on the situation.
For 1., this makes sense if the pull-method doesn't actually do any state changes on the object. Thus the exception is only relevant to the caller.
For 2., this makes sense if you can say that the method is entirely internal to the library or module context. Like for example QUICConnection.recv.
For 3., this makes sense if the method performs a state change, and it's not an "internal" method. It is expected that things outside your library may call it. In this case, you have to document that it will dispatch and throw an exception. In which case, the user of the object has to decide what to do, whether it ignores one and handles the other.

So for QUICConnection.stop it would need to do both dispatch and throw it upwards.
Note that in QUICConnection.recv which needs to do void this.stop().catch(() => {}) is because stop may throw up an error, and it needs to ignore it, because the expectation is that the error would have been already dispatched by QUICConnection.stop.

As it's currently implemented, the this.stop() is triggered in 4 conditions.

In recv() in response to a draining or closed state.
In send() in response to an error sending data through the socket.
In send() if the state transitioned to closed after sending data.
In the timer system in response to the connection timing out.

In cases 1, 3 and 4 it makes sense since we're responding to the the underlying quiche connection's state change.
2 is technically an internal error separate from quiche but it's a transport failure. This is the only one I can see emitting an error event that would trigger stop externally. I really don't like this since it's clean up logic critical to the function of QUICConnection that would need to handled external to the QUICConnection code and Maintained in 2 separate places in QUICServer and QUICClient. It makes things less maintainable.
Also, most errors coming out of quiche are obtained with connection.localError() or connection.peerError() after the connection has drained and closed.

@tegefaulkes you will need to update the pkgs.nix, shell.nix to get node 20. Also there's a rule that you'll need to make use of arguments in .eslintrc. https://github.com/MatrixAI/js-events/blob/91a71b7c7ac04b8a29978533971dd0824da71af0/.eslintrc#L42

As it's currently implemented, the this.stop() is triggered in 4 conditions.

In recv() in response to a draining or closed state.
In send() in response to an error sending data through the socket.
In send() if the state transitioned to closed after sending data.
In the timer system in response to the connection timing out.

In cases 1, 3 and 4 it makes sense since we're responding to the the underlying quiche connection's state change.
2 is technically an internal error separate from quiche but it's a transport failure. This is the only one I can see emitting an error event that would trigger stop externally. I really don't like this since it's clean up logic critical to the function of QUICConnection that would need to handled external to the QUICConnection code and Maintained in 2 separate places in QUICServer and QUICClient. It makes things less maintainable.
Also, most errors coming out of quiche are obtained with connection.localError() or connection.peerError() after the connection has drained and closed.

In your 2, you would follow 2.

Also, most errors coming out of quiche are obtained with connection.localError() or connection.peerError() after the connection has drained and closed.

In this case, these errors are not errors of the stop itself. They should just be dispatched.

So based on discussion,
The QUICSocket.send will not throw, but if it fails it will dispatch an error event. This event should ultimately cause a QUICConnection.stop() call. If it is managed by the QUICClient or QUICServer then handling is set up there based on encapsulation. It the QUICSocket is shared then the handling needs to be setup by whatever manages it.
Switching to this new pattern also means the QUICConnection shouldn't trigger a stop() call internally unless it drained and closed without error. If the quicheConnection enters the draining or closed state then we need to check for an error. Dispatch said error, then whatever is managing the connection, QUICServer or QUIClient, MUST call stop() on that connection while passing the relevant information back into it.
The QUICServer needs to trigger QUICConnection.stop() if it handles an error. If the QUICSocket is encapsulated then any socket errors needs to be handled and trigger stopping of the socket. Then re-dispatched so  whatever manages the QUICServer can clean up the QUICServer. It the QUICSocket is not encapsulated then what ever manages both the QUICSocket and QUICServer needs to handle their errors and clean them up.
Likewise for the QUICClient.

When separating the functionality between encapsulated dependencies and injected dependencies, we have to basically have a boolean that branches out the functionality.
However this results in alot of boilerplate. Requiring a separate xIsInjected boolean for every dependency that may be encapsulated or injected.
Only a few dependencies make use of this distinction during runtime. Most of the time, this is actually only used for testing & mocking.
Consider for example in the 3 situations of EFS, QUIC. In EFS, the db can be encapsulated or injected. In QUIC, the QUICSocket can be encapsulated or injected. Both cases are legitimate usecases. We should definitely have a boolean that indicates the distinction.
Consider the case of PolykeyAgent. Almost all of its dependencies should just be fully encapsulated. The only reason to enable the ability to pass in DB,... etc is allow mocking or unit testing. But the amount of boilerplate to support all of the dependencies would be HUGE!
The only way to make such a thing manageable would be to introduce parameter decorators on the constructor that automatically produces properties.
However I don't want to do that right now, it might introduce too much complexity. Instead let's do something different.

Where we have legitimate reasons for encapsulation/injection, we continue doing it with a manually specificed xIsInjected or equivalent boolean.
Where it's only there for testing, we can remove it..., and instead rely on module mocking in jest to make use of it. See: https://jestjs.io/docs/mock-functions#mocking-modules

This doesn't have to be a wholesale change immediately... we can just keep an issue in PK and slowly apply this across the ecosystem. In some cases, optional injection can still be kept since there's barely any lifecycle related activities, or the relevant objects in question are not evented objects.

If dispatching EventQUICServerError, means that QUICServer is in an errored state, does that mean we need a proper state transition like this.error().
Perhaps instead we would say this.stop({ error }) as indicative of transitioning to a stopped-error state.

So if error-ring is a terminal state, it might be easier to simply react on the stopped state, and check if that was due to an error not.
That would keep it quite simple if we could do this.

3.0.1 of js-events brings in the ability to use evt.target. That target will always point to the evented object. So if I get a EventQUICConnectionStop event I can check evt.target to get the QUICConnection that hit that event.

Ok so I got rid of the EventDefault for now since it was overcomplicating it. Informational observation can be left for later.
I'm attempting to follow the rules where pull methods always throw up exceptions and thus event handlers is where exceptions must be caught and converted to an error.

Regarding the usage of the monitor inside QUICConnection.start. This was moved here from #48.
The reason was that nothing would have a reference to the QUICConnection. But that isn't entirely true.
We actually have the QUICConnection as part of the connection map. So it is possible that, there may be something in between the recv and send call.
But @tegefaulkes you didn't mention that you were keeping the monitor usage.

Also this code doesn't quite make sense to me:
    await this.withMonitor(mon, this.lockbox, RWLockWriter, async (mon) => {
      if (!mon.isLocked(this.lockCode)) {
        return mon.withF(this.lockCode, async (mon) => {
          return this.send(mon);
        });
      }
    });

Why did you do if (!mon.isLocked(this.lockCode)) { ... }?

I just realised you're not actually locking the monitor when you pass it in. You are simply sharing the monitor.

This seems to make more sense:
      await Promise.race([
        Promise.all([
          withF(
            [contextsUtils.monitor(this.lockBox, RWLockWriter)],
            async ([mon]) => {
              await mon.lock(this.lockCode)();
              if (data != null) {
                await this.recv(
                  data,
                  { host: this._remoteHost, port: this._remotePort },
                  mon,
                );
              }
              await this.send(mon);
            }
          ),
          this.establishedP,
          this.secureEstablishedP,
        ]),
        abortP
      ]);

Notice by using the resource acquisition pattern, you don't need to use mon.withF again. It's unnecessary. It's already provided by making use of the resource acquisition of the monitor.
In this case we are ensuring that within the call graph of both of those recv and send calls, that any other calls outside must wait until it completes.

Some changes I'm doing:

All pull-methods always throw exceptions under errors.
Event handlers decide whether to dispatch events upon errors, depending on the type of the error, otherwise let's them bubble up as software errors that will shutdown the program by going to the unhandled promise rejection/unhandled exception.
The QUICConnection.createQUICConnection is removed entirely. This QUICConnection is a StartStop pattern. And thus should not have an async creator at all. Instead a constructor and start and stop can be used.
The QUICServer can then construct the connection first, then set it to the connection map of the socket, since the socket is "managed" by the server optionally. And then run the start.
This also means QUICConnection.start is what is being timed. So the minIdleTimeout is moved there.
If there's an error... we may need to stop the underlying connection. This means it's not possible to just call QUICConnection.stop since that can only be called if QUICConnection.start completed successfully, instead I'm stopping in the underlying connection properly there by forcing it. And since it wasn't setup properly, I believe I need to force this closure without waiting...
Where did stream force destroy go? Doesn't make sense why it got removed.

When we force close a connection due to not completing a start, is it an application error or not?
The code in staging is set to 0 being an application error. Or it uses the reasonToCode. I don't think this makes sense.

I think I'll remove the QUICConnection.withMonitor, it's entirely superfluous.
I also don't quite understand why it even bothers to say that if the locking code is not locked, it will then create a monitor locking it.
The whole point of the monitor is to be re-entrant. Meaning given the same monitor, locking the same key is a noop. So there shouldn't be any need to have a conditional checks.

Ok there are some things I'm going to require from js-errors and js-async-init.

js-errors should supply a checkError function take takes (e, (e) => bool). This can recursively check if an error exists within an "error graph" through the cause property as well as the aggregate error array.
js-async-init - because events need to be post-transition, this means XStopped or XStarted events should be part of the decorator class to ensure this to be the case.

It doesn't make much sense to me that when QUICConnection.stop is called we have to do this.setConnTimeoutTimer.

I've brought in js-errors and js-events to bring in the new checkError and also the ability for @StartStop and related decorators to automatically emit events.

I'm looking over the custom TLS verification. And referring to https://github.com/MatrixAI/js-quic/pull/26.
The implementation doesn't make much sense to me. There's a lack of comments and clear indication as what all these steps are supposed to do. I think it needs to be revisited.
      // Checking if the packet was a short frame.
      // Short indicates that the peer has completed TLS verification
      if (!this.shortReceived) {
        const header = quiche.Header.fromSlice(data, quiche.MAX_CONN_ID_LEN);
        // If short frame
        if (header.ty === 5) {
          this.shortReceived = true;
        }
      }
      // Checks if `secureEstablishedP` should be resolved. The condition for
      //  this is if a short frame has been received and 1 extra frame has been
      //  received. This allows for the remote to close the connection.
      if (!this.secured && this.shortReceived && !this.conn.isDraining()) {
        if (this.count >= 1) {
          this.secured = true;
          this.resolveSecureEstablishedP();
        }
        this.count += 1;
      }

There's no explanation about what the purpose of count is for.
I don't understand this kind of code. @tegefaulkes can you explain all of this?

Why is custom verification done on send, and not on recv? Seems like a strange place to do so.
Is it because connections are not fully established until send has occurred?

I'm going to need explanations on:

How the custom TLS works and why it involves so many different checks, like customVerified, shortReceived, and count. Just seems like alot of this can be simplified to just one thing.
How the establishedP works and secureEstablishedP works.
The connection time out, how is this maintained.
The relationship of start and stop to recv and send.
Why QUICStream doesn't have a force destruction anymore.

One of the key things I'm changing is to ensure that that state changes are now being handled by the event handlers, rather than being done by the start and stop calls.

As it's currently implemented, the this.stop() is triggered in 4 conditions.

In recv() in response to a draining or closed state.
In send() in response to an error sending data through the socket.
In send() if the state transitioned to closed after sending data.
In the timer system in response to the connection timing out.

In cases 1, 3 and 4 it makes sense since we're responding to the the underlying quiche connection's state change.
2 is technically an internal error separate from quiche but it's a transport failure. This is the only one I can see emitting an error event that would trigger stop externally. I really don't like this since it's clean up logic critical to the function of QUICConnection that would need to handled external to the QUICConnection code and Maintained in 2 separate places in QUICServer and QUICClient. It makes things less maintainable.
Also, most errors coming out of quiche are obtained with connection.localError() or connection.peerError() after the connection has drained and closed.

Case 3 is actually a socket error, and should be handled in QUICSocket event handler.
In the other 3 situations, it can make sense for QUICConnection.stop to be called automatically in response to changes. However the error handling needs to be improved, I don't like how its exceptions and events is constructed and emitted.
Since StartStop now automatically emits EventQUICConnectionStopped... then a question is whether the closure of the connection due to local error/remote error or timeout should be separate events. Since it's not possible to add details to the EventQUICConnectionStopped anyway.
It sort of depends on what exactly will be capturing these events... suppose one is using a quic server. A connection is received.... and runs for a while, and then an error occurs on the connection and the connection is now stopped. How does one know about this error...? One could listen for connection error events I guess...

Addressing questions PT1.

Checking the conditions for when we consider the connection secured by TLS is very finicky to work out. I feel the comments do describe what is happening if not in a lot of detail. The count is to track the number of frames after the short frame has be received. As the comments mention the condition for secured is short +1 frame. The extra frame is to allow the remote to reject the connection via custom TLS logic. The count could be replaced with a simple state machine.

Checking if the connection is draining is to prevent the secured established condition if the connection was shutting down.
But yes, as discussed this whole thing can be simplified as a state machine.

established is resolved once the quiche connection has established. This is the first step of the connection establishment. Established just allows data to be sent but TLS has not completed yet. IF an error happens before established then we reject the promise. This just the main way we return the error when awaiting for establishment.

secure establishment is when the TLS has completed locally and remotely and we don't expect TLS to fail beyond this point. This is then we consider the connection fully established for our purposes. the condition for this is described above. If some problem happens before secure establishment completes the we reject it with the appropriate error. This is just how we return the error when awaiting for secure establishment.

We check the timeout and create a timer with the setConnTimeOutTimer method. This sets up the handler and the timer. If the timer exists then we reset the timer with a new delay. The handler it creates triggers stop if the connection had timed out. If not it triggers a send and creates a new timer with the new delay. The main way of this timer cleaning up is if connection.timeout() returns null. I decided that was the source of truth for that decision.

question 4.
I'm not sure if I understand the question. Start and stop create and destroy the state respectively, recv and send just process incoming and outgoing data.
question 5.
From the perspective of the QUICStream, destruction is only ever forced. But after reviewing this it does seem weird. when destroying the streams we're awaiting the destroy method, but we don't actually await any state change inside the destroy method. looks like the connection is awaiting the streams to end with stream.destroyedP. I could move this logic into the stream destroy method.
So yes, this should function the same way as the other layers. QUICStream.destroy() should have a force option and await the destruction of the stream within it. A simple change assuming we don't run into any deadlocks with question 6.
question 6.
We can do this, but we'll have to work around any deadlocking if it comes up. I know QUICConnection needs to process incoming packets during a draining state. I think no packets are sent while draining. So if we enter a draining state, we can't hold a lock on the monitor while awaiting stop.
question 7.
Is this a trick question? It's how we send data on a stream. I need more context here.

I'm going to see what happens if I break 2 rules.

That objects can listen on their own events, such as listening for their own error event and proceed to stop.
That we can just void this.stop({ force: true } ); especially if any errors that occur during stopping become software bugs.

It seems to make sense that if QUICConnection was stopped locally with/without an error, that error is sent to the other side and thus is not an error of the QUICConnection itself.
If the remote side closed it gracefully, then again it's not an error.
If the remote side closed it with an error, then we can say that the QUICConnection suffered an error.
If the QUICConnection send failed on sending to the QUICSocket, then that's also an error on the QUICConnection.

I've updated the QUICStream to avoid using void promise wherever possible. Right now the cancel() still does this but it's required to be synchronious.
There may be deadlocks due to it waiting for calls to the connection.send but I can only tell by testing and we're not back to that stage yet.
I'll review the comments and see if I can expand them based on what you did in QUICSocket.

The QUICConnection should behave in this way:
    // If the connection timed out, it's an error
    // If the connection operations failed (runtime IO errors), it's an error
    // If the remote side closed the connection with an error, it's an error
    // if the remote side closed the connection without error, it's not an error
    // If the local side closed the connection with an error, it's not an error
    // If the local side closed the connection witout error, it's not an error

Because of this connection.stop({ force: true }); doesn't really do anything except force destroy the streams (by not waiting for a graceful close of streams).
And therefore as quiche connection is entering a draining state, we just wait until the idle timeout is exhausted, and then connection.stop will return.
Again this simplifies connection.stop({ force: true }); and we can say that any exceptions thrown here would be considered a software bug, not a runtime error.

Within connection.stop, all of its error checks should be moved or removed. Remote error should be checked in recv. Local errors are not errors for QUICConnection so there's no need to do anything for that. Timeout errors can occur inside the idle timeout timer.

So a few things.

We actually have to attach event listeners prior to start, and remove listeners after stop. This is because if we did the other way around we'd miss Start, Started, Stop and Stopped events.
QUICServer actually does have to define the connection event handlers as properties because that's the only way we can remove the handlers when doing a regular QUICServer.stop.
There's actually a race condition when starting a quic connection. I believe this already existed in the existing code, it was just not obvious until now.

The above screenshot shows both QUICSocket.handleSocketMessage and QUICServer.newConnection.
You can see that when we do:
await connection.start({ data }, { timer: this.minIdleTimeout })`

The data option parameter is the same data used in:
      // No matter the connection state, we will send in the `recv` and `send` calls
      await connection.withMonitor(async (mon) => {
        await mon.lock(connection.lockingKey)();
        await connection.recv(data, remoteInfo_, mon);
        await connection.send(mon);
      });

So basically this means starting a connection involves reading that initial data.
However after starting the connection, we end up re-using that same initial data for the recv and send pairs.
Inside the QUICConnection.start we now also perform a recv and send pair.
Obviously we can see an issue here, we shouldn't be re-processing the same initial data twice.

For problem 3. there's a few ways to solve this.

Move the recv and send pair in QUICSocket.handleSocketMessage to be used only when the connection already exists. That way, when accepting a new connection, we can just rely on the QUICConnection.start to perform the recv and send pair.
Don't do the recv and send pair inside QUICConnection.start... and do the recv and send pair inside the QUICSocket.handleSocketMessage. If this is the case, what is the point of QUICConnection.start... it's sort of useless.

So I think solution 1 is the correct one. It ensures  that starting a connection is self-contained, and it sort of makes sense, as awaiting start means the connection is fully started and ready to go instead of expecting outside send and recv pairs. From QUICSocket.handleSocketMessage perspective, if it is dealing with a new connection, it will let the QUICServer handle all of it (and respectively the QUICConnection). But if it's an existing connection, it can directly interact with it by doing the recv and send pair.

So the way I actually solved it is to move the recv and send pair from QUICConnection.start to the bottom of QUICServer.newConnection.
That kept the starting and stopping of connections consistently managed by QUICServer, while QUICSocket only performed the recv and send pairs when the connection already existed.
Comments added to clarify the concurrent procedures for starting a QUICConnection as this is quite a tricky procedure.

Now that QUICConnection.start simplified to just:
      await Promise.race([
        Promise.all([
          this.establishedP,
          this.secureEstablishedP,
        ]),
        abortP,
      ]);

Why do we even bother waiting for both establishedP and secureEstablishedP?
Why not just use establishedP to represent both the establishment and secure establishment?
We need to review the TLS mutual verification logic @tegefaulkes since I still think the counter is weird.

Some extra questions about QUICConnection.

Why is max idle timer not started inside QUICConnection.start? Surely it should start ticking as soon as we call it.

I believe QUICServer is now fully refactored too.
What's left is:

QUICConnection
QUICClient
QUICStream - if everything is understood here?

@tegefaulkes review the QUICServer too, and also there's some comments inside QUICConnection that require addressing.

We will need to start testing tomorrow.

The TLS checking logic should be moved into the recv and work against these rules:
  // established means - I am established (it doesn't mean both sides are established)
  // As soon as I am established, I can check for the peer certificates
  // If the peer certificates is NULL
  // That means there was no certificates supplied
  // It should not be possible for an empty array (if it is, use `never()`)

  // verifyPeer: true, verifyCallback: true
  //   - if peer doesn't supply certs, it's a TlsFail (local error: 327)
  //   - if peer supplies bad certificates - no TLS fail, but verifyCallback will run
  //   - if peer supplies good certificates - no TLS fail, but verifyCallback will run

  // verifyPeer: true, verifyCallback: false
  //   - if peer doesn't supply certs, it's a TlsFail (local error: 327)
  //   - if peer supplies bad certificates - it's a TlsFail (local error: 304)
  //   - if peer supplies good certificates - no TLS fail

  // verifyPeer: false, verifyCallback: true
  //   - if peer doesn't supply certs, no TLS fail
  //   - if peer supplies bad certificates - no TLS fail, ignore verifyCallback
  //   - if peer supplies good certificates - no TLS fail, ignore verifyCallback

  // verifyPeer: false, verifyCallback: false
  //   - if peer doesn't supply certs - no TLS fail
  //   - if peer supplies bad certficates - no TLS fail
  //   - if peer supplies good certificates - no TLS fail

  // verifyAllowFail will become a derived option of verifyCallback
  // calculate the option change in `config.ts`

  // That means during the recv() call, you want to do this:
  //   - check if you get a TlsFail exception
  //     - check the local error
  //     - connection is automatically closed
  //     - dispatch error event
  //     - throw error
  //   - check if we are established
  //     - if we are verifyPeer && verifyCallback true
  //       - check if peer certificates is null - if so, pass
  //       - otherwise, call verifyCallback
  //       - if fail, 304, dispatch error event, throw error
  //    - else
  //       - pass

Note that 372 means no certs. 304 means bad certs.

This also means we can just wait for secureEstablishedP, and remove establishedP in QUICConnection.start.
Also both peer error and local error are error events for QUICConnection. But if someone were to call QUICConnection.stop() with an error, it shouldn't be an error event. To achieve this you might need to not check for local error or peer error under send.

QUICStream will never trigger QUICConnection.recv right? It will only trigger QUICConnection.send?

@tegefaulkes you mentioned that in one particular case, there may be an issue with calling close in the middle of the handshake.
I've found that this is not a problem from the server side, but there appears to be a bug in quiche where it can result in a problem on the client side.
Specifically in the RSA scenario with custom verification callback a bad server certificate.
In the case where we don't use a custom verification callback, the expected behaviour is:
    RSA fail verifying server with bad server certificate (TlsFail 304)
      ✓ client connect (2 ms)
      ✓ client dialing (1 ms)
      ✓ client and server negotiation (4 ms)
      ✓ server accept (2 ms)
      ✓ client <-initial- server (1 ms)
      ✓ client -initial-> server
      ✓ client <-handshake- server (36 ms)
      ✓ client has local error TlsFail 304
      ✓ client -handshake-> server (4 ms)
      ✓ server has peer error TlsFail 304
      ✓ client and server close (44 ms)

You can see that when the server sends the handshake to the client, the client experiences a local TlsFail 304 error.
However if we attach a custom verification callback, this overrides the underlying verification function so that it always passes.
In that scenario, we are supposed to there perform our custom TLS verification.
We can imagine a call like await verifyCallback(...) to throw, and then translate this to a:
clientConn.close(false, 304, Buffer.from(''))

While this works for the client, and the client's connection after sending a short frame to the server enters draining state and eventually closes, the server receives the short frame, but it doesn't enter a draining state.
In fact, the server connection just becomes dead, but without any draining. The client connection effectively closes, and leaves the server connection to idle until it times out.
It's interesting that this does not happen when we don't use a custom verification callback.
This tells me that quiche is expecting to send a handshake frame, and in this particular case you cannot close the connection.... This is definitely an upstream bug for quiche, but it's an edge case.
To solve this will be tricky. Because it only occurs when the client receives a handshake frame that succeeds due to the custom verification override.
We would have to special case this situation, and then send a regular handshake frame back to the server, and only afterwards perform the clientConn.close(false, 304, Buffer.from(''));.

In the above pseudocode for TLS: https://github.com/MatrixAI/js-quic/pull/53#issuecomment-1707642882
We are just checking the custom callback as soon as we have done recv and where the connection is established, which it is in this case.
In normal cases this would work (normal meaning the server can do this with a custom verification callback).
But due to the above problem with RSA handshake, we cannot immediately close. We have to call send first, before calling close.
One way to solve this is by doing the same thing symmetrically on both sides.
That means in these cases, whenever we have a custom verification callback, after the recv call, we can do check the custom verification callback, but rather than immediately calling close there, we instead change it so that we always end up calling send before sending close.
This means the order of events should be in case of custom tls callback:
recv
  - custom verify callback failure
send
close IF custom verify callback failed
  - send

However this only needs to be done in the context of custom verify callback. If the custom verify callback isn't involved, we could instead do:
recv
  - close on TLS fail
send

In a way the above hack is probably required simply because we are doing custom verification at the JS level, and not at the Rust level. The original plan was for the custom verification function to just be called from Rust to JS. But you hit some problem achieving that. Maybe some problem with the napi bridge, and runtime incompatibility with Rust's boringssl and how JS functions work (which could be asynchronous).
By not doing this, we are circumventing the normal way of doing TLS verification in quiche. And so... now we introduce this other hack here.

Based on discussion. The general flow is recv -> process streams -> send -> process state [-> further sends]
This mainly means the following changes.

Remove any state checking from recv
processing streams is done inside of send as the first step.
A failure in sending due to the socket is emitted as an error event that triggers QUICConnection.stop()
processing state for a connection is all moved to processState() which is called at the end of send. This includes the custom TLS verification logic.
If a state change triggers stopping then this is emitted as an event which then calls QUICConnection.stop().

I think custom TLS verification should occur before step 2.
That should accurately match what happens when it's a normal TLS failure.

I've now done the proper split for both RSA and ECDSA tests, it shows that the intermediate close in the middle of the handshake is indeed affecting both RSA and ECDSA, and probably will affect Ed25519 the same.

What's the streamStats function for?

In quiche? it returns bunch of statistics for that connection. Might be useful to have on QUICConnection.

All native TLS tests passing.

[nix-shell:~/Projects/js-quic]$ npm test -- ./tests/native/tls

><EMAIL_ADDRESS>test
> jest ./tests/native/tls

Determining test suites to run...
GLOBAL SETUP
 PASS  tests/native/tls/rsa.test.ts
  native/tls/rsa
    RSA success with both client and server certificates
      ✓ client connect (4 ms)
      ✓ client dialing (1 ms)
      ✓ client and server negotiation (4 ms)
      ✓ server accept (2 ms)
      ✓ client <-initial- server (1 ms)
      ✓ client -initial-> server
      ✓ client <-handshake- server (2 ms)
      ✓ client is established (1 ms)
      ✓ client -handshake-> server (1 ms)
      ✓ server is established
      ✓ client <-short- server (1 ms)
      ✓ client -short-> server
      ✓ client and server established (2 ms)
      ✓ client close (61 ms)
    RSA success with only server certificates
      ✓ client connect
      ✓ client dialing
      ✓ client and server negotiation (2 ms)
      ✓ server accept (2 ms)
      ✓ client <-initial- server (1 ms)
      ✓ client -initial-> server
      ✓ client <-handshake- server (1 ms)
      ✓ client is established (1 ms)
      ✓ client -handshake-> server
      ✓ server is established (1 ms)
      ✓ client <-short- server (1 ms)
      ✓ client -short-> server
      ✓ client and server established (2 ms)
      ✓ client close (44 ms)
    RSA fail verifying client with bad client certificate (TlsFail 304)
      ✓ client connect
      ✓ client dialing
      ✓ client and server negotiation (2 ms)
      ✓ server accept (2 ms)
      ✓ client <-initial- server (1 ms)
      ✓ client -initial-> server
      ✓ client <-handshake- server (1 ms)
      ✓ client is established (1 ms)
      ✓ client -handshake-> server (27 ms)
      ✓ server has local error TlsFail 304
      ✓ client <-handshake- server (2 ms)
      ✓ client has peer error TlsFail 304 (1 ms)
      ✓ client and server close (29 ms)
    RSA fail verifying client with no client certificate (TlsFail 372)
      ✓ client connect (1 ms)
      ✓ client dialing
      ✓ client and server negotiation (1 ms)
      ✓ server accept (1 ms)
      ✓ client <-initial- server
      ✓ client -initial-> server
      ✓ client <-handshake- server (1 ms)
      ✓ client is established (1 ms)
      ✓ client -handshake-> server (1 ms)
      ✓ server has local error TlsFail 372 (1 ms)
      ✓ client <-handshake- server (3 ms)
      ✓ client has peer error TlsFail 372 (1 ms)
      ✓ client and server close (26 ms)
    RSA fail verifying server with bad server certificate (TlsFail 304)
      ✓ client connect
      ✓ client dialing
      ✓ client and server negotiation (2 ms)
      ✓ server accept (2 ms)
      ✓ client <-initial- server
      ✓ client -initial-> server
      ✓ client <-handshake- server (2 ms)
      ✓ client has local error TlsFail 304
      ✓ client -handshake-> server (2 ms)
      ✓ server has peer error TlsFail 304
      ✓ client and server close (24 ms)
    RSA fail with no server certificates (InternalError 1)
      ✓ client connect
      ✓ client dialing
      ✓ client and server negotiation (1 ms)
      ✓ server accept (2 ms)
      ✓ server has local error 1 (1 ms)
      ✓ client <-initial- server (4 ms)
      ✓ client has peer error 1
      ✓ client and server close (2998 ms)
    RSA with custom verify callback
      RSA success with both client and server certificates
        ✓ client connect
        ✓ client dialing (1 ms)
        ✓ client and server negotiation (1 ms)
        ✓ server accept (2 ms)
        ✓ client <-initial- server
        ✓ client -initial-> server (1 ms)
        ✓ client <-handshake- server (1 ms)
        ✓ client is established (1 ms)
        ✓ client -handshake-> server (1 ms)
        ✓ server is established (1 ms)
        ✓ client <-short- server
        ✓ client -short-> server
        ✓ client and server established (2 ms)
        ✓ client close (36 ms)
      RSA success with only server certificates
        ✓ client connect
        ✓ client dialing
        ✓ client and server negotiation (2 ms)
        ✓ server accept (1 ms)
        ✓ client <-initial- server (1 ms)
        ✓ client -initial-> server
        ✓ client <-handshake- server
        ✓ client is established (1 ms)
        ✓ client -handshake-> server (1 ms)
        ✓ server is established
        ✓ client <-short- server
        ✓ client -short-> server (1 ms)
        ✓ client and server established (1 ms)
        ✓ client close (31 ms)
      RSA fail verifying client with bad client certificate (TlsFail 304)
        ✓ client connect (1 ms)
        ✓ client dialing
        ✓ client and server negotiation (1 ms)
        ✓ server accept (2 ms)
        ✓ client <-initial- server
        ✓ client -initial-> server (1 ms)
        ✓ client <-handshake- server (1 ms)
        ✓ client is established (1 ms)
        ✓ client -handshake-> server (32 ms)
        ✓ server has local error TlsFail 304 (1 ms)
        ✓ client <-short- server (2 ms)
        ✓ client has peer error TlsFail 304
        ✓ client and server close (25 ms)
      RSA fail verifying client with no client certificate (TlsFail 372)
        ✓ client connect (1 ms)
        ✓ client dialing
        ✓ client and server negotiation (1 ms)
        ✓ server accept (2 ms)
        ✓ client <-initial- server (1 ms)
        ✓ client -initial-> server
        ✓ client <-handshake- server (1 ms)
        ✓ client is established
        ✓ client -handshake-> server (3 ms)
        ✓ server has local error TlsFail 372
        ✓ client <-handshake- server (2 ms)
        ✓ client has peer error TlsFail 372
        ✓ client and server close (26 ms)
      RSA fail verifying server with bad server certificate (TlsFail 304)
        ✓ client connect (1 ms)
        ✓ client dialing
        ✓ client and server negotiation (1 ms)
        ✓ server accept (2 ms)
        ✓ client <-initial- server (1 ms)
        ✓ client -initial-> server
        ✓ client <-handshake- server (3 ms)
        ✓ client -handshake-> server (1 ms)
        ✓ client has local error TlsFail 304
        ✓ client -short-> server (2 ms)
        ✓ server has peer error TlsFail 304
        ✓ client and server close (25 ms)

 PASS  tests/native/tls/ed25519.test.ts (6.92 s)
  native/tls/ed25519
    Ed25519 success with both client and server certificates
      ✓ client connect (4 ms)
      ✓ client dialing (1 ms)
      ✓ client and server negotiation (4 ms)
      ✓ server accept (1 ms)
      ✓ client <-initial- server (1 ms)
      ✓ client is established (1 ms)
      ✓ client -initial-> server (1 ms)
      ✓ server is established
      ✓ client <-short- server
      ✓ client -short-> server (1 ms)
      ✓ client and server established (2 ms)
      ✓ client close (56 ms)
    Ed25519 success with only server certificates
      ✓ client connect
      ✓ client dialing
      ✓ client and server negotiation (2 ms)
      ✓ server accept
      ✓ client <-initial- server (1 ms)
      ✓ client is established (1 ms)
      ✓ client -initial-> server
      ✓ server is established
      ✓ client <-short- server (1 ms)
      ✓ client -short-> server
      ✓ client and server established (1 ms)
      ✓ client close (24 ms)
    Ed25519 fail verifying client with bad client certificate (TlsFail 304)
      ✓ client connect
      ✓ client dialing
      ✓ client and server negotiation (2 ms)
      ✓ server accept (1 ms)
      ✓ client <-initial- server (1 ms)
      ✓ client is established
      ✓ client -initial-> server (36 ms)
      ✓ server has local error TlsFail 304 (1 ms)
      ✓ client <-handshake- server (1 ms)
      ✓ client has peer error TlsFail 304
      ✓ client and server close (19 ms)
    Ed25519 fail verifying client with no client certificate (TlsFail 372)
      ✓ client connect (1 ms)
      ✓ client dialing
      ✓ client and server negotiation (1 ms)
      ✓ server accept (1 ms)
      ✓ client <-initial- server (1 ms)
      ✓ client is established
      ✓ client -initial-> server (2 ms)
      ✓ server has local error TlsFail 372 (1 ms)
      ✓ client <-handshake- server (2 ms)
      ✓ client has peer error TlsFail 372
      ✓ client and server close (14 ms)
    Ed25519 fail verifying server bad server certificate (TlsFail 304)
      ✓ client connect
      ✓ client dialing
      ✓ client and server negotiation (2 ms)
      ✓ server accept (1 ms)
      ✓ client <-initial- server (1 ms)
      ✓ client has local error TlsFail 304 (1 ms)
      ✓ client -initial-> server (2 ms)
      ✓ server has peer error TlsFail 304
      ✓ client and server close (2999 ms)
    Ed25519 fail with no server certificates (InternalError 1)
      ✓ client connect
      ✓ client dialing
      ✓ client and server negotiation (2 ms)
      ✓ server accept (2 ms)
      ✓ server has local error 1
      ✓ client <-initial- server (2 ms)
      ✓ client has peer error 1 (1 ms)
      ✓ client and server close (3000 ms)
    Ed25519 with custom verify callback
      Ed25519 success with both client and server certificates
        ✓ client connect
        ✓ client dialing (1 ms)
        ✓ client and server negotiation (1 ms)
        ✓ server accept (1 ms)
        ✓ client <-initial- server
        ✓ client is established (1 ms)
        ✓ client -initial-> server (1 ms)
        ✓ server is established (1 ms)
        ✓ client <-short- server
        ✓ client -short-> server (1 ms)
        ✓ client and server established (1 ms)
        ✓ client close (29 ms)
      Ed25519 success with only server certificates
        ✓ client connect
        ✓ client dialing
        ✓ client and server negotiation (1 ms)
        ✓ server accept (1 ms)
        ✓ client <-initial- server (1 ms)
        ✓ client is established
        ✓ client -initial-> server (1 ms)
        ✓ server is established
        ✓ client <-short- server
        ✓ client -short-> server
        ✓ client and server established (1 ms)
        ✓ client close (25 ms)
      Ed25519 fail verifying client with bad client certificate (TlsFail 304)
        ✓ client connect
        ✓ client dialing
        ✓ client and server negotiation (1 ms)
        ✓ server accept (1 ms)
        ✓ client <-initial- server (1 ms)
        ✓ client is established
        ✓ client -initial-> server (40 ms)
        ✓ server has local error TlsFail 304
        ✓ client <-short- server (2 ms)
        ✓ client has peer error TlsFail 304
        ✓ client and server close (13 ms)
      Ed25519 fail verifying client with no client certificate (TlsFail 372)
        ✓ client connect
        ✓ client dialing
        ✓ client and server negotiation (1 ms)
        ✓ server accept
        ✓ client <-initial- server (1 ms)
        ✓ client is established (1 ms)
        ✓ client -initial-> server (2 ms)
        ✓ server has local error TlsFail 372 (1 ms)
        ✓ client <-handshake- server (2 ms)
        ✓ client has peer error TlsFail 372
        ✓ client and server close (14 ms)
      Ed25519 fail verifying server bad server certificate (TlsFail 304)
        ✓ client connect (1 ms)
        ✓ client dialing
        ✓ client and server negotiation (1 ms)
        ✓ server accept (1 ms)
        ✓ client <-initial- server (2 ms)
        ✓ client -initial-> server (1 ms)
        ✓ client has local error TlsFail 304 (1 ms)
        ✓ client -short-> server (1 ms)
        ✓ server has peer error TlsFail 304 (1 ms)
        ✓ client and server close (15 ms)

 PASS  tests/native/tls/ecdsa.test.ts (6.919 s)
  native/tls/ecdsa
    ECDSA success with both client and server certificates
      ✓ client connect (3 ms)
      ✓ client dialing
      ✓ client and server negotiation (4 ms)
      ✓ server accept (1 ms)
      ✓ client <-initial- server (1 ms)
      ✓ client is established (1 ms)
      ✓ client -initial-> server (1 ms)
      ✓ server is established
      ✓ client <-short- server (1 ms)
      ✓ client -short-> server (1 ms)
      ✓ client and server established (2 ms)
      ✓ client close (51 ms)
    ECDSA success with only server certificates
      ✓ client connect
      ✓ client dialing (1 ms)
      ✓ client and server negotiation (1 ms)
      ✓ server accept (1 ms)
      ✓ client <-initial- server
      ✓ client is established (1 ms)
      ✓ client -initial-> server
      ✓ server is established (1 ms)
      ✓ client <-short- server
      ✓ client -short-> server
      ✓ client and server established
      ✓ client close (26 ms)
    ECDSA fail verifying client with bad client certificate (TlsFail 304)
      ✓ client connect (1 ms)
      ✓ client dialing
      ✓ client and server negotiation (1 ms)
      ✓ server accept (1 ms)
      ✓ client <-initial- server (1 ms)
      ✓ client is established
      ✓ client -initial-> server (28 ms)
      ✓ server has local error TlsFail 304
      ✓ client <-handshake- server (2 ms)
      ✓ client has peer error TlsFail 304 (1 ms)
      ✓ client and server close (14 ms)
    ECDSA fail verifying client with no client certificate (TlsFail 372)
      ✓ client connect (1 ms)
      ✓ client dialing
      ✓ client and server negotiation (2 ms)
      ✓ server accept (1 ms)
      ✓ client <-initial- server
      ✓ client is established (1 ms)
      ✓ client -initial-> server (1 ms)
      ✓ server has local error TlsFail 372
      ✓ client <-handshake- server (2 ms)
      ✓ client has peer error TlsFail 372 (1 ms)
      ✓ client and server close (16 ms)
    ECDSA fail verifying server bad server certificate (TlsFail 304)
      ✓ client connect
      ✓ client dialing (1 ms)
      ✓ client and server negotiation (1 ms)
      ✓ server accept (1 ms)
      ✓ client <-initial- server (2 ms)
      ✓ client has local error TlsFail 304 (1 ms)
      ✓ client -initial-> server (2 ms)
      ✓ server has peer error TlsFail 304
      ✓ client and server close (3000 ms)
    ECDSA fail with no server certificates (InternalError 1)
      ✓ client connect
      ✓ client dialing
      ✓ client and server negotiation (1 ms)
      ✓ server accept (2 ms)
      ✓ server has local error 1
      ✓ client <-initial- server (2 ms)
      ✓ client has peer error 1 (1 ms)
      ✓ client and server close (3000 ms)
    ECDSA with custom verify callback
      ECDSA success with both client and server certificates
        ✓ client connect (1 ms)
        ✓ client dialing
        ✓ client and server negotiation (1 ms)
        ✓ server accept (1 ms)
        ✓ client <-initial- server (1 ms)
        ✓ client is established (1 ms)
        ✓ client -initial-> server
        ✓ server is established (1 ms)
        ✓ client <-short- server (1 ms)
        ✓ client -short-> server
        ✓ client and server established (2 ms)
        ✓ client close (29 ms)
      ECDSA success with only server certificates
        ✓ client connect (1 ms)
        ✓ client dialing
        ✓ client and server negotiation (1 ms)
        ✓ server accept (1 ms)
        ✓ client <-initial- server
        ✓ client is established (1 ms)
        ✓ client -initial-> server
        ✓ server is established
        ✓ client <-short- server
        ✓ client -short-> server (1 ms)
        ✓ client and server established
        ✓ client close (26 ms)
      ECDSA fail verifying client with bad client certificate (TlsFail 304)
        ✓ client connect (1 ms)
        ✓ client dialing
        ✓ client and server negotiation (1 ms)
        ✓ server accept (1 ms)
        ✓ client <-initial- server (1 ms)
        ✓ client is established
        ✓ client -initial-> server (37 ms)
        ✓ server has local error TlsFail 304
        ✓ client <-short- server (2 ms)
        ✓ client has peer error TlsFail 304
        ✓ client and server close (16 ms)
      ECDSA fail verifying client with no client certificate (TlsFail 372)
        ✓ client connect
        ✓ client dialing
        ✓ client and server negotiation (1 ms)
        ✓ server accept (1 ms)
        ✓ client <-initial- server (1 ms)
        ✓ client is established
        ✓ client -initial-> server (3 ms)
        ✓ server has local error TlsFail 372
        ✓ client <-handshake- server (2 ms)
        ✓ client has peer error TlsFail 372
        ✓ client and server close (13 ms)
      ECDSA fail verifying server bad server certificate (TlsFail 304)
        ✓ client connect
        ✓ client dialing
        ✓ client and server negotiation (1 ms)
        ✓ server accept
        ✓ client <-initial- server (2 ms)
        ✓ client -initial-> server (2 ms)
        ✓ client has local error TlsFail 304
        ✓ client -short-> server (2 ms)
        ✓ server has peer error TlsFail 304
        ✓ client and server close (16 ms)

Test Suites: 3 passed, 3 total
Tests:       377 passed, 377 total
Snapshots:   0 total
Time:        7.155 s
Ran all test suites matching /.\/tests\/native\/tls/i.
GLOBAL TEARDOWN

OK, updated the general connection logic flow. based on what was discussed above. Everything is slightly cleaner and things clearly happen in stages now.
Stopping is triggered via once events.

a connection error event due to timeout, local or peer error (possibly TLS)
a connection stop event due to a graceful closure.

All of these stop triggering events are emitted in processState expect for one error due to a socket send problem.
... forgot to hit comment earlier

Did you move all "state processing" code into processState?
You should have a docblock then explains why all relevant code transition happens in this function.
And also document any exceptions to that. Mental model and all that.

QUICClient is mostly done now, needs some review and polish but we're in a testable state now.
I'm looking over the tests now.
One thing I've noticed is that a graceful close results in a peer error on the peer connection. So we may have to special case code 0 as no error.

One thing I've noticed is that a graceful close results in a peer error on the peer connection. So we may have to special case code 0 as no error.

Yes 0 actually means graceful close. And so that's really not an error at all.

Note that's true for application error and transport level too.
So a transport level 0 means the transport closed it gracefully.
Application level 0 means application closed it gracefully.
In both cases, not an error.
However it'd be good for the user to be able to read the ending code and message (if any), by exposing this via a function.
I could imagine a handler on stop/stopped that wants to get this information.

Same idea applies when stream is destroy/destroyed. I'd like to be able to ask about the stream's final code regardless of what it is.

I can imagine 2 public functions:
connection.getError(): { isApp: boolean; code: number; reason: Uint8Array } | undefined;
stream.getError(): number | undefined;

The main expectation in tests that has changed is, we don't wait for the peer to finish verifying TLS before considering secure establishment complete.
The main difference here is that a test that expected the connection to start due to the peer rejecting the connection will now see the connection succeed but soon after fail with the TLS error.
These tests should now expect the connection to succeed but soon emit a peer error of 304.
Other problems are likely errors with the code itself. I haven't had a change to tests streams yet so I have no idea on how well that is working right now.

Hey @tegefaulkes how come you are not calling this.send(mon); in QUICConnection.recv? I thought that was the plan.

The QUICConnection.start shouldn't be calling this.send().... it does not need to do this, since it's the QUICClient and QUICServer's responsibility to do this. The whole point is to just say await quicConnection.start();.
And async public recv should be calling this.send(mon). Why is this missing, this was like the most important part of our discussion!?

Still refactoring QUICConnection but getting closer.
Made some realisations:

When calling quiche conn.recv, it can set the peerError() but it will never throw an exception due to a peer error. It will however throw in response to a localError(). Thus one can check for local error in the exception handler, but not peer error.
The custom TLS verification adds a curve ball to the control flow of recv and send. Basically if it's not being used and we are infact verifying the peer, then we should be processing the streams as soon as the connection is established. And regular established means secure established. However if it is being used, then first call to QUICConnection.processStreams needs to occur instead after the conn.send in QUICConnection.send. After which, if all the conditions is correct, it will be securely established, and subsequent calls to processStreams will occur instead in the QUICConnection.send.
Proved that quiche implementation does not implement stateless reset or any sort of acknowledgement after receiving a CONNECTION_CLOSE frame.
For the QUICServer it has to call QUICConnection.recv to start the connection, while for the QUICClient it has to call QUICConnection.send to start the connection. We don't bother exposing the pair of recv then send, instead recv just calls send. It might be possible to abstract all of this into the QUICConnection.start method later, thus simplifying the logic even further.
Relevant events for QUICConnection include: EventQUICConnectionError and EventQUICConnectionEnd and EventQUICConnectionStopped. These 3 events roughly correspond to error, end and close in regular node's net socket.
Some interesting exceptions have been created to allow distinction between local errors and peer errors: ErrorQUICConnectionLocal and ErrorQUICConnectionPeer, these 2 exceptions will be made available into EventQUICConnectionError.detail, whereas EventQUICConnectionEnd will only have the peerError() as the detail.
Both EventQUICConnectionEnd and EventQUICConnectionError is handled by QUICConnection (thus listening to its own events) and proceeds to: a. reject the start, and b. call stop with force. Rejecting the start is important here in case something is waiting on the start. More specifically it rejecting the secureEstablishedP promise. However since end is a graceful end, the exception we reject with will be a bit different. Thus when doing await quicConnection.start(), you may end up with a ErrorQUICConnectionRecv, ErrorQUICConnectionSend, ErrorQUICConnectionLocal, ErrorQUICConnectionPeer... and we will need one just to represent gracefully ending like ErrorQUICConnectionEnd.

In effect, one can just wait for EventQUICConnectionError or EventQUICConnectionStopped. I don't think it's necessary to listen for EventQUICConnectionEnd (although it can be more convenient to find the peer error code).
As long as we expose connection.getError() which will give the peerError or localError whichever one is relevant, then that should work fine.

So now for recv and send the only exceptions that get thrown upwards as caller errors are internal errors.
This can occur:

Failed this.conn.send that isn't a Done
Failed this.conn.recv that isn't due to a local error
Failed QUICSocket.send

Also possibly processing streams, but I haven't looked at it yet.
This means it calling send and recv is a bit safer. So now QUICConnection.start will also trigger send for a client connection, and a recv for the server connection.
However I still need to experiment with how these calls will concurrently work with awaiting on secureEstablishedP and abortP.
I'm thinking that this is actually necessary:
Promise.race([
 Promise.all([secureEstablishedP, sendOrRecvP]),
 abortP
])

If the abortP rejects first, then we need to handle that with a somewhat graceful exit...
If however one of the other rejects first. In the sense that secureEstablishedP can reject due to EventQUICConnectionError or EventQUICConnectionEnd. But sendOrRecvP can only reject due to internal errors. But because EventQUICConnectionError also is dispatched in those cases, one can argue there's no need to catch any error from sendOrRecvP. I'm also somewhat unclear on the exact order of events. It could be that by dispatching events, which then reject promises, that would trigger secureEstablishedP to reject first, before errors get thrown upwards to sendOrRecvP.
Anyway exception handler of QUICConnection.start cannot call QUICConnection.stop because they have to be independent, so QUICConnection.start has to manage the stopping itself. That means triggering close in a similar way, and then triggering QUICConnection.send.
Also note that the conn.timeout() will only be non-null upon the first send. So client conn's first send, and server conn's first send too.

I'm refactoring the conn timeout timer and keepalive timers. Some notes I've made here: https://github.com/MatrixAI/js-timer/issues/5#issuecomment-1712719531
Note the latest changes I've pushed to js-timer isn't actually necessary here, they are quality of life improvements. And plus it's only 2.x which is ESM based anyway.
The changes required here would be:

Just use this.connTimeoutTimer?.cancel().
Take in the signal: AbortSignal inside our handler, and check it by doing if (signal.aborted) return;, but we also propagate it down to whatever each function calls, which would be await this.send(undefined, { signal });.

I just need to check with js-async-cancellable that the above call signature is correct.

@tegefaulkes I don't understand this comment:
        // No need to read after creation, doing so will throw during early cancellation

What is early cancellation here?
Also if a stream is readable, surely we want to trigger await quicStream.read();.
What is being early cancelled here?

Suppose you mean what if the stream is cancelled early. Ok sure, but then how does a stream get read then if it was not early cancelled... doesn't this delay something?
Shouldn't then early cancellation work even in the case where we run quicStream.read()?

Discovered another issue.
When the connection has max idle timed out, the connection goes straight to being closed. There is no draining phase at all.
And because it is already closed, you cannot then close(true, 0, Buffer.from('abc')) as that would throw a Done exception.
At the same time, there is no localError() nor peerError().
Basically there's no error at all.
So that creates a bit of a problem. I was expecting any if a connection was closed, there would also be either a localError or peerError. But when a connection is closed due to exhausting the max idle timeout, there's just no error.
We may need to synthetically construct one for our error events and/or the close events.

This means that there's no connection close frame sent when the connection is timed out.

If a max_idle_timeout is specified by either endpoint in its transport parameters (Section 18.2), the connection is silently closed and its state is discarded when it remains idle for longer than the minimum of the max_idle_timeout value advertised by both endpoints.

Solved that issue above for now with a new kind of Close event.
Also I think naming standard should be extended to be:

Subject Object Adjective Verb Adverb

Maintains the SOV order, but also puts describing words at the end.
Example: ErrorQUICConnectionConfigInvalid instead of ErrorQUICConnectionInvalidConfig.

Events are now:
class EventQUICConnectionError extends EventQUICConnection<
  | ErrorQUICConnectionLocal<unknown> // errors that I'm sending to the peer (also due to idle timeout)
  | ErrorQUICConnectionPeer<unknown> // errors that I'm receiving from the peer
  | ErrorQUICConnectionIdleTimeout<unknown>
  | ErrorQUICConnectionInternal<unknown> // other internal errors which means the thing is broken
> {}

class EventQUICConnectionClose extends EventQUICConnection<
  { type: 'local' | 'peer' } & ConnectionError
  |
  { type: 'timeout' }
> {}

The order of events is always error then close.
These flows are possible:
// STOP -> ERROR -> CLOSE -> STOPPED
// ERROR -> CLOSE -> STOP -> STOPPED

In order to simplify things, the error event is emitted for both connection status errors whether graceful or not, and internal socket errors. I based this functionality off the tcp net socket which emits errors primarily due to internal socket errors, and nothing to do with the remote peer or yourself. It's possible to distinguish the reason by adding 1 event handler and then investigating the class/type of the event detail.
The close event then is emitted to match the close event in net socket. It also tells us whether it was a local close, a remote close or idle timeout (where there is no error at all).
I'm still not sure whether one should separate the error event. Or maybe even not emit error event in the case of a connection status close, and just rely on the close event instead. But for now just left at this stage.
Another interesting thing popped out is that when doing socket.write() the tcp socket if it hits an error will both emit the error, and pass it to the callback. The equivalent in our case is rejecting the promise.
However in our case, it only makes sense to reject the promise in the case of an internal error, not due to connection closing errors. So I matched the behaviour here.
Thus one can assume a foundational behaviour similar to net socket events. Where if an error event occurs, a close event will always follow. Just that in our case an error event also means connection status close.

Now I'm trying to get QUICStream to behave the same:
class EventQUICStreamError extends EventQUICStream<
  | ErrorQUICStreamLocalRead<unknown> // I may send out errors on both stop sending (shutdown read) and reset stream (shutdown write)
  | ErrorQUICStreamLocalWrite<unknown> // I may send out errors on both stop sending (shutdown read) and reset stream (shutdown write)
  | ErrorQUICStreamPeerRead<unknown> // I may receive errors on both stop sending (shutdown read) and reset stream (shutdown write)
  | ErrorQUICStreamPeerWrite<unknown> // I may receive errors on both stop sending (shutdown read) and reset stream (shutdown write)
  | ErrorQUICStreamInternal<unknown>
> {}

/**
 * QUIC stream readable side was closed
 */
class EventQUICStreamCloseRead extends EventQUICStream<{
  type: 'local' | 'peer';
  code: number;
}> {}

/**
 * QUIC stream writable side was closed
 */
class EventQUICStreamCloseWrite extends EventQUICStream<{
  type: 'local' | 'peer';
  code: number;
}> {}

Yes, when timing out there isn't a specific error on peerError or localError. However there is a flag we can check with isTimedout

If I recall, there may be a race condition with cancelling the stream. This would likely cause any readableController.X() calls to throw since the stream would've already ended. On reflection the change and comment there is probably not the best. We should have a boolean guarding any stream controller calls withing QUICstream.read() or QUICStream.write().

Thinking about this further, actually we could propagate a push from QUICConnection to QUICSocket.
The way it would work is similar but it was important to realise the bridging importance of QUICClient and QUICServer.
So QUICConnection pushes events to QUICClient or QUICServer whichever is the parent to it.
The QUICClient and QUICServer would then handle this event, and convert it to a "pull" on their QUICSocket. This is true whether the QUICSocket is encapsulated or shared, it doesn't matter.
Then it is the responsibility of QUICClient and QUICServer to decide how to deal with the errors from QUICSocket.send and it can know that since the send occurs through a valid QUICConnection, any failure here represents an internal error that therefore leads to shutting down everything.
The QUICServer and QUICClient would then proceed to force shutdown the QUICStream, the QUICConnection and the QUICSocket. Such a forced shutdown should be passing "internal error codes" down the object hierarchy. That translates to readable streams being cancelled and writable streams aborted with a reason (which means readers get done: true, writers get the abortion exception). The connection also gets force closed, but also with an internal error (which would proceed to dispatch a EventQUICConnectionSend event, but we would have already removed such a listener). Finally the quic socket gets stopped with an error too, which due to using dgram socket, would mean we just proceed to closing the UDP socket.
Doing such a thing would mean QUICConnection and QUICStream is completely IO-less. They would be testable without involving an actual udp socket, since all state transitions and changes occurs just through pull methods and push events.

Note that dgram sockets don't have an equivalent .destroy(error) function like net sockets do. Errors can be send-specific... so actually even if a send were to fail, it doesn't necessarily mean the next send would fail. It could be an intermittent failure. That actually means we may not need to be so strict on a failed this.socket.send call. We could just log out the error and proceed. The QUICConnection would be maintaining state, and if the socket send failed, it would eventually retry and resend such data. See in the dgram docs, an error event doesn't automatically mean a subsequent close event. And if no data can be sent, eventually max idle timeout will happen.
On the other hand, we use this.socket.send when negotiating a new connection. And if it fails at any point, we end up shutting down everything too. I wonder... if we should be loose or strict about this? We could also just ignore the connection negotiation if the this.socket.send didn't work.

I've refactored the QUICStream ReadableStream and WritableStream sides.
For now all the logic is inside each step stream to avoid too much complexity spread.
Todo:

Refactor the QUICStream.read and QUICStream.write functions that unpauses the writable side and readable side.
Refactor the QUICStream.closeRecv and QUICStream.closeSend...

The dummy data writes look kind of weird. What's the usage of dummy data @tegefaulkes?
With the way streams are done right now. We always dispatch error and close events. When write is called the whole idea is to unpause the writableP by resolving it if the function exists.
I don't think it's necessary to check if the stream is actually writable, that should be done inside the web stream.... which it is all done inside now.

I believe simply:
public write(): void {
  this.resolveWritableP?.();
}

Is sufficient.

Even for read this is fine:
public read(): void {
  this.resolveReadableP?.();
}

I don't understand what the rest of the code was for...?

I think QUICStream is well refactored. Time to fix up the rest of the stuff, and propagate EventQUICStreamSend and EventQUICConnectionSend.

So I ended up factoring out the readable.cancel and writable.abort back into a shared protected method like before, so it ends up being callable by both readable.cancel, writable.abort and QUICStream.cancel.
Force cancellation of the quic streams from outside of the readers/writers is tricky because if there are existing readers/writers that are locked on the stream, then you cannot just call readable.cancel and writable.abort. Using this.readableController.error and this.writableController.error also doesn't work because it doesn't clean up the underlying quiche state.
So we have protected readableCancel and protected writableAbort that ends up doing this duty for both internal cancel and abort, and QUICStream level force cancellation.

It was also important to ensure that QUICStream.cancel does not in fact throw any errors up too. So I remove the throw e; in readableCancel and writableAbort.

Unidirectional streams is now supported. Basically depending on the unidirectional direction, the QUICStream may have its readable or writable automatically closed upon creation. I had to do this because it was getting awkward handling unidirectional streams in case another QUIC protocol was interacting with it. It ended up being cleaner anyway to support it.
QUICConnection now pushes events upwards using EventQUICConnectionSend.
This was originally in QUICConnection. It needs to be moved to QUICClient and QUICServer.
      try {
        await this.socket.send(
          sendBuffer,
          0,
          sendLength,
          sendInfo.to.port,
          sendInfo.to.host,
        );
      } catch (e) {
        const e_ = new errors.ErrorQUICConnectionInternal(
          'Failed to send data on the QUICSocket',
          {
            data: {
              msg: sendBuffer,
              offset: 0,
              length: sendLength,
              host: sendInfo.to.host,
              port: sendInfo.to.port,
            },
            cause: e,
          }
        );
        // This is a software error
        this.conn.close(
          false,
          ConnectionErrorCode.InternalError,
          Buffer.from('')
        );
        this.dispatchEvent(
          new events.EventQUICConnectionError({ detail: e_ })
        );
        this.dispatchEvent(new events.EventQUICConnectionClose(
          {
            detail: {
              type: 'local',
              ...this.conn.localError()!
            }
          }
        ));
        throw e_;
      }

Interesting notes... you are listening on a push event from an encapsulated object, and this leads you perform an internal pull call (essentially an asynchronous state transition), then you should in fact drop errors by doing await this.doSomethingAsync().catch(() => {}); in case of caller errors.
This is because is your own internal handler of an encapsulated event. Caller errors have to be dropped there.
Instead you rely on the domain error transition, thus your own evented object will just dispatch the necessary error events inside the callstack of this.doSomethingAsync.
Not sure what you'd call this, but it's like you're subscribed to something and doing pull calls internally, you don't want caller errors to bubble up to become an unhandled promise rejection when your domain errors will eventually be dispatched.

The QUICServer is now bridging events from EventQUICConnectionSend to QUICSocket.
Now the issue is between QUICServer and QUICSocket. They are still mutually calling each other.
If we were to break mutual dependency, it would make sense for QUICSocket to dispatch events to the QUICServer instead.
However this complicates things because the QUICSocket is shared between many QUICClient. So any dispatch event would be inefficiently broadcasting function calls.
I came up with an idea so that QUICSocket would instead dispatch specific events to each client or server depending on a the connection ID. However I think it's a radical departure from the usage of QUICConnectionMap.
So for now I think it should be ok for QUICSocket and QUICServer to be tightly integrated where they have mutual recursion. We just need to manage errors from both together whenever a call occurs. Generally speaking, control flow always starts from QUICSocket.handleSocketMessage. That will call QUICServer.acceptConnection, which will then end up calling QUICSocket.send. Thus all exceptions bubble up to the handleSocketMessage as the root control flow point.

Ok I think the architecture is finally refactored to support push flows properly.
Testing is needed @tegefaulkes can you join in and help here? Lots of commentary required and bug checking.

Task 4 is achieved by using EventAll in QUICConnection, QUICServer, QUICClient.

Discussion about mutual recursion between QUICServer and QUICSocket (and QUICConnection)

@tegefaulkes
Linting commits should only be done in 2 places:

With any specific commit
At the end of the PR

Otherwise, the linting commits break git blame and it's difficult to know what the sequence of things are. Now I can't always do 1. because I'm also pushing up commits with intermediate broken code.

I do linting with every commit. even with broken code since it still works fine, you just need to ignore the warnings. So i'd prefer it if we do linting for every commit.

As for the utils.
extractSocket Extracts the socket out of a QUICServer or QUICClient and adds it to an array. An afterEach iterates over all the sockets and forces them to close. This is a fail-safe for making sure tests don't hold the process open since an active socket will do that. It's mainly to prevent the tests from never ending if they fail and skip clean up. Otherwise CI will take an hour to end if a socket wasn't closed.
handleStreamProm A utility for do something in the background. Used to handle streams gracefully without a whole bunch of boilerplate. Will take a stream, read it until it ends while concurrently writing the data you provided. It will insure that the stream is fully cleaned up if it otherwise fails. Will also log the results.
waitForTimeoutNull In a while loop will wait the delay returned by conn.timeout() until it returns null. Calls onTimeout() at each step. Used mostly to trigger a state change in the native connection tests.
connStats Just prints out a bunch of useful information about connections. Not used currently but was used a bunch when exploring what exactly changed about the connection.
generateConfig generates a TLS config of a certain key type. used everywhere in tests since server connection require a config.
createReasonConverters Creates a matching pair of reasonToCode and codeToReason arrow functions. reasonToCode will store the reason in a map and return an incrementing code. codeToReason will retrieve the reason from the map with the given code. Allows us to seamlessly see what reason caused an error for the stream or connection.

Seems like both handleStreamProm and extractSocket is similar. I think it would better to abstract the entire concept to a killSockets fixture. Something you'd use and apply all relevant sockets as an array and it would actually execute afterEach or just afterAll?

handleStreamProm is pretty different. It's just gracefully handling the streams as a promise you can await later. It prevents a whole bunch of boiler plate, especially in the concurrency testing.
extractSocket is different in that it's just getting the protected socket property from the client or server. We just destroy them in the afterEach peferibly or the AfterAll.

handleStreamProm is pretty different. It's just gracefully handling the streams as a promise you can await later. It prevents a whole bunch of boiler plate, especially in the concurrency testing.
extractSocket is different in that it's just getting the protected socket property from the client or server. We just destroy them in the afterEach peferibly or the AfterAll.

The handleStreamProm should be called flushQUICStreamP - give it a big comment explaining that it is flushing both readable and writable until graceful close.
The extractSocket should be turned into a factory function that is executed on the beforeEach and the returned deallocator is called on the afterEach. This pattern we have done before for PK agent process lifecycle testing.

TODO:

QUIC socket failed send should not result in domain error - @CMCDragonkai
QUICConnection.test.ts and QUICClient.test.ts and QUICServer.test.ts and QUICStream.test.ts are all integration tests - that means their fixtures just setup the client and server, but their tests/expectations focus-in on their specification objects, so QUICConnection.test.ts might use nested describes to create different client/server scenarios, and then their tests would focus on the lifecycle of the QUICConnection and test different kinds of starting and stopping and closing scenarios. - @tegefaulkes

With node 20 you cannot do globalThis.crypto = ... it just doesn't work.
So I've removed that from tests/utils.ts.
Also due to jest globals, it's not possible to use tests/utils inside benches, so instead I've used benches/suites/utils.ts.
The benches is now matching the structure that is in Polykey that @amydevs is using in js-ws too.
I've also rewritten the generateConfig into generateTLSConfig and you should copy that over instead of using generateConfig in tests/utils.ts @tegefaulkes. This is in benches/suites/utils.ts.
Atm, the benchmarks isn't working. I'm getting something like this:
[nix-shell:~/Projects/js-quic]$ npm run bench

><EMAIL_ADDRESS>bench
> rimraf ./benches/results && ts-node ./benches

Running "streams.stream_1KiB" suite...
>>>>>>>>> HANDLING THE QUIC SERVER STREAM
Progress: 100%

  send 1Kib of data over QUICStream:
    685 ops/s, ±11.33%   | fastest

Finished 1 case!

Saved to: /home/cmcdragonkai/Projects/js-quic/benches/results/streams/stream_1KiB.json

Saved to: /home/cmcdragonkai/Projects/js-quic/benches/results/streams/stream_1KiB.chart.html

Saved to: /home/cmcdragonkai/Projects/js-quic/benches/results/streams/stream_1KiB_metrics.txt
ERROR:stream_1KiB Bench:ErrorQUICConnectionLocal: QUIC Connection local error - Stopping connection due to local error
ERROR:stream_1KiB Bench.QUICConnection 55c518844f342ef6ed8028f8f9df968fb6e5c59c:ErrorQUICConnectionLocal: QUIC Connection local error - Stopping connection due to local error
^C

There's a few issues:

It's so freaking slow!
It's not properly cleaning up quic streams, it seems there's an error with closing either the stream or the connection, either way the bench doesn't terminate.
The benchmarks unfortunately don't really have a teardown function that can work, we only have an initial setup. This complicates things...

@amydevs Found the reason why I couldn't import from tests/utils.ts. It's because I was using:
import { fc } from '@fast-check/jest';

I changed to using:
import fc from 'fast-check';

Now I can import from tests/utils.ts. That keeps it simple.

I've moved generateTLSConfig back into tests/utils.ts @tegefaulkes please switch over to using it and remove generateConfig and associated types.

Ok I got a working script here. It seems that if things are closed/destroyed too quickly we're getting some weird errors. It was required to put the await testsUtils.sleep(1000); there to allow it all gracefully finish.

import Logger, { LogLevel, StreamHandler, formatting } from '@matrixai/logger';
import QUICClient from './src/QUICClient';
import QUICServer from './src/QUICServer';
import * as events from './src/events';
import * as utils from './src/utils';
import * as testsUtils from './tests/utils';

async function main() {
  const logger = new Logger(`stream_1KiB Bench`, LogLevel.INFO, [
    new StreamHandler(
      formatting.format`${formatting.level}:${formatting.keys}:${formatting.msg}`,
    ),
  ]);
  const data1KiB = Buffer.alloc(1024);
  const tlsConfig = await testsUtils.generateTLSConfig('RSA');
  const quicServer = new QUICServer({
    config: {
      verifyPeer: false,
      key: tlsConfig.leafKeyPairPEM.privateKey,
      cert: tlsConfig.leafCertPEM,
    },
    crypto: {
      key: await testsUtils.generateKeyHMAC(),
      ops: {
        sign: testsUtils.signHMAC,
        verify: testsUtils.verifyHMAC,
      },
    },
    logger: logger.getChild('QUICServer'),
  });
  quicServer.addEventListener(
    events.EventQUICServerConnection.name,
    // @ts-ignore
    (evt: events.EventQUICServerConnection) => {
      const connection = evt.detail;
      connection.addEventListener(
        events.EventQUICConnectionStream.name,
        // @ts-ignore
        async (evt: events.EventQUICConnectionStream) => {
          const stream = evt.detail;
          // Graceful close of writable
          process.stderr.write('>>>>>>>>> HANDLING THE QUIC SERVER STREAM\n');
          await stream.writable.close();
          // Consume until graceful close of readable
          for await (const _ of stream.readable) {
            // Do nothing, only consume
          }
          process.stderr.write('<<<<<<<< HANDLED THE QUIC SERVER STREAM\n');
        }
      );
    }
  );
  await quicServer.start();
  const quicClient = await QUICClient.createQUICClient({
    host: utils.resolvesZeroIP(quicServer.host),
    port: quicServer.port,
    config: {
      verifyPeer: false,
    },
    crypto: {
      ops: {
        randomBytes: testsUtils.randomBytes,
      },
    },
    logger: logger.getChild('QUICClient'),
  });

  const stream = await quicClient.connection.newStream();
  const reader = stream.readable.getReader();
  const writer = stream.writable.getWriter();

  for (let i = 0; i < 1000; i++) {
    await writer.write(data1KiB);
  }

  // This should already be done, because it was closed
  await reader.cancel();

  await writer.close();

  await testsUtils.sleep(1000);

  process.stderr.write('BEFORE QUICCLIENT DESTROY\n');

  // No need to force, streams should already be closed
  // If your force is true by default, then we are technically force closing streams
  // It will cause an error
  await quicClient.destroy({ force: false });

  process.stderr.write('DESROYED\n');

  // If the connections are all gone, we shouldn't need to do this
  await quicServer.stop({ force: false });
}

void main();

This produces a log like:

INFO:stream_1KiB Bench.QUICServer:Start QUICServer on [::]:0
INFO:stream_1KiB Bench.QUICServer.QUICSocket:Start QUICSocket on [::]:0
INFO:stream_1KiB Bench.QUICServer.QUICSocket:Started QUICSocket on [::]:34198
INFO:stream_1KiB Bench.QUICServer:Started QUICServer on [::]:34198
INFO:stream_1KiB Bench.QUICClient:Create QUICClient to [::1]:34198
INFO:stream_1KiB Bench.QUICClient.QUICSocket:Start QUICSocket on [::]:0
INFO:stream_1KiB Bench.QUICClient.QUICSocket:Started QUICSocket on [::]:54495
INFO:stream_1KiB Bench.QUICClient.QUICConnection 7d8d5d89f3bfd6c270ea04d2f8bca50d9d2720a6:Connect QUICConnection
INFO:stream_1KiB Bench.QUICClient.QUICConnection 7d8d5d89f3bfd6c270ea04d2f8bca50d9d2720a6:Start QUICConnection
INFO:stream_1KiB Bench.QUICServer.QUICConnection cd739e144eaee22eef767da287daa6e8208552d5:Accept QUICConnection
INFO:stream_1KiB Bench.QUICServer.QUICConnection cd739e144eaee22eef767da287daa6e8208552d5:Start QUICConnection
INFO:stream_1KiB Bench.QUICClient.QUICConnection 7d8d5d89f3bfd6c270ea04d2f8bca50d9d2720a6:Started QUICConnection
INFO:stream_1KiB Bench.QUICClient:Created QUICClient to [::1]:34198
INFO:stream_1KiB Bench.QUICClient.QUICConnection 7d8d5d89f3bfd6c270ea04d2f8bca50d9d2720a6.QUICStream 0:Create QUICStream
INFO:stream_1KiB Bench.QUICClient.QUICConnection 7d8d5d89f3bfd6c270ea04d2f8bca50d9d2720a6.QUICStream 0:Created QUICStream
INFO:stream_1KiB Bench.QUICServer.QUICConnection cd739e144eaee22eef767da287daa6e8208552d5:Started QUICConnection
INFO:stream_1KiB Bench.QUICServer.QUICConnection cd739e144eaee22eef767da287daa6e8208552d5.QUICStream 0:Create QUICStream
INFO:stream_1KiB Bench.QUICServer.QUICConnection cd739e144eaee22eef767da287daa6e8208552d5.QUICStream 0:Created QUICStream
>>>>>>>>> HANDLING THE QUIC SERVER STREAM
INFO:stream_1KiB Bench.QUICClient.QUICConnection 7d8d5d89f3bfd6c270ea04d2f8bca50d9d2720a6.QUICStream 0:Destroy QUICStream
INFO:stream_1KiB Bench.QUICClient.QUICConnection 7d8d5d89f3bfd6c270ea04d2f8bca50d9d2720a6.QUICStream 0:Destroyed QUICStream
<<<<<<<< HANDLED THE QUIC SERVER STREAM
INFO:stream_1KiB Bench.QUICServer.QUICConnection cd739e144eaee22eef767da287daa6e8208552d5.QUICStream 0:Destroy QUICStream
INFO:stream_1KiB Bench.QUICServer.QUICConnection cd739e144eaee22eef767da287daa6e8208552d5.QUICStream 0:Destroyed QUICStream
BEFORE QUICCLIENT DESTROY
INFO:stream_1KiB Bench.QUICClient:Destroy QUICClient
INFO:stream_1KiB Bench.QUICClient.QUICConnection 7d8d5d89f3bfd6c270ea04d2f8bca50d9d2720a6:Stop QUICConnection
ERROR:stream_1KiB Bench.QUICClient:ErrorQUICConnectionLocal: QUIC Connection local error - Stopping connection due to local error
INFO:stream_1KiB Bench.QUICServer.QUICConnection cd739e144eaee22eef767da287daa6e8208552d5:Stop QUICConnection
INFO:stream_1KiB Bench.QUICClient.QUICConnection 7d8d5d89f3bfd6c270ea04d2f8bca50d9d2720a6:Stopped QUICConnection
INFO:stream_1KiB Bench.QUICClient.QUICSocket:Stop QUICSocket on [::]:54495
INFO:stream_1KiB Bench.QUICClient.QUICSocket:Stopped QUICSocket on [::]:54495
INFO:stream_1KiB Bench.QUICClient:Destroyed QUICClient
DESROYED
INFO:stream_1KiB Bench.QUICServer:Stop QUICServer on [::]:34198
INFO:stream_1KiB Bench.QUICServer.QUICConnection cd739e144eaee22eef767da287daa6e8208552d5:Stopped QUICConnection
INFO:stream_1KiB Bench.QUICServer.QUICSocket:Stop QUICSocket on [::]:34198
INFO:stream_1KiB Bench.QUICServer.QUICSocket:Stopped QUICSocket on [::]:34198
INFO:stream_1KiB Bench.QUICServer:Stopped QUICServer on [::]:34198

Reworking some of the error/info messages. I'm seeing this in my prototyping test.ts:
2023-09-15T15:27:47.185Z-INFO:stream_1KiB Bench.QUICClient.QUICConnection 200a7de2649389ae3a0e6149352171866ee82181:Stop QUICConnection
2023-09-15T15:27:47.185Z-INFO:stream_1KiB Bench.QUICClient.QUICConnection 200a7de2649389ae3a0e6149352171866ee82181:ErrorQUICConnectionLocal: QUIC Connection local error - Locally closed with application code 0
2023-09-15T15:27:47.187Z-INFO:stream_1KiB Bench.QUICServer.QUICConnection 7cf3039e8cc2aa6fc7ab4785a625c2aa54956b03:ErrorQUICConnectionPeer: QUIC Connection peer error - Peer closed with application code 0
2023-09-15T15:27:47.188Z-INFO:stream_1KiB Bench.QUICServer.QUICConnection 7cf3039e8cc2aa6fc7ab4785a625c2aa54956b03:ErrorQUICConnectionPeer: QUIC Connection peer error - Peer closed with application code 0
2023-09-15T15:27:47.188Z-INFO:stream_1KiB Bench.QUICServer.QUICConnection 7cf3039e8cc2aa6fc7ab4785a625c2aa54956b03:Stop QUICConnection

You can see here that the there are 2 of them:
2023-09-15T15:27:47.187Z-INFO:stream_1KiB Bench.QUICServer.QUICConnection 7cf3039e8cc2aa6fc7ab4785a625c2aa54956b03:ErrorQUICConnectionPeer: QUIC Connection peer error - Peer closed with application code 0
2023-09-15T15:27:47.188Z-INFO:stream_1KiB Bench.QUICServer.QUICConnection 7cf3039e8cc2aa6fc7ab4785a625c2aa54956b03:ErrorQUICConnectionPeer: QUIC Connection peer error - Peer closed with application code 0

It's a repitition of the error message.
Now this message can only come from one place under the QUICConnection.send. I believe repeated calls to QUICConnection.send while it is not stopping will basically result in this error message and multiple dispatches of this event.
Now since this is on the QUICServer, nothing is calling QUICConnection.stop yet because this is being stopped from the client first.
This was fixed in 5e5405e3b9117ef10bc156da29d2e8cf7c39a622

A few optimisations:

Remove monitor in favour of a single lock that is applied on the recv call. Because it is now chained to the send, it's possible to only force mutual exclusion on the recv and send combo. The only reason is to prevent another recv running the middle before send is called. (Even this might not be needed, we will need to see) - This improves it from 700 ops/s to 1000 ops/s.
Removal of the resolveHost in the QUICSocket.send. This I've decided to keep, it does not change the performance at all because resolveHost does not try look up via DNS if the address is not a hostname. So it has no effect.

To address 2., I created an @internal send_ that can be used without the additional checks this.socket.send_. So that way the external this.socket.send still does all the checks since the checks is unnecessary for internal sends.

Some extra ideas:

Investigate with recv and send locking is needed at all. Maybe it's not a bad idea to allow another recv to run in between before send.
Investigate if send can be coalesced if multiple areas of send are adding up to each other.
Investigate how much of recv and send can be just synchronous...?

I've changed the both send and recv config default sizes to be 1350 as per the examples in quiche.

Regarding 1. and 2., it seems it should be possible to have recv, send and processStreams entirely synchronous.
Here's why:

The sole reason why all of these are asynchronous is primarily because they infect each other with asynchronousness.
There are only 2 root asynchronous operations:

await this.config.verifyCallback
await stream.writable.close()

For the verifyCallback we can remove the ability for it to be asynchronous. It seems easy enough that checking against the certificates can be a synchronous function with any checks done entirely synchronously.
For await stream.writable.close() it is entirely an artifact of writable stream and for supporting unidirectional streams.
If we wanted to simply do it all synchronously, we could simply initiate error out on the writable, this actually makes more sense, as we cannot gracefully close a writable stream if it's unidirectional.
If we refactor this a bit, this would change the QUICStream to be entirely synchronous. Which is to say, there's no need to have an asynchronous creation function at all.
That would then remove the recv lock too. And then calling recv which then calls send and any intermediate processStreams can be entirely synchronous.
And if it is entirely synchronous, it would also be possible to entirely coalesce calls to QUICConnection.send.

It's possible to make WritableStream closed synchronously if I use controller.error at the beginning. I think that's more correct for unidirectional streams. I just need a special exception to indicate when something was never created due to being unidirectional.
const writableStream = new WritableStream({
  start(controller) {
    controller.error(new Error('LOL'));
  },
  write(chunk) {
    console.log(`Received chunk: ${chunk}`);
  },
  abort(reason) {
    console.log(`Stream aborted, reason: ${reason}`);
  },
});

const writer = writableStream.getWriter();

async function main() {
  console.log('LOL');
  try {
    await writer.close();
  } catch(e) {
    console.log('CLOSE', e.message);
  }
  try {
    console.log(await writer.closed);
  } catch(e) {
    // Closed turns into error
    console.log('CLOSED', e.message);
  }
  try {
    await writer.ready;
  } catch(e) {
    console.log('READY', e.message);
  }
  try {
    await writer.abort();
    console.log('SUCCEEDS');
  } catch (e) {
    console.log('ABORT', e.message);
  }
  console.log('DONE');
}

void main();

Note that the exception passed into controller.error is returned as part of awaiting for closed or ready or even the initial close() operation.
However once the stream is in fact closed... that exception is dropped, and instead we get a TypeError: Invalid state: WritableStream is closed.
That means when interacting with streams, it's possible to directly acquire the original reason exception... but afterwards once it is closed, you will only get the native TypeError: Invalid state: WritableStream is closed exception.
This is ok, since when interacting with streams, you'll always get your custom exception first... before the stream actually closes and thus you lose that exception and you get the native TypeError.
I suspect this is the same behaviour for ReadableStream too...
Of course I close my readable stream for unidirectional streams, thus you don't actually get an error trying to read from it, you just end up getting done: true all the time.

Actually I realise once it is unidirectional to make sure both streams perform symmetrically, it's better to instead throw errors into both streams.
See:
const readableStream = new ReadableStream({
  start(controller) {
    controller.error(new Error('LOL'))
  },
});

const writableStream = new WritableStream({
  start(controller) {
    controller.error(new Error('LOL'));
  },
});

const reader = readableStream.getReader();

const writer = writableStream.getWriter();

async function main() {

  try {
    const r = await reader.read();
    console.log(r);
  } catch (e) {
    console.log('READ', e.name, e.message);
  }

  try {
    const r = await reader.read();
    console.log(r);
  } catch (e) {
    console.log('READ', e.name, e.message);
  }

  try {
    await writer.write(Buffer.from('abc'));
  } catch (e) {
    console.log('WRITE', e.name, e.message);
  }
  try {
    await writer.write(Buffer.from('abc'));
  } catch (e) {
    console.log('WRITE', e.name, e.message);
  }

}

void main();

// Output:
// READ Error LOL
// READ Error LOL
// WRITE Error LOL
// WRITE Error LOL

Removed all asynchronicity, now everything runs synchronously but for a single stream. Now from 1000 ops/s to 1248 ops/s a big improvement, but so much slower than js-ws.
I'm beginning to think this isn't about the JS code, there's something else that is slow.
Maybe it's UDP, maybe it's the native FFI, maybe it's something that quiche is doing perhaps it is doing some sort of congestion limitation...?
@amydevs thoughts?

Even after changing the max stream size, it doesn't do anything, still stays the same speed.
Ok we're going to need to do a few things:

Establish a baseline - first by just testing the baseline speed of node's dgram socket
Then establish a baseline of just using quiche directly (this may require a bit of setup)
Then compare it against the stream writes.

If there's significant difference, the next step is bring a tracer to and flamegraph this thing!

In regards to making QUICStream synchronous. I had this thought a while back, The main problem with it right now is that the verifyCallback in Polykey is async.
So that needs looking into.

I had a look at keys utils.
The relevant functions are

certNodeSigned
certSignedBy

The reason it's async is due to the x509 library, and yes it does use the webcrypto subtle.
I forgot about this cause asymmetric can verify pub keys completely synchronously.
I think it's due to the x509 structure... it is built on top of webcrypto, and because webcrypto is async, then the x509 library is async.
I remember we have a long term issue to eventually replace webcrypto with libsodium... but libsodium doesn't have all the relevant algorithms, it has a much smaller subset. Like there's no RSA support, and no ECDSA support.
It is possible to verify the certificate entirely synchronously but it would necessitate building our own x509 library based on top of libsodium, and it would also limit the key types we can support, in fact we would probably only support the Ed25519 keys...
So yes practically right now we will need to keep using x509 and thus the verify callback will need to be asynchronous, which means we bring back asynchronicity... but to send call which makes recv call async, and we can bring back the single lock to the recv call since sends don't really need to be mutually exclusive.

I added a simple bench that tests sending 1KiB messages trough just quiche and a simple runtime using a promise and loops.
On my laptop I was getting about 2400 ops/s which is still abysmally slow.
It's possible that since we're using erros for control flow that could significantly be reducing the performance. Anything where we need to catch a Done error would be generating a stack. Generaly logic flow using try/catch is pretty slow.
So moving forward I'm updating the native code to return null in the case of Done.
But in some cases such as streamShutdown don't return anything. In this case do we return undefined | null?

Yes undefined | null in the case of streamShutdown.

OK, some new observations.
JUST doing a send loop with no other work. That is, calling connection.send() until it throws done over and over again results in 42000 ops/sec
Doing the same thing but with send returning null instead of throwing results in 270,000-ish ops/s
Doing the full simple runtime test again results in 3,300 ops/s. So there is another minor improvement but not enough.

So 3300 is still native quiche without the rest of the runtime. That's still quite slow.

There could be other exceptions we want to turn into a regular value. If that's the case, we might completely change from throwing exceptions to always returning | ErrorMessage where ErrorMessage is just a special object with a string (to distinguish from regular strings) in case some functions return just strings.
Can you trace which of the functions is the slowest whether connection send/recv or stream write/read?

I also remember changing the per stream buffer size and it had no effect on the performance. I don't get that, could it be that the way we are buffering up the stream is incorrect?

Regarding 1. that should only be an issue if it is a regular thing as part of stream writes and reads. Otherwise it shouldn't be that big of a problem. Perhaps log out whenever an error is being created.
Regarding 2. if we find a set of FFI functions that is in fact the slow ones. It's possible that something I'm doing inside rust is slow. Specifically the conversion of rust types to JS types using the napi-rs bridge. We can identify which of these is slow, and rewrite those operations.

I'm adding task 7. and I'm going to bring back asynchronicity for recv and send due to verifyCallback. @amydevs this means your verifyCallback has to be asynchronous too. Beware of integrating this into the https/tls/wss protocol.

Looking into it more all the components are reasonably fast. But having the simple runtime doing any work significantly slows it down.
I'll check if there's an issue with the simple runtime.

@tegefaulkes in the future you can directly run benchmarks without running npm run bench:
npm run ts-node -- ./benches/suites/buffers/buffer_allocation.ts
npm run ts-node -- ./benches/suites/streams/stream_1KiB.ts

So you don't need to stub it out.

Asynchronicity is back in. Now for some robustness testing.

Ok, so doing the minimial amount of work  or clientSend, client sends, process streams, server sends we get about the same amount of ops of 4000 ops/s.
Here is the break down. Its a bit by the seat of my pants. Each comment is a step that shows the ops for doing everything before that point. The results are total time units to that point, difference between this point and last point, percentage total of all time.
   // Doing nothing - 12 896 010 ops/s, ±1.05%
        clientConn.streamSend(0, data1KiB, false);
        // Just passing data to `streamSend` - 451 088 ops/s, ±1.22% | 2.22 ,, 0.88%
        while(true) {
          const result = clientConn.send(dataBuffer);
          if (result === null) break;
          // Doing clientConn.send inside loop -  89 343 ops/s, ±0.91% | 11.2, 9, 3.6%
        const [serverSendLength, sendInfo] = result;
          serverConn.recv(dataBuffer.subarray(0, serverSendLength), {
            to: sendInfo.to,
            from: sendInfo.from,
          });
          // Passing data to serverConn.recv - 82 360 ops/s, ±1.45% | 12.2, 1, 0.4%
        }
        for (const streamId of serverConn.readable()){
          // Just iterating readable - 17 924 ops/s, ±6.38% | 55.6, 43.4, 17.4% (spot check);
          while(true) {
            // read and ditch information
            if (serverConn.streamRecv(0, clientBuf) === null) break;
            // Checking streamRecv inside loop - 21 113 ops/s, ±5.68% | 47.6, 35.4, 14.2%
          }
        }
        while(true) {
          const result = serverConn.send(dataBuffer);
          if (result === null) break;
          // Doing serverConn.send inside loop - 19 118 ops/s, ±3.26% | 52.6, 5.0, 2.0%
          const [serverSendLength, sendInfo] = result;
          clientConn.recv(dataBuffer.subarray(0, serverSendLength), {
            to: sendInfo.to,
            from: sendInfo.from,
          });
          // Passing data to clientConn.recv - 3 871 ops/s, ±0.88% | 250, 197.4, 79.0%
        }

So most of the work done is just processing the data in quiche. We can see the worst points are the readable iterator and the clientConn.recv which seems really weird.

@amydevs latest fix 4d79d9e89fe5577be090fb9bf96ff026e2e2ee82 is useful for js-ws, consider your parser for the stream codes if you are using regex to ensure you limit to a maximum of 16 digits.
16 digits is due to the largest JS integer that it can represent. QUIC should in fact be limiting the size of such numbers... to u64 inside rust, but I can only see strings anyway.

On the native code, the recv call on the connection might be doing an expensive conversion of the metadata to SocketAddr. Because I'm passing a host and post pair. Where the host is a string.
I suspect it might think that the string my be a hostname and doing some sort of DNS resolution.
impl TryFrom<HostPort> for SocketAddr {
  type Error = io::Error;
  fn try_from(host: HostPort) -> io::Result<Self> {
    (host.host, host.port).to_socket_addrs()?.next().ok_or(
      io::Error::new(
        io::ErrorKind::Other,
        "Could not convert host to socket address"
      )
    )
  }
}

Not only that, the internal docs say that could block the thread when being called. That sounds like a DNS resolution.

Possible replacement:
use std::net::{SocketAddr, Ipv4Addr, Ipv6Addr};
use std::convert::TryFrom;
use std::io;

struct HostPort {
    host: String,
    port: u16,
}

impl TryFrom<HostPort> for SocketAddr {
    type Error = io::Error;

    fn try_from(host: HostPort) -> io::Result<Self> {
        if let Ok(ipv4) = host.host.parse::<Ipv4Addr>() {
            return Ok(SocketAddr::new(ipv4.into(), host.port));
        }

        if let Ok(ipv6) = host.host.parse::<Ipv6Addr>() {
            return Ok(SocketAddr::new(ipv6.into(), host.port));
        }

        Err(io::Error::new(
            io::ErrorKind::Other,
            "Could not convert host to socket address",
        ))
    }
}

Testing just the  RecvInfo part of recv in a separate function results in 266 698 ops/s, ±2.34% It's about the same for the new and old code

One of the main issues debugging things here is to ensure you're catching uncaughtException and unhandledPromiseRejection.
We allow these to occur for "software errors", but it can be quite confusing to understand where they happened.
When testing (not-in-jest cause it might not work there).
You have to put:
process.on('uncaughtException', (err, origin) => {
  console.log('Caught exception:', err);
  console.log('Exception origin:', origin);
  // Custom logic can be added here as well
});

Otherwise when running in ts-node, it shows up like this very confusingly:
/home/cmcdragonkai/Projects/js-quic/node_modules/@cspotcode/source-map-support/source-map-support.js:681
  var match = /\n    at [^(]+ \((.*):(\d+):(\d+)\)/.exec(error.stack);
                                                               ^

TypeError: Cannot read properties of undefined (reading 'stack')
    at getErrorSource (/home/cmcdragonkai/Projects/js-quic/node_modules/@cspotcode/source-map-support/source-map-support.js:681:64)
    at printFatalErrorUponExit (/home/cmcdragonkai/Projects/js-quic/node_modules/@cspotcode/source-map-support/source-map-support.js:714:16)
    at process.emit.sharedData.processEmitHook.installedValue [as emit] (/home/cmcdragonkai/Projects/js-quic/node_modules/@cspotcode/source-map-support/source-map-support.js:753:9)
    at process.exit (node:internal/process/per_thread:193:15)
    at process.emit.sharedData.processEmitHook.installedValue [as emit] (/home/cmcdragonkai/Projects/js-quic/node_modules/@cspotcode/source-map-support/source-map-support.js:750:17)
    at process._fatalException (node:internal/process/execution:159:25)

Pretty sure that's an upstream bug but we are going to switch to tsx later when ESM migration occurs.

A few tips for dealing with this:

https://stackoverflow.com/questions/51105622/how-to-ensure-jest-fails-on-unhandledrejection
https://github.com/jestjs/jest/issues/10784
https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode
https://nodejs.org/api/cli.html#--report-uncaught-exception
https://nodejs.org/api/cli.html#--trace-uncaught

Once you go evented, this can be quite annoying. Primarily due to the our Evented not having a default event catcher that can make it easier to debug this...
I'm thinking maybe our Evented should in fact have something that catches the handler exceptions by default whether by rejection or not, and then subsequently report that error in a nicer way. Can be made more obvious.

Results from the basic rust benchmarking.
This is based off of the bench that just does a streamSend, forward packets, process streams, reverse packets.
for 200_000 loops
total elapsed 13210

send elapsed 625 4%
server send elapsed 1808 13%
client recv elapsed 3753 28%
stream elapsed 672 5%
client send elapsed 2597 19%
server recv elapsed 2832 21%

As we can see, the send and recv methods takes up most of the time. I think this is expected because this is where most of the processing will happen in quiche.
I think we've exhausted the amount of scrutiny this deserves right now. Any more optimisation can come later, I think it's fast enough for our needs right now.

What does 13210 and the percentages mean?

Ok, so i've moved the example code outside of a test harness and into an example file. It's now in ./exampes/test_example. It can be run independently by doing cargo run --example test_example. I'll push this up in a moment.
As for running flamegraph, there's some setup involved.

You need perf installed. in nix that's installed with linuxPackages_latest.perf.
Install flamegraph with `

The SVG of the flamegraph

We've discovered that the difference in speed is due to the rust code being optimised which makes sense.  So the performance results currently look like this.
Optimised rust code is 250000 iterations per second
Optimised quiche code is 35000 ops per second
Optimised quic only 1896

We can conclude now that the quiche rust library isn't the problem here. We need to look into optimisations of the quic and FFI.
Likely the biggest optimisation we can do is process any send or recv calls in a separate thread in the rust layer only. But that would mean a major structural change to the quic API. I'm not sure we could use the node socket in that case.

We've discovered that the difference in speed is due to the rust code being optimised which makes sense. So the performance results currently look like this.
Optimised rust code is 250000 iterations per second
Optimised quiche code is 35000 ops per second
Optimised quic only 1896

We can conclude now that the quiche rust library isn't the problem here. We need to look into optimisations of the quic and FFI.
Likely the biggest optimisation we can do is process any send or recv calls in a separate thread in the rust layer only. But that would mean a major structural change to the quic API. I'm not sure we could use the node socket in that case.

We could still use the socket. We just have to use a shared array buffer and have all quiche calls run in a separate thread reading/writing to the shared array buffer.
Such threading could be done at the JS-level using js-workers but last I benched it, it wasn't worth it unless the call took longer than 0.5 of second or something like that.
So any threading would have be done at the native side at the ffi layer using Napi-rs threading facilities to achieve this. @amydevs knows more about this but it would not be a priority atm.

What does the colours on the flame graph mean btw?
Also I think I remember one can process multiple quic packets in recv.
1 dgram is 1 quic packet. 1 quic packet is multiple quic frames.
But I think the quiche native examples actually take potentially multiple quic packets (multiple dgrams) in one go.
Which would mean a sort of unacked buffer...
Nodejs dispatches event when every dgram arrives, we could potentially buffer it up to execute if multiple dgrams arrive to coalesce.

From what I've seen, quiche doesn't combine multiple frames into a single packet.

From what I've seen, quiche doesn't combine multiple frames into a single packet.