zip.entry callback never called when stream destroyed

[matthieusieben]

Try the following implementation and notice how it causes a dead lock:

'use strict'

const { promisify } = require('node:util')
const ZipStream = require('zip-stream')

const sleep = promisify(setTimeout)

async function* generateItems() {
  // Fake ressource usage. This will cause the process to never exit if the finally block is never executed.
  const interval = setInterval(() => {}, 10)

  console.error('generate start')

  let i = 0
  try {
    while (i++ < 5) {
      await sleep(1000)
      console.error('generate item', i)
      yield { data: Buffer.alloc(100_000_990), name: 'hello.txt', date: new Date() }
    }
  } finally {
    // Clean ressources
    clearInterval(interval)
    console.error('generate done', i)
  }
}

async function populateZip(zip, asyncIterable) {
  try {
    for await (const item of asyncIterable) {
      await new Promise((resolve, reject) => {
        zip.entry(item.data, { name: item.name, date: item.date }, (err, res) => {
          if (err) reject(err)
          else resolve(res)
        })
      })
    }
    zip.finalize()
  } catch (err) {
    console.error('populateZip err', err)
    // Item generation failed or zip.entry() failed
    zip.destroy(err)
  }
}

async function main() {
  const zip = new ZipStream({ zlib: { level: 9 } })

  populateZip(zip, generateItems())

  setTimeout(() => {
    zip.destroy()
  }, 1000)

  zip.once('error', (err) => {
    console.error('zip err', err)
  })

  zip.on('data', (chunk) => {
    console.error('got zip chunk of size', chunk.length)
  })
}

void main()

This is due to the fact that the entry callback is never called when the stream is destroyed.

IMHO the zip.entry callback should always be called, in this case with either a "the stream was destroyed" error or a (fake) success. The second option has the advantage of being backwards compatible.

$ node -v
v16.19.1

[matthieusieben]

It is possible to workaround this issue in userland but it yields more complex code:

async function populateZip(zip, asyncIterable) {
  const iterator = asyncIterable[Symbol.asyncIterator]()

  zip.on('error', () => iterator.return?.())
  zip.on('close', () => iterator.return?.())

  try {
    while (true) {
      const { value, done } = await iterator.next()
      if (done) break
      if (zip.destroyed) return

      await new Promise((resolve, reject) => {
        zip.entry(value.data, { name: value.name, date: value.date }, (err, res) => {
          if (err) reject(err)
          else resolve(res)
        })
      })
    }
    zip.finalize()
  } catch (err) {
    // Might be iterator error or zip entry error
    zip.destroy(err)

    // No need to re-throw as the zip will emit an "error" event
    // throw err
  } finally {
    iterator.return?.()
  }
}