fix(setupServer): suppress "ERR_INVALID_ARG_TYPE" (AbortSignal) errors from "setMaxListeners" in jsdom

[christoph-fricke]

This PR adds a fix for the runtime error in JSDOM that has been reported by @luchsamapparat in this comment.

Node.js setMaxListeners checks whether or not the given event target is a Node.js EventTarget (implementation) and throws an error otherwise. This check is done through an internally defined Symbol("kIsNodeEventTarget") (implementation), so JSDOM and Happy-DOM are not able to implement it in their patched EventTarget.

However, the main reason for calling setMaxListeners (see #1765) in the first place is to prevent memory-leak warnings printed by Node.js. The good thing is that the functionality for this also relies on a Node.js internal Symbol('events.maxEventTargetListenersWarned') (implementation). Therefore, Node.js just won't print memory-leak warnings if the AbortSignal is not based on Node.js EventTarget. We simply don't have to increase the max listeners warnings in JSDOM environment, since it will just not have warning problems.

Since the implementations of AbortSignal differ mostly in internally defined Node.js symbols, we are also not able to differentiate them - at least I found no solution. The best I found is a simple try/catch. Please let me know if you find a better solution than the one proposed in this PR.

[kettanaito]

Hey, @christoph-fricke. Thanks so much for working on this!

The test you've added doesn't fail if I remove the fix on Node.js 18.14.2. What version of Node.js are you using when you're reproducing this?

[christoph-fricke]

Hey @kettanaito, it has been quite fun to do a deep dive into this problem and the Node.js source code to try and find better solution for identifying Node.js abort signals. The experience alone has justified the effort. ;)

I am currently using the latest LTS version, i.e. v18.18.2. However, I wasn't able to run the added tests here due to fetch not being defined. Locally I created a little reproduction based on Vitest and JSOM/Happy-DOM. Maybe Jest adds some quirks, which prevent the problem.

[kettanaito]

@christoph-fricke, I still can't reproduce this issue even on Node.js 18.18.2.

Locally I created a little reproduction based on Vitest and JSOM/Happy-DOM

This may be the pickle here. Afaik, Vitest uses Happy-DOM, which may differ from JSDOM. I do plan on migrating to Vitest once 2.0 is out, I love that framework and a couple of my other projects are already using it to great success. Perhaps the issue will be reproducible then.

But, to be fair, I don't mind more aggressive safe-guarding around this. Developers shouldn't suffer due to the way their test environment polyfills Node.js globals (this phrase make me very sad).

[luchsamapparat]

@christoph-fricke, I still can't reproduce this issue even on Node.js 18.18.2.

Not even with the reproduction repo?
https://github.com/luchsamapparat/msw-jsdom-abort-controller-reproduction

[luchsamapparat]

btw, Vitest by default uses neither jsdom not happy-dom, but node as standard environment (see https://vitest.dev/config/#environment). You have to actively configure them when testing browser/web apps.

I observed the problem for both jsdom and happy-dom.

[kettanaito]

@luchsamapparat, I can reproduce the issue in your repo. Thanks for putting that together.

Caused by: TypeError: The "eventTargets" argument must be an instance of EventEmitter or EventTarget. Received an instance of AbortSignal
 ❯ EventEmitter.setMaxListeners node:events:326:17
 ❯ SetupServerApi.<anonymous> node_modules/msw/lib/node/index.mjs:51:7
 ❯ node_modules/msw/lib/node/index.mjs:18:61
 ❯ __async node_modules/msw/lib/node/index.mjs:2:10
 ❯ _Emitter.<anonymous> node_modules/msw/lib/node/index.mjs:50:44
 ❯ emitAsync node_modules/@mswjs/interceptors/lib/node/chunk-YQGTMMOZ.mjs:50:20
 ❯ node_modules/@mswjs/interceptors/lib/node/interceptors/fetch/index.mjs:59:35

[kettanaito]

@christoph-fricke, I'm adding a more narrow condition here so we don't suppress all the thrown errors (those are still useful!).

When this particular error happens, Node.js throws an ERR_INVALID_ARG_TYPE error code. Let's depend on that and ignore only it.

[kettanaito]

I can verify that this change fixes the reported issue:

 ✓ src/index.test.ts (1)
   ✓ example

[kettanaito]

Looks good from my side. Let's wait for the CI and merge this.

[weyert]

Thanks for working on this! Looks like it was quite dive into the Node code base 🤿