Skip to content

fix(common): preserve Error.cause in activity failures - #2292

Open
mohan-garimella wants to merge 3 commits into
temporalio:mainfrom
mohan-garimella:codex/preserve-application-failure-cause
Open

fix(common): preserve Error.cause in activity failures#2292
mohan-garimella wants to merge 3 commits into
temporalio:mainfrom
mohan-garimella:codex/preserve-application-failure-cause

Conversation

@mohan-garimella

Copy link
Copy Markdown

What changed

  • Preserve a native Error.cause when ensureApplicationFailure converts an Activity error.
  • Add one focused common-package test covering conversion and serialization of the nested failure.

Why

Activity execution calls ensureApplicationFailure before the configured failure converter. The default converter already serializes TemporalFailure.cause, but the conversion step discarded the cause first. Libraries such as database clients commonly use Error.cause for the actionable underlying error, so Temporal history retained only the wrapper message.

This is intentionally narrower than #1734. It does not map AggregateError.errors, use ApplicationFailure.details, or change the failure protobuf.

Closes #2291

Verification

  • pnpm -F @temporalio/proto run build
  • pnpm -F @temporalio/common run build
  • pnpm -F @temporalio/common run test — 39 passed
  • Prettier check on changed files
  • ESLint on changed files

Keep native Error.cause chains when activity errors are converted to ApplicationFailure so the existing failure converter can serialize the underlying error.
@CLAassistant

CLAassistant commented Aug 4, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@mohan-garimella
mohan-garimella marked this pull request as ready for review August 4, 2026 22:46
@mohan-garimella
mohan-garimella requested a review from a team as a code owner August 4, 2026 22:46

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread packages/common/src/failure.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +439 to +440
const cause = isRecord(error) && error.cause instanceof Error ? error.cause : undefined;
const failure = ApplicationFailure.create({ message, type, nonRetryable: false, cause });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Activity errors lose Error.cause during conversion

2 participants