temporalio / sdk-go

Temporal Go SDK
https://docs.temporal.io/application-development?lang=go
MIT License
481 stars 197 forks source link

Stop checking that context is cancelled when a `CanceledError` is received #1527

Closed dmateusp closed 2 weeks ago

dmateusp commented 2 weeks ago

Is your feature request related to a problem? Please describe.

I have an activity that checks some external condition, and doesn't run if that external condition is not fulfilled.

In that scenario I would like to see it as cancelled in Temporal, so I tried to return temporal.NewCanceledError(...) from within the activity.

However, temporal is currently wrapping my error in fmt.Errorf("unexpected activity cancel error: %w", err), and I think that's happening here. This actually causes my error to be retried as any other error would.

I think this is happening because I'm trying to return that cancel error from an activity directly instead of having a workflow initiate an activity context cancel, see the check here

Describe the solution you'd like

I would like temporal to stop checking that the context was cancelled when handling CanceledError

Describe alternatives you've considered

Right now I'm going to have to return a non-retryable error, but this stop scenario is not technically an error.

cretz commented 2 weeks ago

In that scenario I would like to see it as cancelled in Temporal

Activity cancellation can only result from the server issuing a cancellation, an activity cannot appear cancelled just by returning cancelled. This is Temporal platform/server behavior and not Go SDK (it will reject SDK attempts to mark activities as cancelled if a cancellation wasn't requested server side, which is why SDK must convert it to a normal error).

dmateusp commented 2 weeks ago

thanks @cretz for the explanation, I'll close this issue and keep using a non-retryable error