Skip to content

Commit

Permalink
exec: always return at least an empty ExecResponse
Browse files Browse the repository at this point in the history
  • Loading branch information
bogdanteleaga committed Mar 31, 2016
1 parent 55eebc9 commit a5eb7ad
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 14 deletions.
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

0 comments on commit a5eb7ad

Please sign in to comment.