fix(api): serialize NoContent as null - #1387
Conversation
NoContent is the success value for every JSON-RPC method that returns
nothing. It had no MarshalJSON and ResponseObject.Result is a plain any,
so 24 methods sent "result":{} while docs/api/methods.md publishes
"result":null. The docs also contradicted themselves: five entries
described {}, written from the regressed behaviour rather than the
contract. media.generate and media.scrape expressed the same void
success as a literal nil and already sent null, so the API carried two
shapes for one concept.
Give NoContent a value-receiver MarshalJSON returning null. The receiver
must stay a value: handlers return NoContent{} as an any, and
encoding/json only finds a pointer-receiver marshaller on an addressable
value. All three send paths marshal the same ResponseObject, so this
covers plaintext WebSocket, encrypted WebSocket and HTTP POST.
Switch media.generate and media.scrape to the sentinel so void success
has one representation and a nilnil suppression goes away. Neither
changes on the wire.
Correct the five documented-{} methods and normalise profiles.delete's
phrasing. Zaparoo Online is unaffected: stop is the only void method in
the remote allowlist and encodeEmptyResult hard-codes an empty object
regardless of what the method returned.
Closes #1369
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (12)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughVoid JSON-RPC responses now return ChangesVoid response contract
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🔵 Low · up to Void JSON-RPC responses will change from {} to null across HTTP and WebSocket transports. The change is mergeable with explicit owner awareness because external clients that require an object-shaped success result could reject the response or behave differently. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The PR satisfies issue Full details: Docstring CoverageExplanation Docstring coverage is 42.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 11 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
NoContentaMarshalJSONreturningnull, so the 24 void JSON-RPC methods send"result": nullinstead of"result": {}. The receiver must stay a value receiver: handlers returnNoContent{}as anany, andencoding/jsononly finds a pointer-receiver marshaller on an addressable value.models.ResponseObject, so one marshaller covers plaintext WebSocket, encrypted WebSocket and HTTP POST. No server change needed.media.generateandmedia.scrapeto returnNoContent{}instead of a literal nil, so void success has one representation and anilnilsuppression goes away. Neither changes on the wire — both already sentnull.docs/api/methods.mdentries that described{}(media.control,settings.backup.delete,mappings.new,clients.delete,clients.pair.cancel) and normaliseprofiles.delete's phrasing.git blameputs those five after the regression landed, so they documented the drift rather than the contract. The other 19 entries already saidnull.ResponseObjectwhyResulthas noomitempty: JSON-RPC 2.0 §5 requires the key on success, so a void method sendsnullrather than omitting it.stopis the only void method in the remote allowlist and it routes throughencodeEmptyResult, which hard-codes an empty object regardless of what the method returned.zaparoo -api <void-method>now printsnullinstead of{}.BREAKING CHANGE: 24 void methods change their wire result from
{}tonull. For 19 of them this brings the code in line with what the docs have always published, andnullis what they sent before v2.10.0. The five methods listed above are the only ones whose documented contract changes. No first-party client is affected — the app resolves the result verbatim and ignores it for void methods, and the Rust and Go clients discard it.Closes #1369
Summary by CodeRabbit
Bug Fixes
"result": nullon success instead of an empty object or missing result.Documentation
nullsuccess results.