Skip to content

[Go SDK] mustJSONReader silently drops json.Marshal errors #434

@santoshkumarradha

Description

@santoshkumarradha

Summary

mustJSONReader discards the json.Marshal error and returns an empty-body reader, silently sending a malformed or empty POST to the control plane when a value cannot be serialized.

Context

control_plane_memory_backend.go:402 defines func mustJSONReader(v any) io.Reader { data, _ := json.Marshal(v); return bytes.NewReader(data) }. When json.Marshal fails (unsupported type, cyclic struct, channel, function value), the error is thrown away and an empty bytes.NewReader([]byte(nil)) is returned. The downstream HTTP POST to the control plane sends an empty body, the store silently records nothing or overwrites with an empty value, and the real marshal error is never surfaced to the caller. This makes debugging data-loss bugs in the memory backend extremely difficult.

Scope

In Scope

  • Rename to jsonReader(v any) (io.Reader, error) and return the json.Marshal error.
  • Update all call sites (Set, Get, Delete) in control_plane_memory_backend.go to handle the error and propagate it to the caller.
  • Remove the must prefix convention from any helper in this file that can realistically fail.

Out of Scope

  • Changing the MemoryBackend interface signature beyond what is needed to propagate this error (context propagation is tracked in G2).
  • Adding JSON schema validation or custom marshal logic.

Files

  • sdk/go/agent/control_plane_memory_backend.go:402 — replace mustJSONReader with jsonReader(v any) (io.Reader, error) and propagate the error
  • sdk/go/agent/control_plane_memory_backend.go — update all callers of the helper to handle and return the error
  • sdk/go/agent/control_plane_memory_backend_test.go — test: passing an unmarshalable value to Set returns an error rather than silently succeeding

Acceptance Criteria

  • Set (and any other caller of the serialization helper) returns a non-nil error when the value cannot be JSON-marshaled
  • No json.Marshal errors are silently discarded anywhere in the file
  • The mustJSONReader function (or any equivalent silent-drop helper) no longer exists
  • Tests pass (go test ./sdk/go/...)
  • Linting passes (make lint)

Notes for Contributors

Severity: HIGH

Check the rest of the file for any other _, _ = json.Something or data, _ := patterns and fix those at the same time. The must naming convention should only be used for operations that genuinely cannot fail (e.g. parsing a compile-time constant) — not for anything that depends on user-provided data.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingsdk:goGo SDK related

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions