Nexus Error Serialization Update#630
Conversation
| TaskContinuationOptions.ExecuteSynchronously | TaskContinuationOptions.OnlyOnRanToCompletion, | ||
| TaskScheduler.Current)).ConfigureAwait(false); | ||
|
|
||
| public async Task<OperationInfo> FetchInfoAsync(OperationFetchInfoContext context) => |
There was a problem hiding this comment.
This change is just from updating the Nexus RPC library
| <PackageReference Include="Google.Protobuf" Version="3.26.1" /> | ||
| <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="2.2.0" /> | ||
| <PackageReference Include="Microsoft.SourceLink.GitHub" Version="8.0.0" PrivateAssets="All" /> | ||
| <PackageReference Include="NexusRpc" Version="0.2.0" /> |
There was a problem hiding this comment.
NOTE: Need to release the Nexus SDK before merging
VegetarianOrc
left a comment
There was a problem hiding this comment.
Overall looks good to me! There's a bunch of references to the Python implementation that are sprinkled throughout docstrings or comments that I think we should remove. I commented on two but there were more.
A couple other minor suggestions/comments.
| // Simulate a round-trip: an ApplicationFailureInfo that was serialized to | ||
| // NexusRpc.Failure and is now being restored | ||
| var originalProto = new Failure() | ||
| { | ||
| Message = "Original error", | ||
| StackTrace = "at SomeMethod()", | ||
| ApplicationFailureInfo = new() | ||
| { | ||
| Type = "CustomError", | ||
| NonRetryable = true, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
I'm not sure what I think about this test. I feel like what it's testing is a reasonable assertion, but is it realistic that a HandlerError would have an OriginalFailure where the info type isn't NexusHandlerFailureInfo?
There was a problem hiding this comment.
I updated this test let me know what you think
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18c6aa2e32
ℹ️ 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".
dc140dd to
7c4ce26
Compare
Switch dotnet SDK to the new Nexus failure serialization format we are using in Go, Java, Python and soon TS.
Summary
matching Python's nexusrpc.HandlerError
Breaking Changes
Note
Medium Risk
Changes Nexus error/Failure serialization and exception types surfaced to callers (
HandlerExceptioninstead ofNexusHandlerFailureException), which can break error-handling code and affects cross-language interoperability and retry semantics.Overview
Switches Nexus error propagation to the newer cross-SDK format by treating
NexusRpc.Handlers.HandlerExceptionas the primary Nexus handler error type and round-tripping it through TemporalFailureprotos.DefaultFailureConverternow (1) serializes freshHandlerExceptions intoNexusHandlerFailureInfo, (2) deserializesNexusHandlerFailureInfointoHandlerException(including retry behavior) while preserving original Temporal failure structure via JSON proto round-tripping, and (3) adds support for non-Temporal Nexus failure payload fallbacks.NexusWorkernow returnsFailuredirectly (removing deprecatedError/OperationErrorpaths) and mapsOperationExceptionstates toCanceledFailureExceptionor a non-retryableApplicationFailureException("OperationError"), preserving cause chains; relatedIOperationHandlerFetchInfoAsync/FetchResultAsyncstubs are removed across hosting/worker/test helpers. Tests are updated/expanded accordingly, andNexusRpcis bumped to0.3.0.Written by Cursor Bugbot for commit 7c4ce26. This will update automatically on new commits. Configure here.