Skip to content

Commit

Permalink
feat: allow retry 0 directives (#2479)
Browse files Browse the repository at this point in the history
closes #2284
This allows for:
- fsm transitions to opt out of the default fsm retry policy
- having a catch verb without any retries

eg:
```
//ftl:retry 0
//ftl:retry 0 catch recover
//ftl:retry 0 1s catch recover
```

Not allowed because it doesn't make sense:
```
//ftl:retry 0 1s
```
  • Loading branch information
matt2e authored Aug 22, 2024
1 parent b20986b commit 637f20a
Show file tree
Hide file tree
Showing 4 changed files with 138 additions and 15 deletions.
18 changes: 11 additions & 7 deletions backend/schema/metadataretry.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,17 +109,21 @@ func (m *MetadataRetry) RetryParams() (RetryParams, error) {

// min backoff
if m.MinBackoff == "" {
return RetryParams{}, fmt.Errorf("retry must have a minimum backoff")
}
minBackoff, err := parseRetryDuration(m.MinBackoff)
if err != nil {
return RetryParams{}, fmt.Errorf("could not parse min backoff duration: %w", err)
if params.Count != 0 {
return RetryParams{}, fmt.Errorf("retry must have a minimum backoff")
}
params.MinBackoff = MinBackoffLimit
} else {
minBackoff, err := parseRetryDuration(m.MinBackoff)
if err != nil {
return RetryParams{}, fmt.Errorf("could not parse min backoff duration: %w", err)
}
params.MinBackoff = minBackoff
}
params.MinBackoff = minBackoff

// max backoff
if m.MaxBackoff == "" {
params.MaxBackoff = max(minBackoff, DefaultMaxBackoff)
params.MaxBackoff = max(params.MinBackoff, DefaultMaxBackoff)
} else {
maxBackoff, err := parseRetryDuration(m.MaxBackoff)
if err != nil {
Expand Down
125 changes: 121 additions & 4 deletions backend/schema/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ func TestParserRoundTrip(t *testing.T) {

//nolint:maintidx
func TestParsing(t *testing.T) {
zero := 0
ten := 10
tests := []struct {
name string
Expand Down Expand Up @@ -428,11 +429,23 @@ func TestParsing(t *testing.T) {
+retry 1h1m5s
verb C(Empty) Unit
+retry 0h0m5s 1h0m0s
fsm FSM {
verb D(Empty) Unit
+retry 0
verb E(Empty) Unit
+retry 0 1s catch test.catch
verb F(Empty) Unit
+retry 0 catch test.catch
verb catch(builtin.CatchRequest<Any>) Unit
fsm FSM
+ retry 10 1s 10s
{
start test.A
transition test.A to test.B
transition test.A to test.C
transition test.B to test.C
transition test.C to test.D
transition test.D to test.E
transition test.E to test.F
}
}
`,
Expand All @@ -442,6 +455,13 @@ func TestParsing(t *testing.T) {
Decls: []Decl{
&FSM{
Name: "FSM",
Metadata: []Metadata{
&MetadataRetry{
Count: &ten,
MinBackoff: "1s",
MaxBackoff: "10s",
},
},
Start: []*Ref{
{
Module: "test",
Expand All @@ -462,13 +482,43 @@ func TestParsing(t *testing.T) {
{
From: &Ref{
Module: "test",
Name: "A",
Name: "B",
},
To: &Ref{
Module: "test",
Name: "C",
},
},
{
From: &Ref{
Module: "test",
Name: "C",
},
To: &Ref{
Module: "test",
Name: "D",
},
},
{
From: &Ref{
Module: "test",
Name: "D",
},
To: &Ref{
Module: "test",
Name: "E",
},
},
{
From: &Ref{
Module: "test",
Name: "E",
},
To: &Ref{
Module: "test",
Name: "F",
},
},
},
},
&Verb{
Expand Down Expand Up @@ -525,6 +575,73 @@ func TestParsing(t *testing.T) {
},
},
},
&Verb{
Name: "D",
Request: &Ref{
Module: "builtin",
Name: "Empty",
},
Response: &Unit{
Unit: true,
},
Metadata: []Metadata{
&MetadataRetry{
Count: &zero,
},
},
},
&Verb{
Name: "E",
Request: &Ref{
Module: "builtin",
Name: "Empty",
},
Response: &Unit{
Unit: true,
},
Metadata: []Metadata{
&MetadataRetry{
Count: &zero,
MinBackoff: "1s",
Catch: &Ref{
Module: "test",
Name: "catch",
},
},
},
},
&Verb{
Name: "F",
Request: &Ref{
Module: "builtin",
Name: "Empty",
},
Response: &Unit{
Unit: true,
},
Metadata: []Metadata{
&MetadataRetry{
Count: &zero,
Catch: &Ref{
Module: "test",
Name: "catch",
},
},
},
},
&Verb{
Name: "catch",
Request: &Ref{
Module: "builtin",
Name: "CatchRequest",
TypeParameters: []Type{
&Any{},
},
},
Response: &Unit{
Unit: true,
},
},
},
}},
},
Expand Down
7 changes: 5 additions & 2 deletions backend/schema/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -877,8 +877,8 @@ func validateVerbSubscriptions(module *Module, v *Verb, md *MetadataSubscriber,

func validateRetries(module *Module, retry *MetadataRetry, requestType optional.Option[Type], scopes Scopes, schema optional.Option[*Schema]) (merr []error) {
// Validate count
if retry.Count != nil && *retry.Count <= 0 {
merr = append(merr, errorf(retry, "retry count must be at least 1"))
if retry.Count != nil && *retry.Count < 0 {
merr = append(merr, errorf(retry, "retry count can not be negative"))
}

// Validate parsing of durations
Expand All @@ -893,6 +893,9 @@ func validateRetries(module *Module, retry *MetadataRetry, requestType optional.

// validate catch
if retry.Catch == nil {
if retryParams.Count == 0 && retry.MinBackoff != "" {
merr = append(merr, errorf(retry, "can not define a backoff duration when retry count is 0 and no catch is declared"))
}
return
}
req, ok := requestType.Get()
Expand Down
3 changes: 1 addition & 2 deletions backend/schema/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,9 +380,8 @@ func TestValidate(t *testing.T) {
`19:7-7: could not parse min backoff duration: could not parse retry duration: duration has unknown unit "mins" - use 'd', 'h', 'm' or 's', eg '1d' or '30s'`,
`21:7-7: max backoff duration (1s) needs to be at least as long as initial backoff (1m)`,
`23:7-7: could not parse min backoff duration: retry backoff can not be larger than 1d`,
`25:7-7: retry count must be at least 1`,
`25:7-7: can not define a backoff duration when retry count is 0 and no catch is declared`,
`30:7-7: catch can only be defined on verbs`,
`30:7-7: retry count must be at least 1`,
`4:7-7: could not parse min backoff duration: could not parse retry duration: duration has unit "m" out of order - units need to be ordered from largest to smallest - eg '1d3h2m'`,
`6:7-7: could not parse min backoff duration: could not parse retry duration: duration has unit "d" out of order - units need to be ordered from largest to smallest - eg '1d3h2m'`,
`8:7-7: could not parse min backoff duration: retry must have a minimum backoff of 1s`,
Expand Down

0 comments on commit 637f20a

Please sign in to comment.