ogen
ogen copied to clipboard
JSON Marshal error on generated empty `OptString` struct
What version of ogen are you using?
v0.54.1
Can this issue be reproduced with the latest version?
Yes just try to run this test in directory where API is generated with OptString
package api
import (
"testing"
"encoding/json"
)
func TestOpString(t *testing.T) {
s := OptString{}
_, err := json.Marshal(s)
if err != nil {
t.Fatalf("error should be nil, got %v", err)
}
}
So for most use cases with fields set to not required when trying to marshal it (e.g. send through event bus) it's not possible to use ogen.
Getting output:
--- FAIL: TestOpString (0.00s)
/Users/exu/code/kube/testkube-cloud-api/pkg/api/optstring_test.go:16:
error should be nil, got json: error calling MarshalJSON for type api.OptString:
unexpected end of JSON input
Encoding and decoding with encoding/json is not supported for OptString because following limitations: https://github.com/golang/go/issues/11939
So it is impossible to represent OptString with encoding/json (pointers are used instead), that's why we are using go-faster/jx and custom code-generated json marshaling and unmarshaling.
@ernado thanks for the quick response, so how should I deal with it, can I omit OptString and use pointers as most other generators do? Or any other ideas on how to workaround this without changing the generator? (we have quite big API definition)
Looks like just handling zeroed value works
Would it be a case to just do PR to the generator for Opt* structs?
// MarshalJSON implements stdjson.Marshaler.
func (s OptString) MarshalJSON() ([]byte, error) {
if !s.Set {
return []byte(`{}`), nil
}
e := jx.Encoder{}
s.Encode(&e)
return e.Bytes(), nil
}
Just is there a way to check if Type is optional in template?
What are you trying to do?
Using MarshalJSON on requests, responses or most generated types should work.
Returning {} is invalid, probably we should not generate MarshalJSON for OptString at all, it is impossible to express omitempty struct with encoding/json.
I try to send generated struct through external library (NATS) and don't have control over marshaller. Additional info: Fields are not initialized.
It's more about generating valid JSON - as looks like currently, we're ending with something like field: when the struct is uninitialized. It would be ok for me to just store pass it (without handling omitempty) I just need valid JSON and currently it's not.
I see there several ways to fix it:
-
Do not generate
MarshalJSON/UnmarshalJSONfor theOpt/NilOpttypes.- Likely breaks existing code.
- These types would be marshaled/unmarshaled by
encoding/json, as JSON objects.
-
Return
[]byte("null").- Does not follow the optionals semantic.
-
~~Return an error in case if value is not set, add
omitemptytag to optional fields.~~- ~~Forces to add
omitemptytag. Some people may use generated types in their own structs.~~ - ~~Would not work properly in some cases. For example:
Setisfalse, but value is not zero.~~
- ~~Forces to add
@tdakkota omitempty does not work for structures, even if they are zero value.
The only valid solution is (1). Let's remove MarshalJSON/UnmarshalJSON for Opt* types, it should be only for full components (e.g. requests, responses).
Maybe just make no 1 optional? To not break possible existing implementations
Hi,
for objects workaround with overhead
...
// use generated method MarshalJSON.
tempBts, err := s.MarshalJSON()
if err != nil {
return fmt.Errorf("marshal: %w", err)
}
var m map[string]interface{}
// unmarshal to map[string]interface{}
if er := json.Unmarshal(tempBts, &m); er != nil {
return fmt.Errorf("umarshal to map: %w", er)
}
// use std MarshalIndent for get ident json.
bts, err := json.MarshalIndent(m, "", " ")
if err != nil {
return fmt.Errorf("marshal: %w", err)
}
P.S. If all of the Opt fields filled, error not happen!
P.P.S. ogen --version // ogen version v0.72.1 (built with go1.21.0) darwin/amd64
Just checking, is this still being worked on?
@tdakkota omitempty does not work for structures, even if they are zero value.
But they did recently add omitzero in https://github.com/golang/go/issues/45669
The only valid solution is (1). Let's remove MarshalJSON/UnmarshalJSON for Opt* types
Wouldn't this alter data if the structs do have values?
Is this all because of avoiding pointers?
Yes, we avoid pointers for better semantics and removing NPE problem.
Will use omitzero when released tho. Finally.
We've got Go 1.24 and omitzero can we look at having this patched.
Any chance this is being looked at? Otherwise I'd be happy to try help out on #1427 if that's the way to go?
We've had production bug recently due to incorrect usage of NewOpt[Type](...):
// nils.Unwrap(...) returns the zero value of the passed in pointer type for nil values
- out.Details = ogen.NewOptOrderDetails(nils.Unwrap(orderDetails))
+ if orderDetails != nil {
+ out.Details = ogen.NewOptOrderDetails(*orderDetails)
+ }
The old code would end up returning something like this (prettified) invalid json:
{
"id": "9ad94a36-e350-464a-b46c-3e2577dd1193",
"status": "APPROVED",
"customerId": "26b6c9e0-454d-4146-a8bd-469121ec0382",
"details":
"createdAt": "2025-05-06T07:44:00Z",
"updatedAt": "2025-05-06T07:44:00Z",
"fulfilledAt": "2025-05-06T07:44:00Z"
}
While the error definitely lies on our side and was an easy fix, it feels like an easy trap to fall in to by accident. In our case it was early on misunderstanding of what the NewOpt[Type] actually did that was found only after the stars aligned in production...
The problem occurs not only in Marshal but also in Unmarshal.