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

exec: always return at least an empty ExecResponse #203

Merged
merged 1 commit into from
Aug 9, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 12 additions & 4 deletions exec/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,12 @@ func (r *RunParams) Wait() (*ExecResponse, error) {
}
logger.Infof("run result: %v", ee)
}
return result, err

if err != nil {
return nil, errors.Trace(err)
}

return result, nil
}

// ErrCancelled is returned by WaitWithCancel in case it successfully manages to kill
Expand Down Expand Up @@ -232,7 +237,10 @@ func (r *RunParams) WaitWithCancel(cancel <-chan struct{}) (*ExecResponse, error

select {
case resWithError := <-done:
return resWithError.execResult, errors.Trace(resWithError.err)
if resWithError.err != nil {
return nil, errors.Trace(resWithError.err)
}
return resWithError.execResult, nil
case <-cancel:
logger.Debugf("attempting to kill process")
err := r.KillProcess(r.ps.Process)
Expand All @@ -243,8 +251,8 @@ func (r *RunParams) WaitWithCancel(cancel <-chan struct{}) (*ExecResponse, error
// After we issue a kill we expect the wait above to return within timeWaitForKill.
// In case it doesn't we just go on and assume the process is stuck, but we don't block
select {
case resWithError := <-done:
return resWithError.execResult, ErrCancelled
case <-done:
return nil, ErrCancelled
case <-_clock.After(timeWaitForKill):
return nil, errors.Errorf("tried to kill process %v, but timed out", r.ps.Process.Pid)
}
Expand Down
4 changes: 0 additions & 4 deletions exec/exec_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@ import (
"github.com/juju/utils/exec"
)

// 0 is thrown by linux because RunParams.Wait
// only sets the code if the process exits cleanly
const cancelErrCode = 0

func (*execSuite) TestRunCommands(c *gc.C) {
newDir := c.MkDir()

Expand Down
4 changes: 1 addition & 3 deletions exec/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ func (*execSuite) TestWaitWithCancel(c *gc.C) {
cancelChan <- struct{}{}
result, err := params.WaitWithCancel(cancelChan)
c.Assert(err, gc.Equals, exec.ErrCancelled)
c.Assert(string(result.Stdout), gc.Equals, "")
c.Assert(string(result.Stderr), gc.Equals, "")
c.Assert(result.Code, gc.Equals, cancelErrCode)
c.Assert(result, gc.IsNil)
}

func (s *execSuite) TestKillAbortedIfUnsuccessfull(c *gc.C) {
Expand Down
3 changes: 0 additions & 3 deletions exec/exec_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ import (
"github.com/juju/utils/exec"
)

// 1 is thrown by powershell after the a command is cancelled
const cancelErrCode = 1

// longPath is copied over from the symlink package. This should be removed
// if we add it to gc or in some other convenience package
func longPath(path string) ([]uint16, error) {
Expand Down