[Bug] Activity errors lose Error.cause during conversion

[mohan-garimella]

What are you really trying to do?

Preserve the native Error.cause chain when an Activity throws a non-ApplicationFailure error.

The Activity worker first calls ensureApplicationFailure(error), then sends the result through the configured failure converter. DefaultFailureConverter already serializes TemporalFailure.cause, but ensureApplicationFailure currently copies only the error's message, type, and stack. As a result, the cause is lost before serialization.

This affects errors from libraries that wrap their underlying error with Error.cause. For example, a database wrapper error may retain the SQL query in its own message while the actionable PostgreSQL error exists only in cause.

Minimal reproduction

export async function activity(): Promise<void> {
  const databaseError = new Error('connection terminated unexpectedly');
  throw new Error('query failed', { cause: databaseError });
}

The resulting Activity failure contains the outer ApplicationFailure("query failed"), but not the nested "connection terminated unexpectedly" failure.

Expected behavior

When converting a non-ApplicationFailure Error, ensureApplicationFailure should preserve error.cause when it is an Error. The existing failure converter will then serialize the nested failure using the standard Temporal failure cause field.

This is intentionally narrower than #1734: it does not map AggregateError.errors, repurpose details, or change protobuf schemas.

[ethanresnick]

I'd really benefit from this, for being able to move some error metadata more reliably across the workflow boundary. But I think the logic here is not quite right. I.e., instead of doing ApplicationFailure.create({ message: err.message, /* ... */, cause err.cause }), shouldn't the original error be the cause of the application failure — i.e., ApplicationFailure.create({cause: err, /* ... */ }).

[ethanresnick]

That change might have to ship behind a flag for backwards compatibility, but it seems clearly right.

Today, if my activity does throw new SomeCustomError(...), any instance properties are totally invisible in the failure converter (e.g., for putting into details), because the original error never makes it to the failure converter.

[THardy98]

Hey @ethanresnick!

I'll be taking this on this/next week. The proposed PR looks fine, but will take a closer look at the failure/error conversion semantics to see if perhaps we can close off #1734 as well.