search

temporalio / money-transfer-project-template-dotnet

0 stars 1 forks source link

[Bug] Workflow catch blocks are never hit #2

Open GFoley83 opened 4 months ago

GFoley83 commented 4 months ago

What are you really trying to do?

Trying to test the catch logic in the workflow. For ex. it's not possible to trigger the refund logic contained in the workflow: https://github.com/temporalio/money-transfer-project-template-dotnet/blob/main/MoneyTransferWorker/Workflow.cs#L52-L55

Describe the bug

You can verify this by changing the TargetAccount to any other number e.g. 1 and running the client: https://github.com/temporalio/money-transfer-project-template-dotnet/blob/main/MoneyTransferClient/Program.cs#L12

However if you change ApplicationFailureException to a regular Exception, things work as expected, as can be seen in Temporal UI:

image

I can see there was some discussion related to this issue here.

GFoley83 commented 4 months ago

Just wondering if this is on anyone's radar @cretz? Given it's the only example linked in the Learn Temporal section for .NET & it's not currently working properly.

NitroEvil commented 3 months ago

I've spent the past hour trying to figure out why it never hits catch, glad to see it's not just me having this issue.

jsundai commented 3 months ago

Thanks for pointing this out. Will take a look at it!

NitroEvil commented 3 months ago

I had two issues relating to the catch block not being hit.

Based on the docs I think the wrong exception is being used? image

When making this change it still didn't work, it was only when I came across this guide here the MaximumAttempts needed to be set to 1 or smaller so on one failure the catch logic is executed.

I found the demo code by using https://temporal.io/setup/install-temporal-cli which I didn't notice the button to take you to the guide after you setup the enviroment.

GFoley83 commented 3 months ago

I covered all of this above: changing all ApplicationFailureException to regular Exception inside Workflow.cs makes everything run as intended. I'm just not sure if using a regular exception is the best/correct solution from a Temporal point of view.

I also linked to a discussion in the open PR about what the correct exception should be.

NitroEvil commented 3 months ago

I covered all of this above: changing all ApplicationFailureException to regular Exception makes the workflow run as intended. I'm just not sure if using a regular exception is the best/correct solution from a Temporal point of view.

I also linked to a discussion in the open PR about what the correct exception should be.

I changed the exception to ActivityFailureException which resolved my issue. I'd create a pr for the change but nore sure if they is a reason for using ApplicationFailureException

GFoley83 commented 1 month ago

Any update @jsundai