Merge pull request #142 from UiPath/feature/configurable-attempts-and…

…-timeout

Configurable call timeouts and max attempts

commandline/command_builder.go

@@ -326,6 +326,14 @@ func (b CommandBuilder) createOperationCommand(operation parser.Operation) *Comm
 				tenant = config.Tenant
 			}
 			insecure := context.Bool(FlagNameInsecure) || config.Insecure
+			timeout := time.Duration(context.Int(FlagNameCallTimeout)) * time.Second
+			if timeout < 0 {
+				return fmt.Errorf("Invalid value for '%s'", FlagNameCallTimeout)
+			}
+			maxAttempts := context.Int(FlagNameMaxAttempts)
+			if maxAttempts < 1 {
+				return fmt.Errorf("Invalid value for '%s'", FlagNameMaxAttempts)
+			}
 			debug := context.Bool(FlagNameDebug) || config.Debug
 			identityUri, err := b.createIdentityUri(context, *config, baseUri)
 			if err != nil {
@@ -343,6 +351,8 @@ func (b CommandBuilder) createOperationCommand(operation parser.Operation) *Comm
 				parameters,
 				config.Auth,
 				insecure,
+				timeout,
+				maxAttempts,
 				debug,
 				*identityUri,
 				operation.Plugin)

commandline/flag_builder.go

@@ -21,6 +21,8 @@ const FlagNameIdentityUri = "identity-uri"
 const FlagNameServiceVersion = "service-version"
 const FlagNameHelp = "help"
 const FlagNameVersion = "version"
+const FlagNameCallTimeout = "call-timeout"
+const FlagNameMaxAttempts = "max-attempts"
 
 const FlagValueFromStdIn = "-"
 const FlagValueOutputFormatJson = "json"
