temporalio / sdk-core

Core Temporal SDK that can be used as a base for language specific Temporal SDKs
MIT License
266 stars 71 forks source link

[Bug] Very long `Duration` values prevent activities from being run #635

Closed iand675 closed 7 months ago

iand675 commented 10 months ago

What are you really trying to do?

At Mercury Technologies we are developing / maintaining a Haskell SDK that uses the FFI to use sdk-core.

I had a Workflow where I attempted to schedule an Activity with a StartToClose timeout set to be Duration maxBound maxBound to be effectively an inifinite timeout. I know this isn't best recommended, but it was just for testing purposes. The Workflow would time out and be evicted, because something upstream of my Haskell SDK was rejecting/unable to handle values that large.

Describe the bug

Invoke an Activity from a Workflow using a duration of > 2^32 seconds for the start-to-close timeout. The activity will never be recorded in the event log for the Workflow, and the Workflow will timeout and be evicted. I have no reason to believe that this behavior is specific to the Haskell SDK.

Minimal Reproduction

Broken:

https://github.com/MercuryTechnologies/hs-temporal-sdk/commit/f1e13fda15c2e469e00ae26fbb7dcee0c9ceab5e

Fixed by changing the value of "infinity":

https://github.com/MercuryTechnologies/hs-temporal-sdk/commit/89ac5f4aa4f8d44b0571041cf6029cdbd8323b60

Environment/Versions

Additional context

Slack conversation

Sushisource commented 7 months ago

Took a look at this, and Core passes the durations from the lang<=>core commands directly through to the server. So, ultimately converting the durations into the proto durations and throwing an error if they don't work is up to the lang implementation.

IE: If you want this to error, you probably want to bubble that error up from here: https://github.com/MercuryTechnologies/hs-temporal-sdk/blob/7e21046300fa096c8fb5227815453dcb023cb968/src/Temporal/Workflow.hs#L344

Closing this since there's nothing Core can really do here since all it ever saw was a 0 or empty Duration depending on exactly how the conversion works.