Errors are not displayed properly

[zbentley]

If I install a broken cask, in Puppet, I get output like the following:

==> Verifying checksum for Cask appfresh
==> Note: running "brew update" may fix sha256 checksum errors
Error: /Stage[main]/Main/Node[default]/Package[appfresh]/ensure: change from absent to present failed: Could not install package: Execution of '/usr/local/bin/brew cask install appfresh' returned 1: ==> Downloading http://backend.metaquark.de/download/appfresh/versions/909
==> Verifying checksum for Cask appfresh
==> Note: running "brew update" may fix sha256 checksum errors

However, running brew cask install appfresh yields the real error:

==> Downloading http://backend.metaquark.de/download/appfresh/versions/909
Already downloaded: /Users/zbentley/Library/Caches/Homebrew/Cask/appfresh--1.0.5_909
==> Verifying checksum for Cask appfresh
==> Note: running "brew update" may fix sha256 checksum errors
Error: sha256 mismatch
Expected: e4d4e719bebf17f0ea5e7336e5da84d032b26dfaefa2f5102250fe9a208674cc
Actual: eaa4e7d92bfe8985d626507972902b2b50d4700b69fa6e5c273132466dc5008e
File: /Users/zbentley/Library/Caches/Homebrew/Cask/appfresh--1.0.5_909
To retry an incomplete download, remove the file above.

Could the actual error output be surfaced from puppet-homebrew?

[zbentley]

This is at master, btw; not 1.5.0.

[jordigg]

Will investigate, thought was already handled and tested: https://github.com/TheKevJames/puppet-homebrew/blob/master/lib/puppet/provider/package/brewcask.rb#L21

Maybe is because some of the error messages are sent via stdout and the others via strerr and so we don't show all of them back to the user.

[zbentley]

The STDOUT/STDERR thing seems likely. Would it be possible to interleave the two streams (merge on IFS or \n or something sensible) using some Ruby tool, store the union as the output, and print that out for the user if there is an error?

Alternatively, if adding a dependency on the shell for command execution is acceptable, you could change the underlying system call to do brew ... 2>&1.

[jordigg]

Yes we already do this but want to check if the output is like other errors or in this case brew is separating them for some reason. If that's the case will need to check with them and see if can be corrected in case is a "bug". I would expect to get all error messages via the same source.

Because they don't have other exit codes to point to specific errors we have to parse text which is not ideal. Would be nice to get exit code 5 or whatever and know that in this case we have a mismatch between the downloaded file and the source and we need to download again.

Better yet on this specific case, if brew knows there's a mismatch they should handle file deletion and re-try the download themselves without user interaction. I don't understand why they don't fix the problem instead of asking the user to do it manually.

[TheKevJames]

@jordigg I think this is related to the recent change to parsing return values rather than output parsing. Now that failonfail is set here, the line below probably doesn't run on a SHA failure and so the fix_checksum method never gets called. We might be able to move this case to the rescue (if we can still identify sha errors vs other errors in that block).

It would be great to get a test case for this, also, but this seems difficult to replicate in a programmatic fashion. Any thoughts on this? We might be able to create a purposely corrupt version of some package's tarball and attempt to install over it.