fix(installer): Fix reporting of error code (#32034)

cmd/installer/main.go

@@ -29,9 +29,9 @@ func runCmd(cmd *cobra.Command) int {
 	err := cmd.Execute()
 	if err != nil {
 		if rootCauseErr := dig.RootCause(err); rootCauseErr != err {
-			fmt.Fprintln(cmd.ErrOrStderr(), installerErrors.FromErr(rootCauseErr).ToJSON())
+			fmt.Fprintln(cmd.ErrOrStderr(), installerErrors.ToJSON(rootCauseErr))
 		} else {
-			fmt.Fprintln(cmd.ErrOrStderr(), installerErrors.FromErr(err).ToJSON())
+			fmt.Fprintln(cmd.ErrOrStderr(), installerErrors.ToJSON(err))
 		}
 		return -1
 	}

pkg/fleet/daemon/daemon.go

@@ -494,7 +494,10 @@ func (d *daemonImpl) handleRemoteAPIRequest(request remoteAPIRequest) (err error
 		}
 		experimentPackage, ok := d.catalog.getPackage(request.Package, params.Version, runtime.GOARCH, runtime.GOOS)
 		if !ok {
-			return fmt.Errorf("could not get package %s, %s for %s, %s", request.Package, params.Version, runtime.GOARCH, runtime.GOOS)
+			return installerErrors.Wrap(
+				installerErrors.ErrPackageNotFound,
+				fmt.Errorf("could not get package %s, %s for %s, %s", request.Package, params.Version, runtime.GOARCH, runtime.GOOS),
+			)
 		}
 		log.Infof("Installer: Received remote request %s to start experiment for package %s version %s", request.ID, request.Package, request.Params)
 		if request.Package == "datadog-installer" {
@@ -535,10 +538,11 @@ var requestStateKey requestKey
 
 // requestState represents the state of a task.
 type requestState struct {
-	Package string
-	ID      string
-	State   pbgo.TaskState
-	Err     *installerErrors.InstallerError
+	Package   string
+	ID        string
+	State     pbgo.TaskState
+	Err       error
+	ErrorCode installerErrors.InstallerErrorCode
 }
 
 func newRequestContext(request remoteAPIRequest) (ddtrace.Span, context.Context) {
@@ -572,7 +576,8 @@ func setRequestDone(ctx context.Context, err error) {
 	state.State = pbgo.TaskState_DONE
 	if err != nil {
 		state.State = pbgo.TaskState_ERROR
-		state.Err = installerErrors.FromErr(err)
+		state.Err = err
+		state.ErrorCode = installerErrors.GetCode(err)
 	}
 }
 
@@ -633,7 +638,7 @@ func (d *daemonImpl) refreshState(ctx context.Context) {
 			var taskErr *pbgo.TaskError
 			if requestState.Err != nil {
 				taskErr = &pbgo.TaskError{
-					Code:    uint64(requestState.Err.Code()),
+					Code:    uint64(requestState.ErrorCode),
 					Message: requestState.Err.Error(),
 				}
 			}

pkg/fleet/installer/errors/errors.go

@@ -42,27 +42,17 @@ func (e InstallerError) Error() string {
 	return e.err.Error()
 }
 
-// Unwrap returns the wrapped error.
-func (e InstallerError) Unwrap() error {
-	return e.err
-}
-
 // Is implements the Is method of the errors.Is interface.
 func (e InstallerError) Is(target error) bool {
 	_, ok := target.(*InstallerError)
 	return ok
 }
 
-// Code returns the error code of the installer error.
-func (e InstallerError) Code() InstallerErrorCode {
-	return e.code
-}
-
 // Wrap wraps the given error with an installer error.
 // If the given error is already an installer error, it is not wrapped and
 // left as it is. Only the deepest InstallerError remains.
 func Wrap(errCode InstallerErrorCode, err error) error {
-	if FromErr(err).code != errUnknown {
+	if errors.Is(err, &InstallerError{}) {
 		return err
 	}
 	return &InstallerError{
@@ -71,50 +61,40 @@ func Wrap(errCode InstallerErrorCode, err error) error {
 	}
 }
 
-// FromErr returns a new InstallerError from the given error.
-// Unwraps the error until it finds an InstallerError and return unknown error code if not found.
-func FromErr(err error) *InstallerError {
-	if err == nil {
-		return nil
+// GetCode returns the installer error code of the given error.
+func GetCode(err error) InstallerErrorCode {
+	code := errUnknown
+	e := &InstallerError{}
+	if ok := errors.As(err, &e); ok {
+		code = e.code
 	}
-	e, ok := err.(*InstallerError)
-	if !ok {
-		unwrappedErr := errors.Unwrap(err)
-		if unwrappedErr == nil {
-			return &InstallerError{
-				err:  err,
-				code: errUnknown,
-			}
-		}
-		return FromErr(unwrappedErr)
-	}
-	return e
+	return code
 }
 
 // ToJSON returns the error as a JSON string.
-func (e InstallerError) ToJSON() string {
+func ToJSON(err error) string {
 	tmp := installerErrorJSON{
-		Error: e.err.Error(),
-		Code:  int(e.code),
+		Error: err.Error(),
+		Code:  int(GetCode(err)),
 	}
 	jsonErr, err := json.Marshal(tmp)
 	if err != nil {
-		return e.err.Error()
+		return err.Error()
 	}
 	return string(jsonErr)
 }
 
 // FromJSON returns an InstallerError from a JSON string.
-func FromJSON(errStr string) InstallerError {
+func FromJSON(errStr string) *InstallerError {
 	var jsonError installerErrorJSON
 	err := json.Unmarshal([]byte(errStr), &jsonError)
 	if err != nil {
-		return InstallerError{
+		return &InstallerError{
 			err:  errors.New(errStr),
 			code: errUnknown,
 		}
 	}
-	return InstallerError{
+	return &InstallerError{
 		err:  errors.New(jsonError.Error),
 		code: InstallerErrorCode(jsonError.Code),
 	}

pkg/fleet/installer/errors/errors_test.go

@@ -6,40 +6,34 @@
 package errors
 
 import (
+	"errors"
 	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 )
 
-func TestFromErr(t *testing.T) {
+func TestGetCode(t *testing.T) {
+	// Nil case
+	assert.Equal(t, GetCode(nil), errUnknown)
+
+	// Simple case
 	var err error = &InstallerError{
 		err:  fmt.Errorf("test: test"),
 		code: ErrDownloadFailed,
 	}
-	taskErr := FromErr(err)
-	assert.Equal(t, taskErr, &InstallerError{
-		err:  fmt.Errorf("test: test"),
-		code: ErrDownloadFailed,
-	})
+	assert.Equal(t, GetCode(err), ErrDownloadFailed)
 
-	assert.Nil(t, FromErr(nil))
-}
-
-func TestFromErrWithWrap(t *testing.T) {
-	err := fmt.Errorf("test: %w", &InstallerError{
-		err:  fmt.Errorf("test: test"),
-		code: ErrDownloadFailed,
-	})
-	taskErr := FromErr(err)
-	assert.Equal(t, taskErr, &InstallerError{
-		err:  fmt.Errorf("test: test"),
+	// Wrap
+	err = fmt.Errorf("test1: %w", &InstallerError{
+		err:  fmt.Errorf("test2: test3"),
 		code: ErrDownloadFailed,
 	})
+	assert.Equal(t, GetCode(err), ErrDownloadFailed)
 
-	taskErr2 := fmt.Errorf("Wrap 2: %w", fmt.Errorf("Wrap 1: %w", taskErr))
-	assert.Equal(t, FromErr(taskErr2).Code(), ErrDownloadFailed)
-	assert.Nil(t, FromErr(nil))
+	// Multiple wraps
+	err = fmt.Errorf("Wrap 2: %w", fmt.Errorf("Wrap 1: %w", err))
+	assert.Equal(t, GetCode(err), ErrDownloadFailed)
 }
 
 func TestWrap(t *testing.T) {
@@ -59,5 +53,22 @@ func TestWrap(t *testing.T) {
 	})
 
 	taskErr3 := Wrap(ErrFilesystemIssue, fmt.Errorf("Wrap 2: %w", fmt.Errorf("Wrap 1: %w", taskErr2)))
-	assert.Equal(t, FromErr(taskErr3).Code(), ErrDownloadFailed)
+	unwrapped := &InstallerError{}
+	assert.True(t, errors.As(taskErr3, &unwrapped))
+	assert.Equal(t, unwrapped.code, ErrDownloadFailed)
+}
+
+func TestToJSON(t *testing.T) {
+	err := fmt.Errorf("test: %w", &InstallerError{
+		err:  fmt.Errorf("test2: test3"),
+		code: ErrDownloadFailed,
+	})
+	assert.Equal(t, ToJSON(err), `{"error":"test: test2: test3","code":1}`)
+}
+
+func TestFromJSON(t *testing.T) {
+	json := `{"error":"test: test2: test3","code":1}`
+	err := FromJSON(json)
+	assert.Equal(t, err.Error(), "test: test2: test3")
+	assert.Equal(t, GetCode(err), ErrDownloadFailed)
 }

pkg/fleet/internal/exec/installer_exec.go

@@ -252,5 +252,5 @@ func (iCmd *installerCmd) Run() error {
 	}
 
 	installerError := installerErrors.FromJSON(strings.TrimSpace(errBuf.String()))
-	return fmt.Errorf("run failed: %v \n%s", installerError, err.Error())
+	return fmt.Errorf("run failed: %w \n%s", installerError, err.Error())
 }