latest archiver versions break pptx output in some cases

[spollack]

Officegen makes use of the archiver module. With the following module set (i.e. what you get from a default install of latest officegen right now), we get intermittent failures during pptx output. Specifically, we make the call to pptx.generate(), but it often hangs partway through, never emiting either the error or finalize events.

officegen@0.2.7 node_modules/officegen
├── setimmediate@1.0.1
├── fast-image-size@0.1.2
├── archiver@0.5.2 (lazystream@0.1.0, lodash@2.4.1, file-utils@0.1.5, zip-stream@0.1.3)
└── readable-stream@1.0.25-1 (string_decoder@0.10.25)

However, with achiver rolled back to 0.4.10, it works reliably. So it isn't an officegen issue per se.

officegen@0.2.7 node_modules/officegen
├── setimmediate@1.0.1
├── fast-image-size@0.1.2
├── archiver@0.4.10 (readable-stream@1.0.17, iconv-lite@0.2.11)
└── readable-stream@1.0.25-1 (string_decoder@0.10.25)

The environment where we are testing and see issues with archiver@0.5.2 is using node 0.8.26, running on linux (heroku dynos). Interestingly, the exact same code running on my OSX dev machine doesn't have this issue. Something subtle is happening here. It happens quite often with longer powerpoints; single slide ones seem to succeed.

We put in some logging within the officegen generate() code, and were able to see that it dies inside of archiver. The exact spot within the pptx export where it dies varies from run to run.

For now, we are working around this via an npm shrinkwrap file that locks us to the old archiver@0.4.10.

The reason i'm raising the issue here is to warn officegen users, and to see if anyone else has anyone else hit this issue already, or can repro it.

I'd like to open a ticket with archiver, but i don't have a great repro case currently that doesn't involve a ton of other code.

Also, @Ziv-Barber you might want to lock officegen to archiver:~0.4.10 for now?

[spollack]

see also: archiverjs/node-archiver#60

[Ziv-Barber]

Thanks for letting me know about it.

I'll do lock it to 0.4.10

On Fri, Feb 7, 2014 at 2:02 AM, Seth Pollack notifications@github.comwrote:

see also: archiverjs/node-archiver#60archiverjs/node-archiver#60

Reply to this email directly or view it on GitHubhttps://github.com//issues/20#issuecomment-34394203
.

[ctalkington]

i highly recommend ~ over >= specially if a project isn't past 1.0.0. while i try to prevent breaking changes, i consider every minor version (before 1.0.0) to be a new version of api for interacting.

[spollack]

Thanks guys.

[ctalkington]

officegen/lib/basicgen.js

Line 318 in 60dfa66

archive.append ( result, { name: item.name }, function ( err ) {

really should be catching the error and either emitting it back to archive (for existing handler to work) or call done(err). due to the way that people used to use async module (which btw isnt required anymore, archiver handles queue internally) so emitting an error when they have a callback would sidestep their processes. I plan to go all events in archive v0.6.0 instead of callbacks but thats a month out or so.

[spollack]

definitely we should be checking and handling errors from archiver here in the officegen code. this should also help get info on whatever root problem was causing these failures for me earlier.