Stuk / jszip

Create, read and edit .zip files with Javascript
https://stuk.github.io/jszip/
Other
9.78k stars 1.3k forks source link

jszip randomly not including files in .zip archive #529

Open rob-ubnt opened 6 years ago

rob-ubnt commented 6 years ago

I'm developing an API that allows users to download a zip archive containing log files and various other text files. For some reason, jszip randomly leaves files out of the archive. Here's an example of a file that often, but not always, gets left out:

const cp = require('child_process')
const zip = new JSZip()
const systemZip = zip.folder('system')

// ...there is code here for adding other text files to the system folder...

const createTopFile = () => {
    return new Promise((resolve, reject) => {
        cp.exec('top -b -n 1 > /tmp/top.txt', (err, stdout, stderr) => {
          if (err) return reject(err)
          resolve()
        })
    })
}

const readTopFile = () => {
    return new Promise((resolve, reject) => {
        fs.readFile('/tmp/top.txt', (err, data) => {
            if (err) reject(err)
            resolve(data)
        })
    })
}

createTopFile().then(() => {
    return readTopFile().then(data => {
        console.log('Content of /tmp/top.txt is:', data) // ...this prints "Content of /tmp/top.txt is: <Buffer 74 6f 70 20 ... >"
        systemZip.file('top.txt', data)
    })
})

zip.generateAsync({
    type: 'nodebuffer',
    compression: 'DEFLATE',
    compressionOptions: {level: 1},
    platform: process.platform
}).then(buffer => {
    res.status(200).set('Content-Type', 'application/zip').send(buffer)
}).catch(err => {
    res.status(500).json({error: err.message})
})

The /tmp/top.txt file is always created successfully (just like other files before it), and there are no errors thrown, but for some reason the system folder in the zip archive doesn't always have a top.txt file.

Any ideas why?

rob-ubnt commented 6 years ago

I think I've discovered the problem, but I'm not sure how to solve it. I added a log statement after calling the file() method...

createTopFile().then(() => {
    return readTopFile().then(data => {
        console.log('Content of /tmp/top.txt is:', data) // ...this prints "Content of /tmp/top.txt is: <Buffer 74 6f 70 20 ... >"
        systemZip.file('top.txt', data)
        console.log('Added top.txt to system folder') // ...new log statement
    })
})

...and another one in the onUpdate() function:

zip.generateAsync({
    type: 'nodebuffer',
    compression: 'DEFLATE',
    compressionOptions: {level: 1},
    platform: process.platform
}, (metadata) => {
    if (metadata.currentFile) {
        this.verbose(`generateAsync() is processing ${metadata.currentFile}`) // ...new log statement
    }
}).then(buffer => {
    res.status(200).set('Content-Type', 'application/zip').send(buffer)
}).catch(err => {
    res.status(500).json({error: err.message})
})

I never see generateAsync() is processing system/top.txt in the logs. In fact, here you can see that generateAsync() starts processing my other files before top.txt is added to the system folder:

...
Added /tmp/foo.txt to system folder
Added /tmp/bar.txt to system folder
generateAsync() is processing system/
generateAsync() is processing system/foo.txt
generateAsync() is processing system/bar.txt
Added /tmp/top.txt to system folder
...

So for my top.txt file, how can I force the file() method to complete before the call to generateAsync() is executed?

maximerety commented 5 years ago

I fell upon this issue recently, and It does not seem to be an issue with jszip itself.

The problem here seems to be a race condition due to incorrect promise chaining at the end of the code snippet given above. It was probably meant to be written as:

createTopFile().then(() => {
    return readTopFile();
}).then(data => {
    systemZip.file('top.txt', data);

    return zip.generateAsync({
        type: 'nodebuffer',
        compression: 'DEFLATE',
        compressionOptions: {level: 1},
        platform: process.platform
    });
}).then(buffer => {
    res.status(200).set('Content-Type', 'application/zip').send(buffer)
}).catch(err => {
    res.status(500).json({error: err.message})
});
ikbal-nayem commented 3 weeks ago

Did you fix it @rob-ubnt? I am having the same problem.