Skip to content

fix: refund error handling - #17

Open
ipierago wants to merge 1 commit into
temporalio:mainfrom
ipierago:fix/refund-error-handling
Open

fix: refund error handling#17
ipierago wants to merge 1 commit into
temporalio:mainfrom
ipierago:fix/refund-error-handling

Conversation

@ipierago

Copy link
Copy Markdown

What was changed

The catch clause for errors in the refund activity was moved.

Why?

When the refund activity executes successfully, it throws an error to indicate that the workflow has failed but that the funds were refunded. However, this error is caught by another catch clause and subsequently rethrown with additional error information. This second catch clause is intended to catch errors in the refund activity, but it is in the wrong place.

The result is that if the refund succeeds, the workflow result has the confusing message that the refund both failed and succeeded.

Checklist

How was this tested:
Follow the instructions in the temporarl documentation on how to simulate a failed deposit.

  1. replace the BankService deposit call with depositThatFails in activities.ts
  2. modify the maximumAttempts to 3 in workflows.ts
  3. npm run client
    The resulting error message is that the deposit failed but the refund was successfully executed.

@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.

@tomwheeler

Copy link
Copy Markdown
Contributor

Hi @ipierago. Thank you for your contribution. I evaluated the PR and agree with the solution. If you could sign our Contributor License Agreement, I will be able to merge this.

@tomwheeler
tomwheeler self-requested a review November 6, 2025 02:37
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.

3 participants