search

jbenet / node-subcomandante

taaaaake ooooooon meeee
4 stars 8 forks source link

Fix issue around exit events #5

Open JGAntunes opened 8 years ago

JGAntunes commented 8 years ago

This should fix the issues on this PR -> https://github.com/ipfs/js-ipfsd-ctl/pull/89, according to what's described here https://github.com/ipfs/js-ipfsd-ctl/pull/89#issuecomment-234232228

Basically the current implementation of node-subcomandante may emit an error after the duplex stream closes based on the process exit code. This makes it impossible to know if the process finished successfully or not. To fix it I added another event which is emitted if no error occurred. I named it success but let me know if you think we should name it something else.

daviddias commented 8 years ago

@JGAntunes awesome! :D

To make the API simpler, you can just follow the streams convention where an 'error' emits the 'error' event followed by the 'close' event, while a 'close' is emitted by itself. I'm not sure if duplexer follows this pattern correctly (probably does), but I know duplexify (https://www.npmjs.com/package/duplexify) will.

In the end, you get if 'error' was emitted, it is because it failed, if 'close' was emitted by itself, then it succeeded.

Also, could you please add a test?

daviddias commented 7 years ago

@JGAntunes THis is not working locally for me now for exactly the same issue I'm seeing https://github.com/ipfs/js-ipfsd-ctl/pull/160, might not be an issue with your PR but with master itself.

JGAntunes commented 7 years ago

Hey @diasdavid totally forgot about this :smile: will take a look at it between today and tomorrow :+1: