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

Added error handler for stdin and stdout. #4

Merged
merged 1 commit into from Nov 14, 2014

Conversation

freezy
Copy link
Contributor

@freezy freezy commented Nov 7, 2014

I have unhandled stream errors in my tests recently. Something to do with concurrency, the PNG is deleted just before or while launching pngquant, which breaks the whole test suite because the error isn't treatable.

This patch properly adds an error handler on the process' stdin and stdout, making these kind of errors treatable for the caller.

@papandreou
Copy link
Owner

Thanks for this. Is it triggered by an external process such as tmpreaper removing the temporary files while the operation is running?

Sounds like a fix that should be applied to https://github.com/papandreou/node-optipng, https://github.com/papandreou/node-pngcrush, and https://github.com/papandreou/node-inkscape as well. Can you think of a way to add an automated test for it?

@freezy
Copy link
Contributor Author

freezy commented Nov 10, 2014

It's triggered by the after() method of my Mocha test, which cleans up files. Since my setup processes images asynchronously with a message queue, it is launched even though the files might have been deleted already.

I'll look into a test case later. I haven't looked at your other projects, but if you're using the same approach, it probably needs fixing there as well.

@papandreou
Copy link
Owner

I'll take a chance and merge this in its current state. A test would be appreciated if you can come up with one later.

papandreou added a commit that referenced this pull request Nov 14, 2014
Added error handler for stdin and stdout.
@papandreou papandreou merged commit 690ad4d into papandreou:master Nov 14, 2014
@papandreou
Copy link
Owner

Released in 0.4.0.

@freezy freezy deleted the errorfix branch November 14, 2014 16:18
@freezy
Copy link
Contributor Author

freezy commented Nov 14, 2014

Thanks. Still need to finish some other stuff first, but the mental note is still in my head :)

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

Successfully merging this pull request may close these issues.

None yet

2 participants