Catch cask errors to continue cleanup in install, upgrade, reinstall, and uninstall#21615
Conversation
When `Cask::Upgrade.upgrade_casks!` or `Cask::Reinstall.reinstall_casks` raises, the exception now gets caught with `ofail` (which sets `Homebrew.failed = true`) instead of propagating up and skipping cleanup, pkgconf reinstall, and message display. This matches how formula upgrades already handle `BuildError`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extend the same pattern from the previous commit to `uninstall` and the new-cask install path in `install`. In `uninstall`, both `Cask::Uninstall.uninstall_casks` and `Cask::Installer#zap` can raise, which would skip `Cleanup.autoremove`. In `install`, the `new_casks.each` loop calling `Cask::Installer#install` can raise, which would skip formula installations, `Cleanup.periodic_clean!`, and message display. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Looks good when 🟢, thanks @koddsson!
|
Hello! I hope I've followed all the guidance on LLMs as well as the other contributing info. The PR description doesn't really go into detail for this but I'm trying to make sure that The error that is thrown is: And everything is still installed. I'm fine with the one cask erroring but I wish also uninstalled the rest. Now, maybe there's a good reason for this but I figured I could try starting on a fix with a LLM since I'm not super versed with the codebase. IF this is something that we'd like to have in |
So quick! ❤️ |
Summary
install,upgrade,reinstall, anduninstallcommands withbegin/rescueblocks that callofailinstead of letting exceptions propagateBuildErrorTest plan
uninstallcatching cask errors and settingHomebrew.failedupgradecatching cask errors and settingHomebrew.failedbrew tests --online --changedpassesbrew typecheckpassesbrew style --fix --changedpasses🤖 Generated with Claude Code