Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZipStream will keep writing to the readable side after destroy #167

Open
matthieusieben opened this issue Mar 23, 2023 · 1 comment
Open

Comments

@matthieusieben
Copy link

In addition to the error reported in #166, destroying a zip stream will not be handled properly internally:

Running this code (slightly different from the one in #166) will return the error bellow

'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() // (1)
  }, 2000)

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

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

void main()
generate start
generate item 1
got zip chunk of size 4
got zip chunk of size 2
got zip chunk of size 2
got zip chunk of size 2
got zip chunk of size 4
got zip chunk of size 4
got zip chunk of size 4
got zip chunk of size 4
got zip chunk of size 2
got zip chunk of size 2
got zip chunk of size 9
got zip chunk of size 16384
got zip chunk of size 16384
got zip chunk of size 16384
got zip chunk of size 16384
got zip chunk of size 16384
got zip chunk of size 12570
got zip chunk of size 2715
got zip chunk of size 4
got zip chunk of size 4
got zip chunk of size 4
got zip chunk of size 4
generate item 2
zip err NodeError: Cannot call write after a stream was destroyed
    at doWrite (~/node_modules/readable-stream/lib/_stream_writable.js:409:38)
    at writeOrBuffer (~/node_modules/readable-stream/lib/_stream_writable.js:398:5)
    at ZipStream.Writable.write (~/node_modules/readable-stream/lib/_stream_writable.js:307:11)
    at ZipStream.ArchiveOutputStream.write (~/node_modules/compress-commons/lib/archivers/archive-output-stream.js:116:36)
    at ZipStream.ZipArchiveOutputStream._writeLocalFileHeader (~/node_modules/compress-commons/lib/archivers/zip/zip-archive-output-stream.js:390:8)
    at ZipStream.ZipArchiveOutputStream._appendBuffer (~/node_modules/compress-commons/lib/archivers/zip/zip-archive-output-stream.js:74:8)
    at ZipStream.ArchiveOutputStream.entry (~/node_modules/compress-commons/lib/archivers/archive-output-stream.js:86:10)
    at ZipStream.entry (~/node_modules/zip-stream/index.js:166:49)
    at ~/test.js:32:13
    at new Promise (<anonymous>)
zip err NodeError: Cannot call write after a stream was destroyed
    at doWrite (~/node_modules/readable-stream/lib/_stream_writable.js:409:38)
    at writeOrBuffer (~/node_modules/readable-stream/lib/_stream_writable.js:398:5)
    at ZipStream.Writable.write (~/node_modules/readable-stream/lib/_stream_writable.js:307:11)
    at ZipStream.ArchiveOutputStream.write (~/node_modules/compress-commons/lib/archivers/archive-output-stream.js:116:36)
    at ZipStream.ZipArchiveOutputStream._writeLocalFileHeader (~/node_modules/compress-commons/lib/archivers/zip/zip-archive-output-stream.js:393:8)
    at ZipStream.ZipArchiveOutputStream._appendBuffer (~/node_modules/compress-commons/lib/archivers/zip/zip-archive-output-stream.js:74:8)
    at ZipStream.ArchiveOutputStream.entry (~/node_modules/compress-commons/lib/archivers/archive-output-stream.js:86:10)
    at ZipStream.entry (~/node_modules/zip-stream/index.js:166:49)
    at ~/test.js:32:13
    at new Promise (<anonymous>)
[... many more of these] 
$ node -v
v16.19.1
@ctalkington
Copy link
Member

ctalkington commented Mar 10, 2024

thinking out loud, the desired behavior would be a single error emitted and abort trying to write anything further?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants