Improvements to resources endpoint to support debugging

[chrisghill]

Summary of changes:

Changes to massdriver/provisioning/resources/resources.go

• Added omitempty to ID, Field, Type, Name JSON tags (left Payload alone — nil should surface as a clean server error). Added a comment on the struct explaining why omitempty matters here so a future contributor doesn't innocently remove it.
• Added formatAPIError helper that parses the server's JSON error body via rest.ParseJSONErrorResponse and renders it like create resource: 422: payload: can't be blank. Wired into every method.
• Dropped the field arg from DeleteResource — the Elixir controller's delete only pattern-matches on %{"id" => artifact_identifier} from the URL; it never reads the body.

Changes to resources_test.go

• Added TestCreateResource_OmitsEmptyFields — proves a Resource with only Type + Name + Payload serializes without a "field":"" key (the bug that was misrouting imported creates).
• Added TestCreateResource_SurfacesServerError — proves a 422 with {"errors":{"payload":["can't be blank"]}} produces an error string containing both payload and can't be blank.
• Updated TestDeleteResource for the new no-body signature.

[chrisghill]

@claude review

[claude]

Claude finished @chrisghill's task in 1m 22s —— View job

---

Code Review

• Gather context (git diff, changed files, internal packages)
• Analyze changes in resources.go
• Analyze changes in resources_test.go
• Post review feedback

---

Overall this is a solid improvement. The motivation is clear — surfacing structured server error bodies instead of bare status strings — and the implementation is clean. A few things worth addressing before merge:

---

Breaking change in DeleteResource

DeleteResource signature changed from (ctx, id, field string) to (ctx, id string). Any existing callers outside this repo will break silently at compile time. Since this is an SDK, this warrants a note in the PR description (and ideally a changelog entry or semver bump discussion). The change itself looks intentional and correct, just undocumented.

---

Payload field: nil serializes as null

In resources.go:34, Payload has no omitempty:

Payload map[string]interface{} `json:"payload"`

If a caller passes &Resource{Field: "network"} (no Payload), the wire body will be {"field":"network","payload":null}. Whether the server accepts null vs omitted is worth confirming. If Payload is always required, consider adding a validation step or at minimum documenting that nil Payload is not supported. Fix this →

---

Minor style inconsistency in test

In resources_test.go:63, require.True with a strings.Contains is used inconsistently — the two assertions just above use require.Contains directly:

// Lines 61-63
require.Contains(t, err.Error(), "payload")
require.Contains(t, err.Error(), "can't be blank")
require.True(t, strings.Contains(err.Error(), "422"), "expected status in error, got: %s", err.Error())

The third should match:

require.Contains(t, err.Error(), "422")

require.Contains produces cleaner failure output. Fix this →

---

Untested formatAPIError fallthrough

formatAPIError (resources.go:144-145) has a path where the JSON parses successfully but has neither error nor errors fields, returning just the status code:

return fmt.Errorf("%s: %d", op, resp.StatusCode())

This is a silent case — an empty-body or unexpected-shape JSON response drops all context. A test for this path (e.g., server returns {}) would confirm the behavior is intentional and catch regressions. This is minor but worth noting.

---

What's good

• Sorted field names in the validation error formatter (sort.Strings(fields)) ensures deterministic output in logs and tests — well done.
• omitempty on all Resource fields and the test TestCreateResource_OmitsEmptyFields correctly pins the server dispatch contract on the wire. The struct comment explaining the two routing paths is excellent.
• formatAPIError fallback (strings.TrimSpace(string(resp.Body()))) for non-JSON responses handles edge cases gracefully.
• DeleteResource simplification removing the body is cleaner if the server no longer requires it.

---