kylefarris / clamscan

A robust ClamAV virus scanning library supporting scanning files, directories, and streams with local sockets, local/remote TCP, and local clamscan/clamdscan binaries (with failover).
MIT License
230 stars 68 forks source link

fix: node 16 passthrough multiple callback error #88

Closed ngraef closed 2 years ago

ngraef commented 2 years ago

Node 16 includes a behavior change that breaks many stream implementation functions if they use async/await syntax. Async functions implicitly return a Promise, and node's stream internals automatically call the underlying callback if the function's result is a thenable. There is some logic to skip the automatic callback if it was already called explicitly in the function; however, this can execute before other asynchronous tasks, which results in a ERR_MULTIPLE_CALLBACK error when the explicit callback is invoked. For example, the callback in this drain event handler will often trigger the error:

async transform(chunk, encoding, cb) {
  if (!forkStream.write(chunk)) {
    forkStream.once('drain', () => {
      cb(null, chunk);
    });
  } else {
    cb(null, chunk);
  }
}

This new behavior for thenables is undocumented and has already been deprecated. Unfortunately, we need to live with it for now. The most straightforward solution is to use a Promise chain instead of async/await in NodeClam passthrough()'s Transform implementation.

This PR also makes a change to use stream.Readable in jsdoc instead of ReadableStream from the experimental Web Streams API. (Complementary types change in https://github.com/DefinitelyTyped/DefinitelyTyped/pull/58698.)

Fixes #82

Refs https://github.com/nodejs/node/issues/39535, https://github.com/nodejs/node/pull/40773

kylefarris commented 2 years ago

Thanks for the PR! I'm reviewing and testing it now.

kylefarris commented 2 years ago

Hey man, very thorough explanation. You had to dive deep into this issue to understand it and I really appreciate the help! All the tests have passed--everything looks good. I'm going to merge this as soon as I add you as a contributor.

kylefarris commented 2 years ago

New version: 2.0.2 has been pushed to NPM.

ngraef commented 2 years ago

Thanks for the quick review and release!