internal/workflow: always use JSON-marshaled task results
We only realized JSON marshaling of the release tasks was broken when
we retried them because in normal operation we used the in-memory
versions. Change to always use the marshaled results.
For golang/go#51797.
Change-Id: I9dbca62520732a7b00d64dc2481ab35b221fecf0
Reviewed-on: https://go-review.googlesource.com/c/build/+/411894
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/internal/workflow/workflow.go b/internal/workflow/workflow.go
index b6d11bf..3ef4da7 100644
--- a/internal/workflow/workflow.go
+++ b/internal/workflow/workflow.go
@@ -518,11 +518,11 @@
serializedResult: tState.SerializedResult,
}
if state.serializedResult != nil {
- ptr := reflect.New(reflect.ValueOf(taskDef.f).Type().Out(0))
- if err := json.Unmarshal(tState.SerializedResult, ptr.Interface()); err != nil {
+ result, err := unmarshalNew(reflect.ValueOf(taskDef.f).Type().Out(0), tState.SerializedResult)
+ if err != nil {
return nil, fmt.Errorf("failed to unmarshal result of %v: %v", taskDef.name, err)
}
- state.result = ptr.Elem().Interface()
+ state.result = result
}
if tState.Error != "" {
state.err = fmt.Errorf("serialized error: %v", tState.Error) // untyped, but hopefully that doesn't matter.
@@ -532,6 +532,14 @@
return w, nil
}
+func unmarshalNew(t reflect.Type, data []byte) (interface{}, error) {
+ ptr := reflect.New(t)
+ if err := json.Unmarshal(data, ptr.Interface()); err != nil {
+ return nil, err
+ }
+ return ptr.Elem().Interface(), nil
+}
+
// Run runs a workflow to completion or quiescence and returns its outputs.
// listener.TaskStateChanged will be called immediately, when each task starts,
// and when they finish. It should be used only for monitoring and persistence
@@ -612,17 +620,21 @@
WorkflowID: w.ID,
}
in := append([]reflect.Value{reflect.ValueOf(tctx)}, args...)
- out := reflect.ValueOf(state.def.f).Call(in)
+ fv := reflect.ValueOf(state.def.f)
+ out := fv.Call(in)
if errIdx := len(out) - 1; !out[errIdx].IsNil() {
state.err = out[errIdx].Interface().(error)
}
state.finished = true
- if len(out) == 2 {
- state.result = out[0].Interface()
- }
- if state.err == nil {
- state.serializedResult, state.err = json.Marshal(state.result)
+ if len(out) == 2 && state.err == nil {
+ state.serializedResult, state.err = json.Marshal(out[0].Interface())
+ if state.err == nil {
+ state.result, state.err = unmarshalNew(fv.Type().Out(0), state.serializedResult)
+ }
+ if state.err == nil && !reflect.DeepEqual(out[0].Interface(), state.result) {
+ state.err = fmt.Errorf("JSON marshaling changed result from %#v to %#v", out[0].Interface(), state.result)
+ }
}
return state
}
diff --git a/internal/workflow/workflow_test.go b/internal/workflow/workflow_test.go
index b0cc255..7630476 100644
--- a/internal/workflow/workflow_test.go
+++ b/internal/workflow/workflow_test.go
@@ -342,6 +342,23 @@
})
}
+type badResult struct {
+ unexported string
+}
+
+func TestBadMarshaling(t *testing.T) {
+ greet := func(_ context.Context) (badResult, error) {
+ return badResult{"hi"}, nil
+ }
+
+ wd := workflow.New()
+ wd.Output("greeting", wd.Task("greet", greet))
+ w := startWorkflow(t, wd, nil)
+ if _, err := w.Run(context.Background(), &verboseListener{t}); err == nil {
+ t.Errorf("running a workflow with bad JSON should give an error, got none")
+ }
+}
+
type mapListener struct {
workflow.Listener
states map[uuid.UUID]map[string]*workflow.TaskState
