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

Properly wrap Ruby StandardError w/ add'l context #390

Merged
merged 1 commit into from
Feb 22, 2017

Conversation

mattbrictson
Copy link
Member

Prior to this PR, any Ruby StandardError (such as socket connection errors) would not be rescued or interpreted by SSHKit. In a scenario where you are running commands on multiple hosts, this meant the error would be devoid of any context, like on which host the error occurred.

This behavior was actually a mistake: we were intending to rescue and wrap all Ruby StandardErrors, but the rescue clause was written such that it only caught SSHKit::StandardError, which is something entirely different.

Fix this by referring to the correct constant, and add unit tests.

Fixes #389.

Prior to this commit, any Ruby StandardError (such as socket connection
errors) would not be rescued or interpreted by SSHKit. In a scenario
where you are running commands on multiple hosts, this meant the error
would be devoid of any context, like on which host the error occurred.

This behavior was actually a mistake: we were intending to rescue and
wrap all Ruby StandardErrors, but the rescue clause was written such
that it only caught `SSHKit::StandardError`, which is something entirely
different.

Fix this by referring to the correct constant, and add unit tests.
@will-in-wi
Copy link
Contributor

This looks good to me!

@mattbrictson
Copy link
Member Author

Thanks! I'll merge it.

@mattbrictson mattbrictson merged commit 018a3cc into master Feb 22, 2017
@leehambley
Copy link
Member

Late to the party, but this looks ace, nice one Matt!

@mattbrictson mattbrictson deleted the fix-exception-wrap branch March 5, 2017 00:47
jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Apr 22, 2017
## [1.13.1][] (2017-03-31)

### Breaking changes

  * None

### Bug fixes

  * [#397](https://github.com/capistrano/sshkt/pull/397): Fix NoMethodError assign_defaults with net-ssh older than 4.0.0 - [@shirosaki](https://github.com/shirosaki)

## [1.13.0][] (2017-03-24)

### Breaking changes

  * None

### New features

  * [#372](capistrano/sshkit#372): Use cp_r in local backend with recursive option - [@okuramasafumi](https://github.com/okuramasafumi)

### Bug fixes

  * [#390](capistrano/sshkit#390): Properly wrap Ruby StandardError w/ add'l context - [@mattbrictson](https://github.com/mattbrictson)
  * [#392](capistrano/sshkit#392): Fix open two connections with changed cache key - [@shirosaki](https://github.com/shirosaki)
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.

3 participants