Repository navigation
fix(sdk): checkpoint the wrapped error for failed contexts - #816
ParidelPooya wants to merge 2 commits into
Conversation
A context that failed because an SDK error escaped it checkpointed the inner operation's raw ErrorObject. For a timed-out callback or invoke that object is null, and for a stopped invoke it is the stop error (for example States.Stopped), so the context checkpoint lost which error failed it. The context now checkpoints the wrapped error: the original ErrorObject of a wrapper such as StepFailedException when it has a type, otherwise an ErrorObject typed with the SDK error's class name (CallbackTimeoutException, InvokeTimedOutException, InvokeStoppedException, or the exception class when the error has no type), keeping message, data and stack trace. SDK operation exception types are not rebuilt from error data on translation. waitForCallback keeps the callback's own ErrorObject, because it rethrows the callback's exception live.
| errorObject = | ||
| getSubType() == OperationSubType.WAIT_FOR_CALLBACK ? opEx.getErrorObject() : wrappedError(opEx); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
Only an SDK error that names the failure itself (timed-out callback, timed-out or stopped invoke) and a CallbackFailedException or InvokeFailedException whose error has no type are checkpointed under their class name. Any other DurableOperationException that escapes a context, such as one a user throws with an empty or missing error object, keeps that object, as the OpenTelemetry plugin's VirtualOperationErrorStatusTest requires.
| errorObject = | ||
| getSubType() == OperationSubType.WAIT_FOR_CALLBACK ? opEx.getErrorObject() : wrappedError(opEx); |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_kxt4ac5ayulxw4xmu4ktdps4xu
[P2] Preserve callback error identity through enclosing contexts
When a virtual child context calls waitForCallback and the callback times out, the enclosing context now throws ChildContextFailedException with an error object typed CallbackTimeoutException. If the handler catches this and waits, replay prunes the completed wait's children. The wait then throws ChildContextFailedException with a null error object, which wrappedError preserves, so the enclosing failure changes from a typed error to null. Previously, both runs exposed null. This can change error-dependent branches and durable operation sequences during replay. Extend the preservation rule to failures escaping waitForCallback into enclosing contexts, using the persisted wait context's error consistently, and add a virtual-child live/pruned-replay regression test.
To suppress repeats, reply /ai verdict deferred, /ai verdict false-positive, or /ai verdict out-of-scope on the first nonblank line. Add rationale on later lines. A new /ai verdict accepted allows future repeats again.
Codex AI reviewOne P2 replay-consistency regression remains at Reviewed commit |
|
Closing in favour of a simpler approach. We no longer try to make |
Problem
When an SDK operation exception escapes a child context, map item or parallel branch,
ChildContextOperation.handleChildContextFailurecheckpoints the inner operation's rawErrorObject. That is right for a wrapper such asStepFailedException, whose object is the user's original error, but it loses identity for SDK errors that name the failure themselves:runInChildContextErrorTypebeforeStates.StoppedInvokeStoppedExceptionStates.Stoppedcom.acme.PaymentErrorInvokeStoppedExceptioncom.acme.PaymentErrorInvokeTimedOutExceptionCallbackTimeoutExceptionCallbackFailedExceptionnulltypeSo a reader of the context checkpoint (telemetry exporters,
MapResultconsumers, replay) cannot tell what failed the context. Telemetry spec v3 ("Core SDK requirement") requires every failed context to checkpoint the wrapped error, not the wrapper.Change
ChildContextOperation.wrappedErrorpicks the error to checkpoint:CallbackTimeoutException,InvokeTimedOutException,InvokeStoppedException, and aCallbackFailedExceptionorInvokeFailedExceptionwhose error has no (or a blank) type: anErrorObjectwithErrorType= the exception's class name, keeping the original message,ErrorDataand stack trace when present. For these the raw object is missing or is the stop error, so it does not say what failed the context.StepFailedExceptionor a typed callback/invoke failure, a nestedChildContextFailedException, or aDurableOperationExceptionthrown by user code, even with an empty or missing error object): itsErrorObject, unchanged, as before.waitForCallbackcontexts keep the callback's ownErrorObjectas before. Live,waitForCallbackrethrows the callback's exception (whose error object is the callback's), and a replay of the completed, pruned context can read only the context checkpoint, so the two must hold the same object.translateExceptionno longer tries to rebuild an SDK operation exception from error data. It falls through toChildContextFailedException/MapIterationFailedException/ParallelBranchFailedExceptionas before.User-visible changes
ContextDetails.error) of a context failed by a stopped or timed-out invoke, a timed-out callback, or a typeless callback/invoke failure now carryErrorType= the SDK exception class name (for examplesoftware.amazon.lambda.durable.exception.InvokeStoppedException) instead of the stop error's type,null, or no error.ChildContextFailedExceptionthrown for those contexts carries thatErrorObject, so its message isChild context failed with error of type ...InvokeStoppedException. Message: ...instead of... of type States.Stopped ...orChild context failed without an error. Its class is unchanged, live and replayed.ChildContextFailedExceptioncarries thatErrorObject.MapResult.getError(i)for such an item is unchanged (it still reports the thrownMapIterationFailedException). Only the iteration's checkpoint changes.waitForCallback.Compatibility
ErrorTypevalues. They are SDK class names thattranslateExceptionnow skips, so replay does not try to deserialize an SDK exception.MapResultand the public API are unchanged.Tests
ChildContextWrappedErrorIntegrationTest(LocalDurableTestRunner), each scenario live and replayed (the handler catches, waits for a new invocation, and the replay throws again):PaymentError; invoke failed with an external type; invoke stopped (States.Stopped, user type); invoke timed out; callback failed (typed, no type,"Error"); callback timed out. Live and replayed failures have the same class and error object.outer>child> invoke) stopped and timed out: both contexts checkpoint the SDK error.waitForCallback: the context keeps the callback's own error object.mvn -B installforsdk,sdk-testing,sdk-integration-testsandotel-pluginon JDK 17: 1589 / 53 / 741 / 916 tests passed, 0 failures (5 skipped, all already present).spotless:checkpassed. The existingPluginErrorContainerCompatibilityTest,WaitForCallbackPrunedReplayTestand otelVirtualOperationErrorStatusTestpass unchanged. The second commit narrows the first one so these pass.Known, not changed here: after the backend prunes a completed
waitForCallbackcontext's children, its replay throwsChildContextFailedExceptionwhile the live run throwsCallbackFailedException/CallbackTimeoutException. This was already the case, andWaitForCallbackPrunedReplayTestasserts it. The error object is the same in both runs.