Skip to content

fix(go-sdk): surface json.Marshal errors in memory backend requests#679

Open
ardittirana wants to merge 1 commit into
Agent-Field:mainfrom
ardittirana:fix-go-sdk-jsonreader-marshal-error
Open

fix(go-sdk): surface json.Marshal errors in memory backend requests#679
ardittirana wants to merge 1 commit into
Agent-Field:mainfrom
ardittirana:fix-go-sdk-jsonreader-marshal-error

Conversation

@ardittirana

Copy link
Copy Markdown

Fixes #434.

Problem

mustJSONReader discarded the json.Marshal error:

func mustJSONReader(v any) io.Reader { data, _ := json.Marshal(v); return bytes.NewReader(data) }

When a value can't be serialized (unsupported type, cyclic struct, channel, func), json.Marshal returns nil, err, the error is dropped, and the function returns a reader over a nil slice. The downstream control-plane POST is then sent with an empty body — silently storing nothing or overwriting with an empty value — and the real error never reaches the caller, making data-loss bugs in the memory backend very hard to debug.

Fix

Replace it with jsonReader(v any) (io.Reader, error) that returns the marshal error, and propagate that error at each of the five call sites (Set, Get, Delete, SetVector, SearchVector) — all of which already return an error. No behavior change on the success path. Stdlib only.

Test

Updates the existing helper test and adds a case asserting that an unserializable value (a channel) returns an error and a nil reader instead of silently yielding an empty body. go test ./agent/ passes (all existing backend tests included); gofmt/go vet clean.

mustJSONReader discarded the json.Marshal error and returned a reader
over a nil byte slice. When a value could not be serialized (unsupported
type, cyclic struct, channel, func), the control-plane POST was sent with
an empty body — silently storing nothing or overwriting with an empty
value — and the real error never reached the caller.

Replace it with jsonReader(v any) (io.Reader, error) and propagate the
error at each call site (Set, Get, Delete, SetVector, SearchVector), all
of which already return an error.

Adds a test asserting an unserializable value yields an error instead of
an empty reader. Fixes Agent-Field#434.
@ardittirana ardittirana requested review from a team and AbirAbbas as code owners June 21, 2026 21:38
@CLAassistant

CLAassistant commented Jun 21, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@santoshkumarradha

Copy link
Copy Markdown
Member

Thanks @ardittirana, this looks good.

CLA is still pending. Can you sign it here?

https://cla-assistant.io/Agent-Field/agentfield?pullRequest=679

Once that clears, we can continue.

@ardittirana

Copy link
Copy Markdown
Author

Signed 🫡

@github-actions

Copy link
Copy Markdown
Contributor

Performance

SDK Memory Δ Latency Δ Tests Status
Go 221 B -21% 0.61 µs -39%

✓ No regressions detected

@github-actions

Copy link
Copy Markdown
Contributor

📊 Coverage gate

Thresholds from .coverage-gate.toml: per-surface ≥ 84%, aggregate ≥ 85%, max per-surface regression ≤ 1.0 pp, max aggregate regression ≤ 0.50 pp.

Surface Current Baseline Δ
control-plane 87.10% 87.40% ↓ -0.30 pp 🟡
sdk-go 91.70% 92.00% ↓ -0.30 pp 🟢
sdk-python 93.73% 93.73% ↑ +0.00 pp 🟢
sdk-typescript 90.20% 90.42% ↓ -0.22 pp 🟢
web-ui 84.94% 84.79% ↑ +0.15 pp 🟡
aggregate 85.74% 85.75% ↓ -0.01 pp 🟡

✅ Gate passed

No surface regressed past the allowed threshold and the aggregate stayed above the floor.

@github-actions

Copy link
Copy Markdown
Contributor

📐 Patch coverage gate

Threshold: 80% on lines this PR touches vs origin/main (from .coverage-gate.toml:thresholds.min_patch).

Surface Touched lines Patch coverage Status
control-plane 0 ➖ no changes
sdk-go 31 67.00%
sdk-python 0 ➖ no changes
sdk-typescript 0 ➖ no changes
web-ui 0 ➖ no changes

❌ Patch gate failed

sdk-go — 67.00% on 31 touched lines (10 uncovered):

File Patch coverage Missing lines
sdk/go/agent/control_plane_memory_backend.go 67.7% 61, 62, 94, 95, 135, 136, 217, 218, 306, 307

How to fix

  1. For each file listed above, add tests that exercise the missing line numbers in this same PR.
  2. Re-run locally: ./scripts/coverage-summary.sh && ./scripts/patch-coverage-gate.sh.
  3. Do not lower min_patch in .coverage-gate.toml to silence this — the floor is the contract.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Go SDK] mustJSONReader silently drops json.Marshal errors

3 participants