Ensure clone is only used with Stream-based input

[whmountains]

Steps to Reproduce

const sharp = require('sharp')

const main = async () => {
  const image = sharp('test.jpg')

  for (let i = 0; i < 25; i++) {
    await image
      .clone()
      .resize(800)
      .toFormat('jpeg')
      .toBuffer()
  }
}

main()

Error Message

(node:93880) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 finish listeners added. Use emitter.setMaxListeners() to increase limit

It always shows up after processing the ninth image.

[whmountains]

I can also reproduce this using old-fashioned callbacks:

const image = require('sharp')('test.jpg')
let counter = 0

const runTask = () => {
  image
    .clone()
    .resize(800)
    .toFormat('jpeg')
    .toBuffer((err, buff) => {
      console.log('done', counter++)
      if (counter < 25) {
        setImmediate(runTask)
      }
    })
}

runTask()

[lovell]

Hello, the use of clone is unnecessary here as Stream-based input is not being used.

The reason you see a warning is because Node has an internal default of 10 and this code is creating 25 instances.

[whmountains]

Thank you for the response. Knowing what function is causing the problem helps a lot!

If clone() can only be used for streaming inputs, what am I supposed to do when I need to create two independent outputs from the same input?

const image = require('sharp')('test.jpg')

// create a jpeg version that's 800px wide
image.resize(800).jpeg().toBuffer()

// create a webp version without changing the resolution
// This doesn't work because the previous output already set the width to 800px
image.webp().toBuffer()

Of course I could just output the webp image first, but that's missing the point. In a large complex app I need a way to pass a "clean" source object to many different functions without worrying about how they interact.

Thanks!

[whmountains]

Could you just add a check to the clone function that doesn't register the listener if streaming mode is disabled?

See this diff: master...whmountains:master#diff-787480a57face985b2cd22edc8bdaeb9

[lovell]

pass a "clean" source object to many different functions

The input is not mutated so you can do the following:

const sharp = require('sharp');

// create a jpeg version that's 800px wide
sharp('test.jpg').resize(800).jpeg().toBuffer()

// create a webp version without changing the resolution
sharp('test.jpg').webp().toBuffer()

If you would prefer to pass a sharp instance then perhaps try:

const sharp = require('sharp');

const image = () => sharp('test.jpg');

// create a jpeg version that's 800px wide
image().resize(800).jpeg().toBuffer()

// create a webp version without changing the resolution
image().webp().toBuffer()

Both of these approaches have the benefit of input caching within libvips.

[whmountains]

Thank you. That is a reasonable workaround.

Are you opposed to also patching clone() to allow it to be used for this as well?

[lovell]

Perhaps clone() should throw an Error when _isStreamInput() is false?

[whmountains]

That sounds reasonable to me.

[lovell]

PR welcome for this - please target at the suit branch as this would be a breaking API change.

[Razinsky]

I'm using a stream-based input and .clone() and have 16 instances. I get the same EventEmitter warning. Is there a way to silence this warning anyway?

Thanks!

[lovell]

@Razinsky see https://nodejs.org/docs/latest/api/events.html#events_emitter_setmaxlisteners_n

sharp()
  .setMaxListeners(0)
  .clone()
  ...

[Razinsky]

Oh ... yeah well I didn't saw that method was exposed :) So easy.

Thanks for the quick answer!

[lovell]

Commit 9fa04a0 adds a check before attaching the event listener. This will be in v0.19.0. Thanks for reporting this.

[lovell]

sharp v0.19.0 now available.