fix: preserve entity executor stack traces from protobuf setValue()#324
Open
YunchuWang wants to merge 1 commit into
Open
fix: preserve entity executor stack traces from protobuf setValue()#324YunchuWang wants to merge 1 commit into
YunchuWang wants to merge 1 commit into
Conversation
The entity executor's error handling code incorrectly used a chained call pattern (new StringValue().setValue(error.stack)) to set the stack trace on failure details. In google-protobuf, setValue() returns void, not the StringValue instance, so setStacktrace() received undefined — silently discarding the stack trace for all entity operation failures. Fix: construct the StringValue separately and pass it to setStacktrace(), matching the correct pattern used elsewhere in the codebase (pb-helper.util.ts). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes loss of stack traces in entity operation failure details by correcting how protobuf StringValue.setValue() is used in the entity executor. This improves debuggability of entity failures by ensuring the original error stack is preserved in TaskFailureDetails.
Changes:
- Fix
TaskEntityShimfailure handling to setTaskFailureDetails.stacktraceusing a properly constructedStringValue. - Add a regression test that asserts stack traces are present for
Error-based entity failures (and absent for non-Errorthrows).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/durabletask-js/src/worker/entity-executor.ts | Fixes protobuf wrapper misuse so captured error stack traces are actually stored in failure details. |
| packages/durabletask-js/test/entity-executor.spec.ts | Adds coverage to prevent regression of stack trace loss in entity failure details. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #248.
The entity executor misused the protobuf
setValue()return value, causing captured stack traces to be silently dropped from failure details. This preserves the original stack trace.Opens the existing fix branch
copilot-finds/bug/fix-entity-executor-stacktrace-lostas a PR (automated triage of active [copilot-finds] issues that had a ready fix branch but no open PR).