search

elastic / elastic-transport-js

Transport classes and utilities shared among Node.js Elastic client libraries
Apache License 2.0
4 stars 24 forks source link

TypeError [ERR_INVALID_ARG_TYPE]: The "event" argument must be an instance of Event #73

Closed andreainnocenti closed 6 months ago

andreainnocenti commented 8 months ago

It's not uncommon that somebody already opened an issue or in the best case it's already fixed but not merged. That's the reason why you should search at first before submitting a new one.

Please read this entire template before posting any issue. If you ignore these instructions and post an issue here that does not follow the instructions, your issue might be closed, locked, and assigned the not reproducible label.

🐛 Bug Report

According to https://nodejs.org/api/events.html#eventtargetdispatcheventevent, the dispatchEvent function must take in input an Event object but in https://github.com/elastic/elastic-transport-js/blob/v8.3.4/src/connection/UndiciConnection.ts#L144, the input is a string.

Currently our microservice crashes if the search reaches the timeout:

 TypeError [ERR_INVALID_ARG_TYPE]: The "event" argument must be an instance of Event. Received type string ('abort')
    at new NodeError (node:internal/errors:405:5)
    at EventTarget.dispatchEvent (node:internal/event_target:687:13)
    at Timeout._onTimeout (/opt/app-root/src/node_modules/@elastic/transport/lib/connection/UndiciConnection.js:128:36)
    at listOnTimeout (node:internal/timers:569:17)
    at process.processTimers (node:internal/timers:512:7) {
  code: 'ERR_INVALID_ARG_TYPE'
}

To Reproduce

Steps to reproduce the behavior:

Expected behavior

Do not crash in case of timeouts.

Your Environment

andreainnocenti commented 8 months ago

I was able to reproduce the bug also in your unit test.

Screenshot 2023-11-03 at 11 32 43

I only removed this line https://github.com/elastic/elastic-transport-js/blob/main/test/unit/undici-connection.test.ts#L27C1-L27C7. The package node-abort-controller should not be used in Node.js version >= 16 as described in https://www.npmjs.com/package/node-abort-controller#nodejs-library-only-supports-node-16-or-above.

If you use the native AbortController definition, the app crashes.

JoshMock commented 8 months ago

Good catch @andreainnocenti. I'll pick this up and get it fixed soon.

andreainnocenti commented 7 months ago

Hi @JoshMock, any update on this bug?

JoshMock commented 7 months ago

I got sidetracked! I'm planning to publish some upgrades in the next few days so I'll try to fit this in.

JoshMock commented 7 months ago

Got a fix for this in https://github.com/elastic/elastic-transport-js/pull/78. I have another PR pending before I planned to push a new version to npm, but if that gets delayed I will try to remember to push a patch version in the next few days.

JoshMock commented 6 months ago

8.4.0 was published today, including a fix for this.