uber-go / cadence-client

Framework for authoring workflows and activities running on top of the Cadence orchestration engine.
https://cadenceworkflow.io
MIT License
344 stars 130 forks source link

Ignore EOF errors in `jsonEncoding.Unmarshal()` #1188

Closed jjti closed 1 year ago

jjti commented 1 year ago

What changed?

I expect this will better align with user expectations. For example, if you json.Unmarshal a 0-length JSON array into a 1-length slice of struct pointers, the pointer remains zero'ed without err: https://go.dev/play/p/ppz62H1Pqpw

To unmarshal a JSON array into a Go array, Unmarshal decodes JSON array elements into corresponding Go array elements. If the Go array is smaller than the JSON array, the additional JSON array elements are discarded. If the JSON array is smaller than the Go array, the additional Go array elements are set to zero values.

https://pkg.go.dev/encoding/json#Unmarshal:~:text=To%20unmarshal%20a%20JSON%20array%20into%20a%20Go%20array

Why?

I've found that the Cadence Replayer fails on many of my team's Workflows:

resp: RespondDecisionTaskCompletedRequest{
    TaskToken: [82 101 112 108 97 121 84 97 115 107 84 111 107 101 110],
    Decisions: [
        Decision{
            DecisionType: FailWorkflowExecution,
            FailWorkflowExecutionDecisionAttributes: FailWorkflowExecutionDecisionAttributes{
                Reason: cadenceInternal:Generic,
                Details: [117 110 97 98 108 101 32 116 111 32 100 101 99 111 100 101 32 97 114 103 117 109 101 110 116 58 32 48 44 32 42 105 110 116 101 114 102 97 99 101 32 123 125 44 32 119 105 116 104 32 106 115 111 110 32 101 114 114 111 114 58 32 69 79 70]}}],
                Identity: replayID,
                ReturnNewDecisionTask: true
                ForceCreateNewDecisionTask: false,
                BinaryChecksum: e5dd254c6d311f3fc3021e413368a456}

last: HistoryEvent{
    EventId: 386,
    Timestamp: 1662901315491153124,
    EventType: WorkflowExecutionCompleted,
    Version: -24,
    TaskId: 2831498722,
    WorkflowExecutionCompletedEventAttributes: WorkflowExecutionCompletedEventAttributes{DecisionTaskCompletedEventId: 385}}

Ie we're getting: unable to decode argument: 0, *interface {}, with json error: EOF in Cadence Replayer where the Workflows happily succeed in our production code. Others have noticed the same: https://github.com/uber-go/cadence-client/issues/1016

This happens when we pass an interface{} to a future.Get(ctx, value) (as a throw away value, we don't actually expect a result):

    var result interface{}
    future := UpdateActivity.ExecuteRaw(ctx, p)
    return future.Get(ctx, &result)

How did you test it?

determinism_test.go:174: successfully replayed history

Potential risks

CLAassistant commented 1 year ago

CLA assistant check
All committers have signed the CLA.

jjti commented 1 year ago

closing because I don't like seeing this sit open in my PRs page, still think ya'll should do it