Merge pull request #225 from flashcatcloud/fix/warn-dropped-data-fields

fix(cli): warn when --data fields cannot be sent

Author: ysyneu

internal/cli/alert.go

@@ -386,7 +386,7 @@ Request fields:
 					return err
 				}
 				req := new(flashduty.AlertMergeRequest)
-				if err := genBindBody(body, req); err != nil {
+				if err := genBindBody(ctx.Cmd.ErrOrStderr(), body, req); err != nil {
 					return err
 				}
 				resp, err := ctx.Client.Alerts.WriteMerge(cmdContext(ctx.Cmd), req)

internal/cli/automation.go

@@ -512,7 +512,7 @@ func runAutomationFire(ctx *RunContext, triggerID, token, text, dataJSON string)
 		return nil, err
 	}
 	req := new(flashduty.AutomationFireAPITriggerRequest)
-	if err := genBindBody(body, req); err != nil {
+	if err := genBindBody(ctx.Cmd.ErrOrStderr(), body, req); err != nil {
 		return nil, err
 	}
 	token = strings.TrimSpace(token)

internal/cli/channel.go

@@ -197,7 +197,7 @@ func newChannelEscalateRuleListCmd() *cobra.Command {
 					return err
 				}
 				req := new(flashduty.ChannelScopedListRequest)
-				if err := genBindBody(body, req); err != nil {
+				if err := genBindBody(ctx.Cmd.ErrOrStderr(), body, req); err != nil {
 					return err
 				}
 				out, _, err := ctx.Client.Channels.ChannelEscalateRuleList(cmdContext(ctx.Cmd), req)

internal/cli/gen_support.go

@@ -5,8 +5,10 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"maps"
 	"os"
 	"reflect"
+	"slices"
 	"strconv"
 	"strings"
 	"time"
@@ -215,7 +217,11 @@ func genFoldPositional(args []string, body map[string]any, wire, kind string) er
 // command un-driveable. After the json pass, additively bind any url-tagged
 // field from the body by its url wire-name. For POST structs the url pass is a
 // no-op, so existing behavior is unchanged.
-func genBindBody(body map[string]any, req any) error {
+//
+// json.Unmarshal discards body fields the request struct has no field for, so a
+// --data field newer than this build's SDK would vanish before the request is
+// sent while the call still succeeds. Each such field is named on warn instead.
+func genBindBody(warn io.Writer, body map[string]any, req any) error {
 	var missing []string
 	var inspect func(reflect.Type)
 	inspect = func(rt reflect.Type) {
@@ -271,9 +277,103 @@ func genBindBody(body map[string]any, req any) error {
 		return fmt.Errorf("failed to bind request: %w", err)
 	}
 	bindURLTagged(body, reflect.ValueOf(req))
+	if dropped := unboundFields(body, reflect.TypeOf(req), ""); len(dropped) > 0 {
+		_, _ = fmt.Fprintf(warn, "warning: --data fields not sent, unknown to this CLI version: %s\n",
+			strings.Join(dropped, ", "))
+	}
 	return nil
 }
 
+var jsonUnmarshalerType = reflect.TypeFor[json.Unmarshaler]()
+
+// unboundFields lists the paths in v (dotted, "[]" for array elements) that
+// json.Unmarshal into t discards because t has no field for them. Types that
+// decode themselves (json.RawMessage, time.Time) and interface/scalar targets
+// take whatever they are given, so nothing below them is reported.
+func unboundFields(v any, t reflect.Type, path string) []string {
+	for t.Kind() == reflect.Pointer {
+		t = t.Elem()
+	}
+	if reflect.PointerTo(t).Implements(jsonUnmarshalerType) {
+		return nil
+	}
+	var out []string
+	switch t.Kind() {
+	case reflect.Struct:
+		m, _ := v.(map[string]any)
+		for _, k := range slices.Sorted(maps.Keys(m)) {
+			p := joinFieldPath(path, k)
+			ft, ok := wireField(t, k)
+			if !ok {
+				out = append(out, p)
+				continue
+			}
+			out = append(out, unboundFields(m[k], ft, p)...)
+		}
+	case reflect.Map:
+		m, _ := v.(map[string]any)
+		for _, k := range slices.Sorted(maps.Keys(m)) {
+			out = append(out, unboundFields(m[k], t.Elem(), joinFieldPath(path, k))...)
+		}
+	case reflect.Slice, reflect.Array:
+		items, _ := v.([]any)
+		for _, item := range items {
+			out = append(out, unboundFields(item, t.Elem(), path+"[]")...)
+		}
+		slices.Sort(out)
+		out = slices.Compact(out)
+	}
+	return out
+}
+
+// wireField resolves a body key to the struct field json.Unmarshal (or, for GET
+// query structs, bindURLTagged) would fill: exact wire-name match first, then
+// encoding/json's case-insensitive fallback, descending into untagged embedded
+// structs.
+func wireField(t reflect.Type, key string) (reflect.Type, bool) {
+	var fold reflect.Type
+	var walk func(reflect.Type) (reflect.Type, bool)
+	walk = func(t reflect.Type) (reflect.Type, bool) {
+		for i := 0; i < t.NumField(); i++ {
+			f := t.Field(i)
+			tag := f.Tag.Get("json")
+			if tag == "" {
+				tag = f.Tag.Get("url")
+			}
+			name := strings.Split(tag, ",")[0]
+			if f.Anonymous && name == "" {
+				et := f.Type
+				for et.Kind() == reflect.Pointer {
+					et = et.Elem()
+				}
+				if et.Kind() == reflect.Struct {
+					if ft, ok := walk(et); ok {
+						return ft, true
+					}
+					continue
+				}
+			}
+			if !f.IsExported() || name == "-" {
+				continue
+			}
+			if name == "" {
+				name = f.Name
+			}
+			if name == key {
+				return f.Type, true
+			}
+			if fold == nil && strings.EqualFold(name, key) {
+				fold = f.Type
+			}
+		}
+		return nil, false
+	}
+	if ft, ok := walk(t); ok {
+		return ft, true
+	}
+	return fold, fold != nil
+}
+
 // bindURLTagged fills fields carrying a `url` struct tag (GET query params) from
 // the body map keyed by the url wire-name. json.Unmarshal cannot reach these
 // because they lack a json tag. The 6 GET request types use only int64/string

internal/cli/gen_support_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"io"
 	"strings"
 	"testing"
 	"unicode/utf8"
@@ -16,14 +17,60 @@ func TestGenBindBodyAllowsNullForRequiredNullableField(t *testing.T) {
 		Value *bool `json:"value"`
 	})
 
-	if err := genBindBody(map[string]any{"value": nil}, req); err != nil {
+	if err := genBindBody(io.Discard, map[string]any{"value": nil}, req); err != nil {
 		t.Fatalf("genBindBody required nullable field: %v", err)
 	}
 	if req.Value != nil {
 		t.Fatalf("Value = %v, want nil", req.Value)
 	}
 }
 
+func TestGenBindBodyWarnsOnFieldsTheRequestCannotCarry(t *testing.T) {
+	type target struct {
+		Kind  string          `json:"kind"`
+		Query json.RawMessage `json:"query,omitempty"`
+	}
+	type embedded struct {
+		Team int64 `json:"team_id,omitempty"`
+	}
+	req := new(struct {
+		embedded
+		Name    string            `json:"name"`
+		Targets []target          `json:"targets,omitempty"`
+		Labels  map[string]target `json:"labels,omitempty"`
+		Page    int64             `url:"p,omitempty"`
+	})
+	body := map[string]any{
+		"name":    "r",
+		"NAME":    "folded",
+		"team_id": 1,
+		"p":       2,
+		"extra":   true,
+		"targets": []any{
+			map[string]any{"kind": "x", "purpose": "a", "query": map[string]any{"anything": 1}},
+			map[string]any{"kind": "y", "purpose": "b"},
+		},
+		"labels": map[string]any{"k": map[string]any{"kind": "z", "note": "c"}},
+	}
+
+	var warn bytes.Buffer
+	if err := genBindBody(&warn, body, req); err != nil {
+		t.Fatalf("genBindBody: %v", err)
+	}
+	want := "warning: --data fields not sent, unknown to this CLI version: extra, labels.k.note, targets[].purpose\n"
+	if warn.String() != want {
+		t.Fatalf("warning = %q, want %q", warn.String(), want)
+	}
+
+	warn.Reset()
+	if err := genBindBody(&warn, map[string]any{"name": "r", "targets": []any{map[string]any{"kind": "x"}}}, req); err != nil {
+		t.Fatalf("genBindBody: %v", err)
+	}
+	if warn.Len() != 0 {
+		t.Fatalf("warning on a fully bound body: %q", warn.String())
+	}
+}
+
 // oversizedInsightRows returns n insight-incident rows whose fat description
 // fields push any page of them well past compactListOutputLimit, plus the
 // incident IDs in row order so a test can tell emitted rows from dropped ones.

internal/cli/insight.go

@@ -303,7 +303,7 @@ Request fields:
 					return err
 				}
 				req := new(flashduty.InsightFilter)
-				if err := genBindBody(body, req); err != nil {
+				if err := genBindBody(ctx.Cmd.ErrOrStderr(), body, req); err != nil {
 					return err
 				}
 				resp, err := ctx.Client.Analytics.IncidentExport(cmdContext(ctx.Cmd), req)
@@ -382,8 +382,9 @@ func countCSVDataRows(raw []byte) (int, error) {
 // is verified against. Only the total is needed, so a single 1-item page is
 // fetched.
 func insightIncidentTotal(ctx *RunContext, body map[string]any) (int64, error) {
+	// The export call already warned about this body's unsendable fields.
 	req := new(flashduty.InsightIncidentListRequest)
-	if err := genBindBody(body, req); err != nil {
+	if err := genBindBody(io.Discard, body, req); err != nil {
 		return 0, err
 	}
 	req.Page = 1