Simplify retry policy typing: retry_policy(http.default_retry)

CHANGELOG.md

@@ -1,5 +1,11 @@
 <unreleased>
 
+Breaking changes:
+
+- retry_policy() takes a plain policy function name instead of a {policy: ...} object
+
+Fixes:
+
 - The try statement support a finally block
 - Support empty body in a catch block
 - retry_policy() may now include expressions in addition to number literals

language_reference.md

@@ -612,9 +612,28 @@ If an exception gets thrown inside a try block, the stack trace in Workflows log
 
 It is possible to set a retry policy for a try-catch statement. Because Typescript does not have `retry` keyword, the retry is implemented by a special `retry_policy` function. It must be called immediately after a try-catch block. A call to the `retry_policy` is ignored elsewhere.
 
-The `retry_policy` function must be called with parameters defining a retry policy. It can be either a policy provided by GCP Workflows or a custom retry policy. See the GCP documentation for the [required parameters for the two policy types](https://cloud.google.com/workflows/docs/reference/syntax/retrying#try-retry).
+Finally and catch blocks are run after possible retry attempts. The following sample retries `http.get()` if it throws an exception and executes `sys.log('Error!')` and `closeConnection()` after retry attempts.
+
+```javascript
+import { http, retry_policy, sys } from 'ts2workflows/types/workflowslib'
+
+function main() {
+  try {
+    http.get('https://visit.dreamland.test/')
+  } catch (err) {
+    sys.log('Error!')
+  } finally {
+    closeConnection()
+  }
+  retry_policy(http.default_retry)
+}
+```
+
+The `retry_policy` function must be called with a parameter that defines the retry policy. It can be either a policy provided by GCP Workflows or a custom retry policy.
 
-A sample with a GCP-provided retry policy:
+### GCP-provided retry policy
+
+GCP retry policy must be either `http.default_retry` or `http.default_retry_non_idempotent`. Their effects are described by the [GCP documentation](https://cloud.google.com/workflows/docs/reference/syntax/retrying#default-retry-policy).
 
 ```javascript
 import { http, retry_policy } from 'ts2workflows/types/workflowslib'
@@ -625,11 +644,15 @@ function main() {
   } catch (err) {
     return 'Error!'
   }
-  retry_policy({ policy: http.default_retry })
+  retry_policy(http.default_retry)
 }
 ```
 
-A sample with a custom retry policy:
+### Custom retry policy
+
+A custom retry policy is an object with the properties shown in the following example. See the GCP documentation for the [explanation of the properties](https://cloud.google.com/workflows/docs/reference/syntax/retrying#try-retry).
+
+The parameter must be a literal map object (not a variable). The values may be literals or expressions.
 
 ```javascript
 import { http, retry_policy } from 'ts2workflows/types/workflowslib'
@@ -678,23 +701,6 @@ main:
                 return: Error!
 ```
 
-Finally and catch blocks are run after possible retry attempts. The following sample retries `http.get()` if it throws an exception and executes `sys.log('Error!')` and `closeConnection()` after retry attempts.
-
-```javascript
-import { http, retry_policy, sys } from 'ts2workflows/types/workflowslib'
-
-function main() {
-  try {
-    http.get('https://visit.dreamland.test/')
-  } catch (err) {
-    sys.log('Error!')
-  } finally {
-    closeConnection()
-  }
-  retry_policy({ policy: http.default_retry })
-}
-```
-
 ## Throwing errors
 
 The statement
@@ -799,9 +805,7 @@ The `parallel` function executes code blocks in parallel (using [parallel step](
 ```typescript
 function retry_policy(
   params:
-    | {
-        policy: (exception: unknown) => void
-      }
+    | ((exception: unknown) => void)
     | {
         predicate: (exception: unknown) => boolean
         max_retries: number

src/transpiler/expressions.ts

@@ -64,9 +64,7 @@ export function convertObjectAsExpressionValues(
   node: TSESTree.ObjectExpression,
 ): Record<string, Expression> {
   // Convert Primitive values to PrimitiveExpressions
-  return mapRecordValues(convertObjectExpression(node), (val) =>
-    isExpression(val) ? val : new PrimitiveExpression(val),
-  )
+  return mapRecordValues(convertObjectExpression(node), asExpression)
 }
 
 function convertExpressionOrPrimitive(

src/transpiler/statements.ts

@@ -22,6 +22,7 @@ import {
   BinaryOperator,
   Expression,
   FunctionInvocationExpression,
+  Primitive,
   PrimitiveExpression,
   VariableName,
   VariableReferenceExpression,
@@ -30,7 +31,7 @@ import {
   isLiteral,
 } from '../ast/expressions.js'
 import { InternalTranspilingError, WorkflowSyntaxError } from '../errors.js'
-import { flatMapPair, isRecord } from '../utils.js'
+import { flatMapPair, isRecord, mapRecordValues } from '../utils.js'
 import { transformAST } from './transformations.js'
 import {
   convertExpression,
@@ -1016,49 +1017,30 @@ function parseRetryPolicy(
     return undefined
   }
 
-  const argumentsNode = node.arguments
-  const argsLoc = argumentsNode[0].loc
-  if (
-    argumentsNode.length < 1 ||
-    argumentsNode[0].type !== AST_NODE_TYPES.ObjectExpression
-  ) {
-    throw new WorkflowSyntaxError(
-      'Expected an object literal with "policy" or all of the following properties: "predicate", "max_retries", "backoff"',
-      argsLoc,
-    )
+  if (node.arguments.length < 1) {
+    throw new WorkflowSyntaxError('Required argument missing', node.loc)
   }
 
-  const workflowArguments = convertObjectAsExpressionValues(argumentsNode[0])
-
-  if ('policy' in workflowArguments) {
-    return retryPolicyFromFunctionName(workflowArguments.policy, argsLoc)
-  } else {
-    return retryPolicyFromParams(workflowArguments, argsLoc)
-  }
-}
+  const arg0 = throwIfSpread(node.arguments).map(convertExpression)[0]
+  const argsLoc = node.arguments[0].loc
 
-function retryPolicyFromFunctionName(
-  policyEx: Expression,
-  argsLoc: TSESTree.SourceLocation,
-): string {
-  if (isFullyQualifiedName(policyEx)) {
-    return policyEx.toString()
+  if (isFullyQualifiedName(arg0)) {
+    return arg0.toString()
+  } else if (arg0.expressionType === 'primitive' && isRecord(arg0.value)) {
+    return retryPolicyFromParams(arg0.value, argsLoc)
   } else {
-    throw new WorkflowSyntaxError('"policy" must be a function name', argsLoc)
+    throw new WorkflowSyntaxError('Unexpected type', argsLoc)
   }
 }
 
 function retryPolicyFromParams(
-  workflowArguments: Record<string, Expression>,
+  paramsObject: Record<string, Primitive | Expression>,
   argsLoc: TSESTree.SourceLocation,
 ): CustomRetryPolicy {
-  if (
-    'predicate' in workflowArguments &&
-    'max_retries' in workflowArguments &&
-    'backoff' in workflowArguments
-  ) {
+  const params = mapRecordValues(paramsObject, asExpression)
+  if ('predicate' in params && 'max_retries' in params && 'backoff' in params) {
     let predicate = ''
-    const predicateEx = workflowArguments.predicate
+    const predicateEx = params.predicate
 
     if (isFullyQualifiedName(predicateEx)) {
       predicate = predicateEx.toString()
@@ -1069,7 +1051,7 @@ function retryPolicyFromParams(
       )
     }
 
-    const backoffEx = workflowArguments.backoff
+    const backoffEx = params.backoff
 
     if (backoffEx.expressionType === 'primitive' && isRecord(backoffEx.value)) {
       const backoffLit = backoffEx.value
@@ -1081,7 +1063,7 @@ function retryPolicyFromParams(
       ) {
         return {
           predicate,
-          maxRetries: workflowArguments.max_retries,
+          maxRetries: params.max_retries,
           backoff: {
             initialDelay: asExpression(backoffLit.initial_delay),
             maxDelay: asExpression(backoffLit.max_delay),

test/transpiler.spec.ts

@@ -2043,7 +2043,7 @@ describe('Try-catch-finally statement', () => {
       } catch {
         log("Error!");
       }
-      retry_policy({ policy: http.default_retry })
+      retry_policy(http.default_retry)
     }`
 
     const expected = `
@@ -2081,7 +2081,7 @@ describe('Try-catch-finally statement', () => {
       } finally {
         closeConnection();
       }
-      retry_policy({ policy: http.default_retry })
+      retry_policy(http.default_retry)
     }`
 
     const expected = `
@@ -2389,7 +2389,7 @@ describe('Try-catch-finally statement', () => {
       } catch {
         log("Error!");
       }
-      retry_policy({ policy: 1000 })
+      retry_policy(1000)
     }`
 
     expect(() => transpile(code)).to.throw()
@@ -2406,7 +2406,7 @@ describe('Try-catch-finally statement', () => {
 
       log("try block completed")
 
-      retry_policy({ policy: http.default_retry })
+      retry_policy(http.default_retry)
     }`
 
     const expected = `
@@ -2485,7 +2485,7 @@ describe('Try-catch-finally statement', () => {
       } catch {
         log("Error!");
       }
-      const x = 1 + retry_policy({ policy: http.default_retry })
+      const x = 1 + retry_policy(http.default_retry)
     }`
 
     expect(() => transpile(code)).to.throw()

types/workflowslib.d.ts

@@ -709,9 +709,7 @@ export declare function parallel(
 
 export declare function retry_policy(
   params:
-    | {
-        policy: (exception: unknown) => void
-      }
+    | ((exception: unknown) => void)
     | {
         predicate: (exception: unknown) => boolean
         max_retries: number