Disposing miniflare@3 when a queue is running

[SamyPesse]

When writing unit tests where each test uses its own miniflare instance (for isolation), meaning the miniflare instance is disposed at the end of the tests, we can observe the following logs:

    MiniflareCoreError [ERR_DISPOSED]: Cannot use disposed instance

      at Miniflare.#checkDisposed (node_modules/miniflare/src/index.ts:899:13)
      at QueuesGateway.dispatchFetch (node_modules/miniflare/src/index.ts:927:10)
      at QueuesGateway.#dispatchBatch (node_modules/miniflare/src/plugins/queues/gateway.ts:119:33)
      at Timeout.#flush [as _onTimeout] (node_modules/miniflare/src/plugins/queues/gateway.ts:141:29)

It doesn't prevent the tests from working, but it's verbose. Maybe there is an opportunity to improve the lifecycle when disposing a worker with messages in the queue.

[SamyPesse]

In fact, it negatively impacts our tests and makes them unreliable, we are exploring why, but in the mid-time, we had to disable the queues for our tests.

[mrbbot]

Hey! 👋 Great catch, thanks for reporting this. We should probably update Miniflare#dispose() to wait for all queue dispatches to complete. 👍

[mrbbot]

Hmmm, alternatively, could you wait for some side effect of your queue handler in your tests? Presumably, the tests should only pass (and then dispose), once the queued message has been processed?

[SamyPesse]

could you wait for some side effect of your queue handler in your tests?

Potentially but hard to write, especially as we have multiple queues.

Presumably, the tests should only pass (and then dispose), once the queued message has been processed?

In some cases, yes, in others, the queue is just a side effect.

We should probably update Miniflare#dispose() to wait for all queue dispatches to complete. 👍

Or expose a function like miniflare.waitQueuesFinished() and not do it in dispose(). Something like miniflare.cancelAllQueues() would be great as well.

[SamyPesse]

The main issue here is that it's an uncaught exception, it makes it very hard to integrate it in jest (jestjs/jest#10364).

One alternative could be to let us easily handle these "async" errors, with something like:

const miniflare = new Miniflare();

miniflare.on('exception', () => {
  // Log it and ignore
});

[mrbbot]

Hey! 👋 I think this should've been fixed by #656. The queue dispatcher is now implemented in workerd, and uses service bindings to dispatch queue messages. When you call dispose(), the workerd process is shutdown, so it should be impossible for a message to be dispatched.