💥 Standardize Nexus Operation Input Arg. Deserialization Failure#615
💥 Standardize Nexus Operation Input Arg. Deserialization Failure#615cretz merged 4 commits intotemporalio:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 392052069b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
LGTM, but will see if @jmaeagle99 wants to look/merge
| { | ||
| value = null; | ||
| } | ||
| var payload = await dataConverter.ToPayloadAsync(value).ConfigureAwait(false); |
There was a problem hiding this comment.
To confirm, we consider serialization failures to keep the logic of use app error or fall through to internal on conversion?
There was a problem hiding this comment.
Yes, that is consistent across all SDKs
| { | ||
| throw; | ||
| } | ||
| catch (Exception e) |
There was a problem hiding this comment.
Use the when keyword to exclude exception types e.g. catch (Exception e) when (e is not ApplicationFailureException).
There was a problem hiding this comment.
Ah neat, done!
a710d57 to f266d91 Compare
Standardize Nexus Operation Input Arg. Deserialization Failure. Previously dotnet was treating all failures as retryable which is different then all other core based SDKs. After consolation with the SDK team, the agreed behaviour is in core based SDKs failures from the payload converter are non retry able (BAD_REQUEST), and failures in the codec are retry able (INTERNAL) and
ApplicationFailureExceptionshould be respected.see also: temporalio/features#762
closes #614
Note
Medium Risk
Changes Nexus handler error classification and retry behavior, which can alter production failure/retry semantics for Nexus operations; scope is contained and backed by targeted tests.
Overview
Nexus operation input deserialization now explicitly runs
PayloadCodec.DecodeAsync(when configured) beforePayloadConverter.ToValue, and maps failures to standardizedHandlerExceptions: codec/decode errors become retryableINTERNAL, while converter/type/format errors become non-retryableBAD_REQUEST;ApplicationFailureExceptionis allowed to propagate unchanged.Tests were updated to assert workflows fail immediately for bad argument types (non-retryable), and new coverage was added to verify codec failures trigger retries while converter failures do not.
Written by Cursor Bugbot for commit 296cc1e. This will update automatically on new commits. Configure here.