Skip to content

fix: JSON marshal errors of parameters with optional structs #1427

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

maelick
Copy link

@maelick maelick commented Mar 17, 2025

I experienced an issue similar to #660 when passing a parameter struct to stdlib slog (using a JSONHandler).

From what I was able to reproduce, the error occurs when calling encoding/json.Marshal on missing optional parameters that that the Opt* struct is used in response types. The problem does not seem to occur if there are no response objects including the optional type (because the Encode/Decode methods are not generated on the Opt* types).

For me the error occurred when using slog with a JSONHandler inside an ogen handler:

logger := slog.New(slog.NewJSONHandler(&strings.Builder{}, &slog.HandlerOptions{}))
[...]
logger.Info("test", "params", params)

Which gives this type of log output:

{"time":"2025-03-17T16:13:56.22672301+01:00","level":"INFO","msg":"test","params":"!ERROR:json: error calling MarshalJSON for type api.OptInt: unexpected end of JSON input"}

This PR includes:

  • test to reproduce the problem
    • both with encoding/json and slog
    • test can be run without the fix with maelick/ogen@e772bdb
    • the problem specifically occurs when the optional struct is used both as a param and as part of a return type
    • I had to add an endpoint that also returns each value type (see maelick/ogen@14db889), otherwise the Encode/Decode methods that use go-faster/jx are not generated
  • a (partial) fix for JSON Marshal error on generated empty OptString struct #660
    • based on Go 1.24 omitzero tag
    • change limited to parameter structs (i.e. does not affect schema structs)
    • add method GoStructTag on Parameter to return struct field tags:
      • omitempty for slices, maps and non-optional nullable fields
      • omitempty and omitzero for optional nullable fields
      • nothing for other fields
    • call method in parameters.tmpl similar to struct.tmpl

Notes:

  • I didn't change the minimum Go version in go.mod
    • not sure whether the new tag is backward compatible and whether Go 1.23 would just ignore it or not
    • edit: tests are failing in the pipeline, I assume because Go 1.23 is used to execute them
  • I only partially squashed my branch to keep a working commit with the test but without the fix
    • if needed I can re-squash to have one commit with the changes and one with the generated files
    • edit: did more squashing

maelick added 2 commits March 18, 2025 08:35
Update parameter integration test with:
* new /optionalParameters endpoint in test spec
* add empty handler for new endpoint
* add test for logging and marshaling params
@maelick maelick force-pushed the optional-params-omitzero-squashed branch 2 times, most recently from cbf3aa0 to 4b1ce7c Compare March 18, 2025 07:47
maelick added 2 commits March 18, 2025 15:51
Generated parameter structs have field tags omitempty and/or omitzero annotations:
* omitempty for slices, maps and non-optional nullable fields
* omitempty and omitzero for optional nullable fields
* nothing for other fields
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.

1 participant