Skip to content
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

Unable to set options (UseNumber & DisallowUnknownFields()) on the json decoder #514

Closed
MatthewDolan opened this issue Jul 20, 2023 · 2 comments

Comments

@MatthewDolan
Copy link

Is your feature request related to a problem? Please describe.

Here's documentation for the two options:

Currently, while it's possible to set a few fields on the encoder (SetEscapeHTML(...) & SetIndent(...)), it's not possible to set any on the decoder.

An example reason why someone would want to use number is that by default, the json decoding library uses a float64 to represent a number value. If the number is intended to always be an integer, casting it first to float and then to integer is confusing and might introduce a chance of error in the code.

Describe the solution you'd like

#513

Describe alternatives you've considered

Before this change, the lambda API supported setting some of the json encoder options (SetEscapeHTML(...) & SetIndent(...)). I needed the ability to set the UseNumber() option on the decoder and I considered setting it in the same way (by creating a one-off option), but instead I ended up creating two new options that allow setting arbitrary fields on the encoder and decoder.

I think this is a better option because it's future-proof (if new options are added to the json library, this library won't also need to be changed to make this option available) and more backward compatible.

Additional context

N/A

@bmoffatt
Copy link
Collaborator

If Lambda in the future allows inputs other than json, I'd think to add the option to set an arbitrary decoder. Something like:

type Decoder interface {
    Decode(out any) error
}

// Set the decoder implemention for the handler
// Lambda's default behavior is as if the function was configured with
// lambda.WithDecoder(json.NewDecoder)
func WithDecoder(provider func(io.Reader) Decoder) Option {
    return Option(func (h *handlerOptions) {
        h.decoderProvider = provider
    })
}

So through that lens I'm hesitant to also take the approach shown in #513, as I think it'd stand out as API bloat alongside a possible future with more generic approach to decoding.


I'm OK with adding an option like WithUseNumber as it'd be consistent with the current design of the library.

The future-proofing aspect of the approach in #513 is nice, but from the release history it doesn't seem like the plumbing would have to happen often. It looks like the last time the stdlib json Decoder/Encoder even got a new option was > 5 years ago in v1.10!

@bmoffatt
Copy link
Collaborator

New options available github.com/aws/aws-lambda-go@main. And I'll be able to tag a release in the next week or two.

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

No branches or pull requests

2 participants