Workflow.retry that takes a function to decide if retry is needed

[mfateev]

Is your feature request related to a problem? Please describe.
Workflow.retry RetryOptions.doNotRetry is a list of strings. So any exception is converted to a string before matching. In many scenarios, the retry is based on a specific information in a specific exception down the chain or even on some other information within the workflow body.

Describe the solution you'd like
An API that takes a function (or lambda) that can execute random user provided logic to decide if retry is needed.

Additional context
This only works for retries from the workflow code. The service side retries have to rely on string matching in the current architecture. An activity already can decide itself that the error is not retryable by throwing an non-retryable ApplicationFailure.

[niegus]

Is there an ETA about when this could be tackled? :)

[Spikhalskiy]

@niegus We typically don't provide ETAs for feature requests. Contributions are welcomed, this is a great starter issue.

[niegus]

How do you envision this feature? Workflow.retry retries based on the expiration parameter, plus the maximum attempts configured in the RetryOptions, always if the exception is not set in the list of doNotRetry exceptions.

If there is a new function/lambda to decide if the retry is need, how it would work with the current behavior? Taking precedence over them? Validate on the creation of the RetryOption so there can not be contradictions between the "retryOn" and "doNotRetry"?

[niegus]

cc: @Spikhalskiy @mfateev

[Spikhalskiy]

I think to have it consistent, it should be doNotRetryPedicate and retry doesn't happen if ex.getErrorType in retryOptions.getDoNotRetry() || doNotRetryPedicate(ex).

But I'm getting back the comment about the great starter issue.
I thought about a bit different place in the system (workflow method exceptions and failures).
Workflow.retry works with persisted side effects and it may get tricky. But if you want to give it a shot, the effort is very welcomed.

[niegus]

@Spikhalskiy This feature request comes from this thread: https://community.temporal.io/t/temporal-queue-activities/3095/8 to retry in certain cases, doNotRetryPredicate() could do the trick, but it would require to write negative code 😅

Eg.

doNotRetryPredicate(ex -> !(ex.getCause() instanceof TimeoutFailure && 
               ((TimeoutFailure) ex.getCause()).getTimeoutType() == TimeoutType.TIMEOUT_TYPE_SCHEDULE_TO_START))

IMHO (as a customer of this lib) it would be easier:

retryOnPredicate(ex -> ex.getCause() instanceof TimeoutFailure && 
               ((TimeoutFailure) ex.getCause()).getTimeoutType() == TimeoutType.TIMEOUT_TYPE_SCHEDULE_TO_START)

WDYT? Would that make sense?