@@ -33,6 +35,8 @@ var FlagNamesPredefined = []string{
 	FlagNameOrganization,
 	FlagNameTenant,
 	FlagNameInsecure,
+	FlagNameCallTimeout,
+	FlagNameMaxAttempts,
 	FlagNameOutputFormat,
 	FlagNameQuery,
 	FlagNameWait,
@@ -118,6 +122,14 @@ func (b FlagBuilder) defaultFlags(hidden bool) []*FlagDefinition {
 			WithEnvVarName("UIPATH_INSECURE").
 			WithDefaultValue(false).
 			WithHidden(hidden),
+		NewFlag(FlagNameCallTimeout, "Call Timeout", FlagTypeInteger).
+			WithEnvVarName("UIPATH_CALL_TIMEOUT").
+			WithDefaultValue(60).
+			WithHidden(true),
+		NewFlag(FlagNameMaxAttempts, "Max Attempts", FlagTypeInteger).
+			WithEnvVarName("UIPATH_MAX_ATTEMPTS").
+			WithDefaultValue(3).
+			WithHidden(true),
 		NewFlag(FlagNameOutputFormat, fmt.Sprintf("Set output format: %s (default), %s", FlagValueOutputFormatJson, FlagValueOutputFormatText), FlagTypeString).
 			WithEnvVarName("UIPATH_OUTPUT").
 			WithDefaultValue("").

executor/execution_context.go

@@ -2,6 +2,7 @@ package executor
 
 import (
 	"net/url"
+	"time"
 
 	"github.com/UiPath/uipathcli/config"
 	"github.com/UiPath/uipathcli/plugin"
@@ -21,6 +22,8 @@ type ExecutionContext struct {
 	Parameters   ExecutionParameters
 	AuthConfig   config.AuthConfig
 	Insecure     bool
+	Timeout      time.Duration
+	MaxAttempts  int
 	Debug        bool
 	IdentityUri  url.URL
 	Plugin       plugin.CommandPlugin
@@ -37,8 +40,26 @@ func NewExecutionContext(
 	parameters []ExecutionParameter,
 	authConfig config.AuthConfig,
 	insecure bool,
+	timeout time.Duration,
+	maxAttempts int,
 	debug bool,
 	identityUri url.URL,
 	plugin plugin.CommandPlugin) *ExecutionContext {
-	return &ExecutionContext{organization, tenant, method, uri, route, contentType, input, parameters, authConfig, insecure, debug, identityUri, plugin}
+	return &ExecutionContext{
+		organization,
+		tenant,
+		method,
+		uri,
+		route,
+		contentType,
+		input,
+		parameters,
+		authConfig,
+		insecure,
+		timeout,
+		maxAttempts,
+		debug,
+		identityUri,
+		plugin,
+	}
 }

executor/http_executor.go

@@ -13,7 +13,6 @@ import (
 	"net/url"
 	"runtime"
 	"strings"
-	"time"
 
 	"github.com/UiPath/uipathcli/auth"
 	"github.com/UiPath/uipathcli/config"
@@ -44,7 +43,7 @@ type HttpExecutor struct {
 }
 
 func (e HttpExecutor) Call(context ExecutionContext, writer output.OutputWriter, logger log.Logger) error {
-	return resiliency.Retry(func() error {
+	return resiliency.RetryN(context.MaxAttempts, func() error {
 		return e.call(context, writer, logger)
 	})
 }
@@ -345,7 +344,7 @@ func (e HttpExecutor) call(context ExecutionContext, writer output.OutputWriter,
 
 	transport := &http.Transport{
 		TLSClientConfig:       &tls.Config{InsecureSkipVerify: context.Insecure}, //nolint // This is user configurable and disabled by default
-		ResponseHeaderTimeout: 60 * time.Second,
+		ResponseHeaderTimeout: context.Timeout,
 	}
 	client := &http.Client{Transport: transport}
 	if context.Debug {

test/show_command_test.go

@@ -141,7 +141,23 @@ paths:
 		names = append(names, parameter["name"].(string))
 	}
 
-	expectedNames := []string{"debug", "profile", "uri", "organization", "tenant", "insecure", "output", "query", "wait", "wait-timeout", "file", "identity-uri", "service-version", "help"}
+	expectedNames := []string{
+		"debug",
+		"profile",
+		"uri",
+		"organization",
+		"tenant",
+		"insecure",
+		"call-timeout",
+		"max-attempts",
+		"output",
+		"query",
+		"wait",
+		"wait-timeout",
+		"file",
+		"identity-uri",
+		"service-version",
+		"help"}
 	if !reflect.DeepEqual(names, expectedNames) {
 		t.Errorf("Unexpected global parameters in output, expected: %v but got: %v", expectedNames, names)
 	}

test/validation_test.go

@@ -6,6 +6,48 @@ import (
 	"testing"
 )
 
+func TestInvalidCallTimeoutShowsError(t *testing.T) {
+	definition := `
+paths:
+  /ping:
+    get:
+      summary: Simple ping
+`
+
+	context := NewContextBuilder().
+		WithDefinition("myservice", definition).
+		WithResponse(200, "").
+		Build()
+
+	result := RunCli([]string{"myservice", "get-ping", "--call-timeout", "-1"}, context)
+
+	expected := "Invalid value for 'call-timeout'"
+	if !strings.Contains(result.StdErr, expected) {
+		t.Errorf("stderr does not invalid call-timeout error error, expected: %v, got: %v", expected, result.StdErr)
+	}
+}
+
+func TestInvalidMaxAttemptsShowsError(t *testing.T) {
+	definition := `
+paths:
+  /ping:
+    get:
+      summary: Simple ping
+`
+
+	context := NewContextBuilder().
+		WithDefinition("myservice", definition).
+		WithResponse(200, "").
+		Build()
+
+	result := RunCli([]string{"myservice", "get-ping", "--max-attempts", "0"}, context)
+
+	expected := "Invalid value for 'max-attempts'"
+	if !strings.Contains(result.StdErr, expected) {
+		t.Errorf("stderr does not invalid call-attempts error error, expected: %v, got: %v", expected, result.StdErr)
+	}
+}
+
 func TestMissingRequiredParameterShowsError(t *testing.T) {
 	definition := `
 paths:

utils/resiliency/retry.go

@@ -8,11 +8,16 @@ const MaxAttempts = 3
 
 // Retries the given function up to 3 times when it returns an RetryableError.
 func Retry(f func() error) error {
+	return RetryN(MaxAttempts, f)
+}
+
+// Retries the given function up to n times when it returns an RetryableError.
+func RetryN(maxAttempts int, f func() error) error {
 	var err error
 	for i := 1; ; i++ {
 		err = f()
 		_, retryable := err.(*RetryableError)
-		if !retryable || i == MaxAttempts {
+		if !retryable || i == maxAttempts {
 			return err
 		}
 		time.Sleep(1 * time.Second)