Conversation
|
|
||
| // Step 2: waitForCallback with retry — if the external system fails, try again with a fresh callback | ||
| var approvalResult = WithRetryHelper.withRetry( | ||
| context, |
There was a problem hiding this comment.
If withRetry requires a durable context parameter, why don't we put this method into DurableContext?
| * @return the operation result | ||
| */ | ||
| @SuppressWarnings("unchecked") | ||
| public static <T> T withRetry(DurableContext context, String name, WithRetry<T> operation, WithRetryConfig config) { |
There was a problem hiding this comment.
Where is async version withRetryAsync of it? withRetryAsync will return a DurableFuture and users can combine that with anyOf/allOf or just kick off an operation with retry in the background.
| context, | ||
| (ctx, attempt) -> ctx.waitForCallback( | ||
| "approval-" + attempt, String.class, (callbackId, stepCtx) -> stepCtx.getLogger() | ||
| .info("Attempt {}: sending callback {} to approval system", attempt, callbackId)), |
There was a problem hiding this comment.
Does this supplier have to contain only one operation? How do we ensure and validate this?
| * | ||
| * @return true if child-context wrapping is enabled | ||
| */ | ||
| public boolean wrapInChildContext() { |
There was a problem hiding this comment.
In which scenarios wrapInChildContext is mandate?
| */ | ||
| public WithRetryConfig build() { | ||
| if (retryStrategy == null) { | ||
| throw new IllegalArgumentException("retryStrategy is required"); |
There was a problem hiding this comment.
All existing configurations are optional. I think we should keep the convention for this new WithRetryConfig
|
|
||
| if (config.wrapInChildContext()) { | ||
| return (T) context.runInChildContext( | ||
| name, new TypeToken<Object>() {}, childCtx -> executeRetryLoop(childCtx, name, operation, config)); |
There was a problem hiding this comment.
We could have a new subtype for this child context.
3 participants
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Issue Link, if available
N/A
Description
Add a public
WithRetryhelper that retries any durable operation (waitForCallback,invoke,waitForCondition, etc.) end-to-end with configurable backoff. Today users have to hand-roll a while loop with manualcontext.wait()backoff calls. This helper provides a single, replay-safe, well-tested primitive for that pattern, reusing the exact sameRetryStrategyshape developers already know fromStepConfig.New files:
WithRetry<T>(sdk/.../util/) —@FunctionalInterfacethat takes(DurableContext, int attempt)and returnsT. Enables lambda syntax for the operation to retry.withRetryConfig(sdk/.../config/) — Builder-pattern config holding theRetryStrategy(required, same type asStepConfig) andwrapInChildContext(defaults totrue).withRetryHelper(sdk/.../util/) — Static utility with two overloads:retryOperation(context, name, operation, config)— wraps inrunInChildContextby default for clean execution history grouping.retryOperation(context, operation, config)— runs directly in the caller's context, no wrapping.Key behaviors:
RetryStrategy/RetryDecisiontypes — zero new retry concepts.operation.execute()calls durable primitives,context.wait()handles backoff).SuspendExecutionExceptionandUnrecoverableDurableExecutionExceptionwithout retrying — these are internal SDK control flow signals.RetryDecision.delay()isDuration.ZERO.Demo/Screenshots
N/A — utility library change, no UI.
Checklist
Testing
Unit Tests
Yes. 36 unit tests across 3 test classes in
sdk/src/test/:WithRetryConfigTest(8 tests) — builder validation, defaults, required fields, chaining.WithRetryHelperTest(24 tests in 3 nested classes):NamedForm(9) — child-context wrapping, opt-out, backoff wait naming, default delay, null validation.AnonymousForm(7) — no wrapping, anonymous backoff naming, exception identity, null validation.RetryBehavior(8) — attempt numbers, error forwarding, custom delays, context passthrough,SuspendExecutionExceptionpropagation,UnrecoverableDurableExecutionExceptionpropagation, retry exhaustion.RetryableOperationTest(4 tests) — lambda implementation, context/attempt passthrough, exception propagation, null return.Integration Tests
Yes. 12 integration tests across 2 test classes in
sdk-integration-tests/src/test/:RetryInvokeIntegrationTest(6 tests) — first-attempt success, retry after failure, all retries exhausted, custom backoff delays, composition with steps, original exception type preservation.RetryWaitForCallbackIntegrationTest(6 tests) — first-attempt success, retry after callback failure, all retries exhausted, composition with steps, multiple failures then success, submitter re-execution on each retry.Examples
Yes. 2 example handlers with 8 example tests in
examples/:RetryInvokeExample+RetryInvokeExampleTest(4 tests) — demonstrateswithRetrywrappingcontext.invokewith retry on transient failures.RetryWaitForCallbackExample+RetryWaitForCallbackExampleTest(4 tests) — demonstrateswithRetrywrappingcontext.waitForCallbackwith retry on callback rejection.