Skip to content

fix(ssh): remove unreachable code in connectOrDie, wrap last error#655

Merged
ArangoGutierrez merged 1 commit intoNVIDIA:mainfrom
ArangoGutierrez:fix/connect-or-die
Feb 13, 2026
Merged

fix(ssh): remove unreachable code in connectOrDie, wrap last error#655
ArangoGutierrez merged 1 commit intoNVIDIA:mainfrom
ArangoGutierrez:fix/connect-or-die

Conversation

@ArangoGutierrez
Copy link
Collaborator

Summary

  • Remove dead code after retry loops in both provisioner and dryrun connectOrDie
  • Wrap last dial error with %w so callers can inspect the root cause

Audit Findings

Changes

  • pkg/provisioner/provisioner.go: Simplify connectOrDie retry loop
  • cmd/cli/dryrun/dryrun.go: Simplify connectOrDie retry loop + wrap error

Test plan

  • gofmt — no formatting issues
  • go build — compiles
  • go test ./pkg/... — all tests pass

After the retry loop, connectionFailed is always true, making the
final return statement unreachable. Simplify to unconditionally return
the error after the loop. Also wrap the last dial error with %w so
callers see the root cause (e.g. 'connection refused').

Audit findings NVIDIA#5 (HIGH), NVIDIA#6 (HIGH), NVIDIA#27 (LOW).

Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>
Copilot AI review requested due to automatic review settings February 12, 2026 20:27
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes unreachable code and improves error handling in two connectOrDie functions that establish SSH connections with retry logic. The changes address three audit findings (#5, #6, and #27) by removing dead code that was never executed after retry loops and wrapping the final error with %w to enable proper error chain inspection.

Changes:

  • Removed dead code (unreachable connectionFailed flag and return statements) after retry loops
  • Updated error formatting to wrap the last error with %w instead of discarding it
  • Improved code clarity by eliminating unnecessary boolean flag logic

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
pkg/provisioner/provisioner.go Simplified connectOrDie retry loop by removing dead code and wrapping the last SSH dial error with %w
cmd/cli/dryrun/dryrun.go Simplified connectOrDie retry loop by removing dead code, introducing explicit lastErr tracking, and wrapping the error with %w

@coveralls
Copy link

Pull Request Test Coverage Report for Build 21963044306

Details

  • 0 of 2 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.06%) to 47.556%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pkg/provisioner/provisioner.go 0 2 0.0%
Totals Coverage Status
Change from base Build 21955389842: 0.06%
Covered Lines: 2500
Relevant Lines: 5257

💛 - Coveralls

@ArangoGutierrez ArangoGutierrez merged commit 39dc654 into NVIDIA:main Feb 13, 2026
25 checks passed
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