Skip to content

Commit d744c76

Browse files
authored
Fix the incorrect error message output of the command cadence workflow start/run (#7182)
Why? Fixes #6726 What changed? Fix the incorrect error message output of the command cadence workflow start/run. Improve the usage message of cadence workflow start/run. How did you test it? unit tests, local functional tests Potential risks Release notes Documentation Changes
1 parent ffc2ad3 commit d744c76

File tree

3 files changed

+88
-5
lines changed

3 files changed

+88
-5
lines changed

tools/cli/flags.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -412,11 +412,11 @@ func getFlagsForStart() []cli.Flag {
412412
},
413413
&cli.IntFlag{
414414
Name: FlagRetryExpiration,
415-
Usage: "Optional retry expiration in seconds. If set workflow will be retried for the specified period of time.",
415+
Usage: "Optional retry expiration in seconds. If set workflow will be retried for the specified period of time. retry_attempts and retry_expiration must not both be 0.",
416416
},
417417
&cli.IntFlag{
418418
Name: FlagRetryAttempts,
419-
Usage: "Optional retry attempts. If set workflow will be retried the specified amount of times.",
419+
Usage: "Optional retry attempts. If set workflow will be retried the specified amount of times. retry_attempts and retry_expiration must not both be 0.",
420420
},
421421
&cli.IntFlag{
422422
Name: FlagRetryInterval,
@@ -426,7 +426,7 @@ func getFlagsForStart() []cli.Flag {
426426
&cli.Float64Flag{
427427
Name: FlagRetryBackoff,
428428
Value: 1.0,
429-
Usage: "Optional retry backoff coeficient. Must be or equal or greater than 1.",
429+
Usage: "Optional retry backoff coefficient. Must be or equal or greater than 1.",
430430
},
431431
&cli.IntFlag{
432432
Name: FlagRetryMaxInterval,

tools/cli/workflow_commands.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ func startWorkflowHelper(c *cli.Context, shouldPrintProgress bool) error {
322322
resp, err := serviceClient.StartWorkflowExecution(tcCtx, startRequest)
323323

324324
if err != nil {
325-
return commoncli.Problem("Failed to create workflow.", err)
325+
return commoncli.Problem("Failed to create workflow.", replaceRetryPropertiesInErrorMessageWithRetryArguments(err))
326326
}
327327
fmt.Printf("Started Workflow Id: %s, run Id: %s\n", wid, resp.GetRunID())
328328
return nil
@@ -337,7 +337,7 @@ func startWorkflowHelper(c *cli.Context, shouldPrintProgress bool) error {
337337
resp, err := serviceClient.StartWorkflowExecution(tcCtx, startRequest)
338338

339339
if err != nil {
340-
return commoncli.Problem("Failed to run workflow.", err)
340+
return commoncli.Problem("Failed to run workflow.", replaceRetryPropertiesInErrorMessageWithRetryArguments(err))
341341
}
342342

343343
// print execution summary
@@ -366,6 +366,21 @@ func startWorkflowHelper(c *cli.Context, shouldPrintProgress bool) error {
366366
return startFn()
367367
}
368368

369+
func replaceRetryPropertiesInErrorMessageWithRetryArguments(err error) error {
370+
errMsg := err.Error()
371+
// This mapping is built based on the implementation of the function constructStartWorkflowRequest
372+
for requestField, cliFlag := range map[string]string{
373+
"InitialIntervalInSeconds": FlagRetryInterval,
374+
"BackoffCoefficient": FlagRetryBackoff,
375+
"MaximumIntervalInSeconds": FlagRetryMaxInterval,
376+
"MaximumAttempts": FlagRetryAttempts,
377+
"ExpirationIntervalInSeconds": FlagRetryExpiration,
378+
} {
379+
errMsg = strings.ReplaceAll(errMsg, requestField, cliFlag)
380+
}
381+
return errors.New(errMsg)
382+
}
383+
369384
func constructStartWorkflowRequest(c *cli.Context) (*types.StartWorkflowExecutionRequest, error) {
370385
domain, err := getRequiredOption(c, FlagDomain)
371386
if err != nil {

tools/cli/workflow_commands_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2088,6 +2088,74 @@ func Test_NewTest(t *testing.T) {
20882088

20892089
}
20902090

2091+
func Test_StartWorkflowHelper_RetryErrorMapping(t *testing.T) {
2092+
requiredArguments := []clitest.CliArgument{
2093+
clitest.StringArgument(FlagDomain, "test-domain"),
2094+
clitest.StringArgument(FlagTaskList, "test-tasklist"),
2095+
clitest.StringArgument(FlagWorkflowType, "test-workflow-type"),
2096+
clitest.StringArgument(FlagExecutionTimeout, "10"),
2097+
}
2098+
tests := []struct {
2099+
cliArguments []clitest.CliArgument
2100+
respondedErrorMessage string
2101+
expectedErrorMessage string
2102+
}{
2103+
{
2104+
append(requiredArguments, clitest.IntArgument(FlagRetryAttempts, 1), clitest.IntArgument(FlagRetryInterval, -1)),
2105+
"InitialIntervalInSeconds must be greater than 0 on retry policy.",
2106+
"retry_interval must be greater than 0 on retry policy.",
2107+
},
2108+
{
2109+
append(requiredArguments, clitest.IntArgument(FlagRetryAttempts, 1), clitest.IntArgument(FlagRetryBackoff, -1)),
2110+
"BackoffCoefficient cannot be less than 1 on retry policy.",
2111+
"retry_backoff cannot be less than 1 on retry policy.",
2112+
},
2113+
{
2114+
append(requiredArguments, clitest.IntArgument(FlagRetryAttempts, 1), clitest.IntArgument(FlagRetryMaxInterval, -1)),
2115+
"MaximumIntervalInSeconds cannot be less than 0 on retry policy.",
2116+
"retry_max_interval cannot be less than 0 on retry policy.",
2117+
},
2118+
{
2119+
append(requiredArguments, clitest.IntArgument(FlagRetryAttempts, 1), clitest.IntArgument(FlagRetryInterval, 2), clitest.IntArgument(FlagRetryMaxInterval, -1)),
2120+
"MaximumIntervalInSeconds cannot be less than InitialIntervalInSeconds on retry policy.",
2121+
"retry_max_interval cannot be less than retry_interval on retry policy.",
2122+
},
2123+
{
2124+
append(requiredArguments, clitest.IntArgument(FlagRetryAttempts, -1)),
2125+
"MaximumAttempts cannot be less than 0 on retry policy.",
2126+
"retry_attempts cannot be less than 0 on retry policy.",
2127+
},
2128+
{
2129+
append(requiredArguments, clitest.IntArgument(FlagRetryAttempts, 1), clitest.IntArgument(FlagRetryExpiration, -1)),
2130+
"ExpirationIntervalInSeconds cannot be less than 0 on retry policy.",
2131+
"retry_expiration cannot be less than 0 on retry policy.",
2132+
},
2133+
{
2134+
append(requiredArguments, clitest.IntArgument(FlagRetryAttempts, 0), clitest.IntArgument(FlagRetryBackoff, 0)),
2135+
"MaximumAttempts and ExpirationIntervalInSeconds are both 0. At least one of them must be specified.",
2136+
"retry_attempts and retry_expiration are both 0. At least one of them must be specified.",
2137+
},
2138+
}
2139+
2140+
for _, tc := range tests {
2141+
t.Run(tc.expectedErrorMessage, func(t *testing.T) {
2142+
ctrl := gomock.NewController(t)
2143+
2144+
mockServiceClient := frontend.NewMockClient(ctrl)
2145+
mockServiceClient.EXPECT().
2146+
StartWorkflowExecution(gomock.Any(), gomock.Any()).
2147+
Return(nil, errors.New(tc.respondedErrorMessage))
2148+
2149+
app := NewCliApp(&clientFactoryMock{serverFrontendClient: mockServiceClient})
2150+
2151+
ctx := clitest.NewCLIContext(t, app, tc.cliArguments...)
2152+
err := startWorkflowHelper(ctx, false)
2153+
assert.ErrorContains(t, err, tc.expectedErrorMessage)
2154+
2155+
})
2156+
}
2157+
}
2158+
20912159
func Test_ProcessSearchAttr(t *testing.T) {
20922160
app := NewCliApp(&clientFactoryMock{})
20932161
set := flag.NewFlagSet("test", 0)

0 commit comments

Comments
 (0)