Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR adds support for exception properties and inner failure details to the FailureDetails class, allowing richer error information to be captured and propagated through the Durable Task framework. The PR also includes a protobuf file sync from the upstream repository, which introduces many additional protocol buffer changes beyond the core feature.
Changes:
- Added
propertiesfield toTaskFailureDetailsprotobuf message to store exception metadata - Added
innerFailurefield support to capture exception cause chains - Implemented bidirectional conversion between protobuf
Valuetypes and Java objects - Added comprehensive unit tests covering properties, inner failures, and round-trip serialization
- Updated
DurableTaskGrpcWorkerto use the newFailureDetailsconstructor for consistent exception handling - Synced protobuf definitions from upstream repository (commit hash 1caadbd7ecfdf5f2309acbeac28a3e36d16aa156)
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/durabletask-protobuf/protos/orchestrator_service.proto | Added properties field to TaskFailureDetails; synced many other protobuf changes from upstream |
| internal/durabletask-protobuf/PROTO_SOURCE_COMMIT_HASH | Updated to track new protobuf source commit |
| client/src/main/java/com/microsoft/durabletask/FailureDetails.java | Added properties and innerFailure fields with conversion methods; added toString() override |
| client/src/test/java/com/microsoft/durabletask/FailureDetailsTest.java | Comprehensive new test file covering all aspects of the new functionality |
| client/src/main/java/com/microsoft/durabletask/DurableTaskGrpcWorker.java | Refactored to use FailureDetails constructor instead of building proto directly |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1 +1 @@ | |||
| fbe5bb20835678099fc51a44993ed9b045dee5a6 No newline at end of file | |||
| 1caadbd7ecfdf5f2309acbeac28a3e36d16aa156 No newline at end of file | |||
There was a problem hiding this comment.
The PR description contains placeholder text "resolves #issue_for_this_pr" instead of an actual issue number. The checklist also indicates that changes should be added to CHANGELOG.md, but no changes to CHANGELOG.md are included in this PR. Please update the PR description with the actual issue number and add an entry to the "Unreleased" section of CHANGELOG.md documenting this feature addition.
| public boolean isCausedBy(Class<? extends Exception> exceptionClass) { | ||
| String actualClassName = this.getErrorType(); | ||
| try { | ||
| // Try using reflection to load the failure's class type and see if it's a subtype of the specified | ||
| // exception. For example, this should always succeed if exceptionClass is System.Exception. | ||
| Class<?> actualExceptionClass = Class.forName(actualClassName); | ||
| return exceptionClass.isAssignableFrom(actualExceptionClass); | ||
| } catch (ClassNotFoundException ex) { | ||
| // Can't load the class and thus can't tell if it's related | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
The new innerFailure field is not tested with the isCausedBy method. It's unclear whether isCausedBy should only check the current failure or should traverse the innerFailure chain (similar to how one might need to check both an exception and its causes). Consider adding tests to clarify and document this behavior, or update the isCausedBy method to traverse the chain if that's the intended behavior.
| * @return an unmodifiable map of property names to values, or {@code null} | ||
| */ | ||
| @Nullable | ||
| public Map<String, Object> getProperties() { |
Adds support for inner failure details and custom exception properties in the Java SDK, matching the .NET SDK implementation (durabletask-dotnet#474, azure-functions-durable-extension#3215).