temporalio / sdk-java

Temporal Java SDK
https://temporal.io
Apache License 2.0
208 stars 141 forks source link

Async activity completion with primitive return value converts null to zero #701

Open osi opened 3 years ago

osi commented 3 years ago

Expected Behavior

When using async completion of an activity returning a primitive value and completing with null, a NullPointerException should be generated in either the Workflow or when completing the Activity. (Similar to how auto-unboxing behaves)

Actual Behavior

The workflow has a return value of zero.

Steps to Reproduce the Problem

  1. Create an activity method that returns a primitive, eg long.
  2. Use useLocalManualCompletion to get a completion client
  3. Complete the activity with null, eg complete(null)
  4. Inspect the returned value of the activity in the workflow. It will be zero.

Specifications

vkoby commented 2 years ago

Hi @osi Would you mind sending a code sample? I couldn't reproduce your issue (see my attempt here) Also, I noticed that your version of SDK is super outdated, I would highly recommend to update.

Spikhalskiy commented 2 years ago

@vkoby So, the test in https://github.com/temporalio/sdk-java/pull/846 passes. According to the build status. It shouldn't. This is exactly the problem that is reported in this issue.

vkoby commented 2 years ago

@Spikhalskiy @osi , Sorry, I wasn't clear: The reported issue is that null is converted to zero, and the repro step 4 says it's converted to zero. That doesn't reproduce - we are seeing null propagated instead, which if converted to int will result in NPE. There is a separate question whether we want null propagated in these cases, but that's not what that issue is about.

osi commented 2 years ago

Yes, I was on an older SDK. The missing detail in the reproduction steps is to invoke the activity synchronously from the workflow, so that there's unboxing required from the null completion value to the local primitive variable.

(yes, this is a programming error in the activity to return null)