ipfs / js-ipfs-utils

IPFS utils
Other
23 stars 30 forks source link

Bug: Too much event listeners attached to an AbortSignal #274

Open ukstv opened 1 year ago

ukstv commented 1 year ago

Let's assume you call ipfs.dag.get(cid, '/s/o/m/e/v/e/r/y/l/o/n/g/p/a/t/h', {signal: abortSignal}) where ipfs is an IPFS HTTP Client. The control flow would eventually go to ipfs-http-client resolve function, and then end up in src/http.js fetch function of this package.

Every path element would add an event listener to the original abortSignal. Eventually, if the path is long enough, Node.js starts emitting MaxListenersExceededWarning warnings. I consider this a bug.

Now any-signal library allows you to clear event listeners after use. The solution to the warnings problem might be to call signal.clear after response is done. I would happily contribute, yet any-signal can not really be used here due to CJS/ESM incompatibility: You can not use ESM any-signal from CJS js-ipfs-utils package. See also: https://github.com/ipfs/js-ipfs-utils/issues/266

welcome[bot] commented 1 year ago

Thank you for submitting your first issue to this repository! A maintainer will be here shortly to triage and review. In the meantime, please double-check that you have provided all the necessary information to make this process easy! Any information that can help save additional round trips is useful! We currently aim to give initial feedback within two business days. If this does not happen, feel free to leave a comment. Please keep an eye on how this issue will be labeled, as labels give an overview of priorities, assignments and additional actions requested by the maintainers:

Finally, remember to use https://discuss.ipfs.io if you just need general support.

avious00 commented 1 year ago

+1 we've seen this become a developer experience pain point - several forum and discord posts on this, e.g. [1] [2] [3]

avious00 commented 1 year ago

@achingbrain you helped us on the anysignal portion, can we do it again? :D

avious00 commented 1 year ago

hi @achingbrain - we're still seeing errors from this