Standardize Nexus Operation Input Arg. Deserialization Failure#1949
Standardize Nexus Operation Input Arg. Deserialization Failure#1949Quinn-With-Two-Ns merged 5 commits intotemporalio:mainfrom
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| }); | ||
|
|
||
| t.true(decodeCount >= 2, `Expected decode count >= 2, got ${decodeCount}`); | ||
| }); |
There was a problem hiding this comment.
Codec retry test may hit wrong decode
Low Severity
failingCodec is installed on both the Worker and the Client, so the first decode failure can occur while decoding workflow inputs/other payloads (before any Nexus operation input is decoded). This can make the test pass without exercising the intended Nexus operation codec failure path, and can also make it flaky depending on decode ordering.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0cac77d95
ℹ️ 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".
| if (decodeCount === 1) { | ||
| throw new Error('Intentional codec decode failure'); |
There was a problem hiding this comment.
This injection throws on the first decode call globally, but the same codec is also configured on the worker (dataConverter.payloadCodecs) where it is used for workflow activation decoding (WorkflowCodecRunner.decodeActivation) before the Nexus handler runs. That means the initial failure can be consumed outside the Nexus operation path, and decodeCount >= 2 can still pass even if Nexus input deserialization is never retried, so this test can miss regressions in the behavior this commit is trying to validate.
Useful? React with 👍 / 👎.
Quinn-With-Two-Ns
force-pushed
the
issue-1948
branch
from
fb2d86e to
51e6ff2
Compare
Quinn-With-Two-Ns
force-pushed
the
issue-1948
branch
from
51e6ff2 to
608ad70
Compare
2 participants
Standardize Nexus Operation Input Arg. Deserialization Failure. Previously typescript was treating all failures as non-retryable. 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 temporalio/sdk-dotnet#614
Note
Medium Risk
Changes Nexus operation input decoding error classification and retryability, which can alter production retry behavior and error surfaces for callers. Scope is contained and covered by new integration tests, but impacts a core request path.
Overview
Standardizes Nexus operation input deserialization error handling so payload codec decode failures are surfaced as
INTERNALnexus.HandlerErrors (with underlying cause attached) to allow retries, while payload converter failures becomeBAD_REQUESThandler errors and remain non-retryable;ApplicationFailureis now passed through without being wrapped.Adds integration coverage to verify codec failures are retried and converter failures are not (including a new intentionally failing payload converter), and slightly relaxes an existing JSON parse error assertion in
test-nexus-handler.ts.Written by Cursor Bugbot for commit 608ad70. This will update automatically on new commits. Configure here.