Event 'finished' no longer valid

[yuryoparin]

Hi @jed. I've recently tried to build a chrome extension using grunt-crx, which uses your crx module and failed to do so. After a few hours of crawling through the code, I spotted a bug in https://github.com/jed/crx/blob/master/src/crx.js#L125:
archive.on("finish",function() {...

In a new version of archiver's dependency: readable-stream, there's a line 704 which listens for the 'readable' event but not for the 'finish' event (https://github.com/isaacs/readable-stream/blob/master/lib/_stream_readable.js#L704). Also this event is listed in NodeJS Stream docs: http://nodejs.org/api/stream.html (see section: "Event: 'readable'").

After changing that line 125 in src/crx.js to "archive.on("readable",function() {...", everything worked OK. Can you test it yourself and fix the issue.

My versions:
node version: v0.10.29
crx version: 0.4.3
readable-stream: 1.0.27-1
uname -a: Linux server-two 2.6.32-431.5.1.el6.x86_64 #1 SMP Wed Feb 12 00:41:43 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux

[jed]

hey there @yuryoparin, do you think you could turn this into a PR?

[yuryoparin]

@jed, the behaviour of the stream turned out to be very weird. 'Finish' or 'end' events of the stream won't get fired unless the 'readable' event is listened to. Although in ZipStream module (https://github.com/ctalkington/node-zip-stream/blob/master/lib/zip-stream.js#L298) specifying an 'end' listener works perfectly.

Just listening to the 'readable' event is not enough though, because it fires after every new file is being 'zipped' and added to the stream. So I use Buffer.concat([this.contents, archive.read()]) in the 'readable' event callback and go back to the main flow (https://github.com/jed/crx/blob/master/src/crx.js#L43) in the 'finish' event callback.

I'll try to make a pull request ASAP.

[localjo]

I'm running into a problem related to this pull request. I'm trying to build a Chrome extension using grunt-crx and when I run it I get the following feedback, which I traced back to https://github.com/jed/crx/blob/master/src/crx.js#L130

Running "crx:production" (crx) task
Verifying property crx.production exists in config...OK
Files: extension/ -> dist/devtools-name-0.1.0.crx
Reading .../../data/config-default.json...OK
Parsing  .../../data/config-default.json...OK
Reading cert/devtools-name.pem...OK
Reading extension/manifest.json...OK
Parsing extension/manifest.json...OK
Reading package.json...OK
Parsing package.json...OK
Fatal error: Cannot read property 'length' of null

I'm not really sure what's going on here, but I'm not able to build a crx file.

[yuryoparin]

Hi @josiahsprague. What version of node are you using?

[localjo]

@yuryoparin I'm using node 0.11.12-pre

[yuryoparin]

@josiahsprague can you check the code in the archiver's dependency "readable-stream" in node_modules (as in here https://github.com/isaacs/readable-stream/blob/master/lib/_stream_readable.js#L704). Does it have a hook on the 'readable' event?

Also can you put console.log(this.contents) above https://github.com/jed/crx/blob/master/src/crx.js#L130 and give the grunt output.

[youbbi]

The problem is still present in master, and I have the exact same symptoms than @josiahsprague.
I am using node 0.11.12

My workaround until this issue is fixed is to use nvm and fallback to node 0.10

# nvm use 0.10

In this node environment, the crx can be built.

[thom4parisot]

This issue is surfaced with Node 0.11 on CI. Have you guys found out more about the origin of the issue in Node 0.11?

[thom4parisot]

It should be addressed by #20.