fix(common): preserve Error.cause in activity failures - #2292
fix(common): preserve Error.cause in activity failures#2292mohan-garimella wants to merge 3 commits into
Conversation
Keep native Error.cause chains when activity errors are converted to ApplicationFailure so the existing failure converter can serialize the underlying error.
|
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11e7a5e6e8
ℹ️ 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42e250e185
ℹ️ 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".
| const cause = isRecord(error) && error.cause instanceof Error ? error.cause : undefined; | ||
| const failure = ApplicationFailure.create({ message, type, nonRetryable: false, cause }); |
There was a problem hiding this comment.
Guard serialization against cyclic error causes
When an Activity throws an Error whose cause graph is cyclic (for example, error.cause = error, which is permitted because cause is mutable), retaining that cause makes DefaultFailureConverter.errorToFailureInner recurse indefinitely through optionalErrorToOptionalFailure. Instead of returning an Activity failure, the worker hits a RangeError while encoding it; detect cycles before attaching or serializing the cause.
Useful? React with 👍 / 👎.
What changed
Error.causewhenensureApplicationFailureconverts an Activity error.Why
Activity execution calls
ensureApplicationFailurebefore the configured failure converter. The default converter already serializesTemporalFailure.cause, but the conversion step discarded the cause first. Libraries such as database clients commonly useError.causefor the actionable underlying error, so Temporal history retained only the wrapper message.This is intentionally narrower than #1734. It does not map
AggregateError.errors, useApplicationFailure.details, or change the failure protobuf.Closes #2291
Verification
pnpm -F @temporalio/proto run buildpnpm -F @temporalio/common run buildpnpm -F @temporalio/common run test— 39 passed