go-kit / kit

A standard library for microservices.
https://gokit.io
MIT License
26.35k stars 2.42k forks source link

[NATS transport] Support for headers in EncodeJSONResponse #1238

Open mcosta74 opened 2 years ago

mcosta74 commented 2 years ago

What would you like?

Overview

Since release v1.11.0, nats.go supports message headers. headers are very useful to transfer metadata together with messages.

I'm proposing to add basic support for headers in the default response encoder.

Proposed API

The idea is to do something similar it's done for transport/http with the Headerer interface. API change proposal is backward compatible (no public API change)

type Headerer interface {
    Headers() Header
}

func EncodeJSONResponse(_ context.Context, reply string, nc *nats.Conn, response interface{}) error {
        msg := nats.NewMessage(reply)
        if nc. HeadersSupported() {
                if headerer, ok := response.(Headerer); ok {
                         msg.Header = headerer.Headers()  // I wrote that to make it shorter
                }
        }

    msg.Data, err := json.Marshal(response)
    if err != nil {
        return err
    }

    return nc.PublishMsg(msg)
}

If the proposal is accepted I can raise a PR

peterbourgon commented 2 years ago

EncodeJSONResponse is a default implementation of a function that consumers can also provide themselves. Does anything prevent that, here?

mcosta74 commented 2 years ago

No, I'm wondering if a default implementation could have the same feature than the HTTP one.

==============

Massimo Costa

Il mer 29 giu 2022, 18:51 Peter Bourgon @.***> ha scritto:

EncodeJSONResponse is a default implementation of a function that consumers can also provide themselves. Does anything prevent that, here?

— Reply to this email directly, view it on GitHub https://github.com/go-kit/kit/issues/1238#issuecomment-1170233764, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA4I7OL4KRL6VJYRFXQYT23VRR5JRANCNFSM52EO2ZYA . You are receiving this because you authored the thread.Message ID: @.***>

peterbourgon commented 2 years ago

So I'd like to try to avoid the situation where default implementations provided by Go kit, like this function, being continuously updated with new features/capabilities of their underlying systems. I see that as unsustainable, as well as unnecessary: Go kit is designed specifically to allow consumers to provide their own EncodeJSONResponse functions.

Can you suggest some wording for the docs that would make this clear?

mcosta74 commented 1 year ago

Something like

Go kit provides default implementation for many functions (for instance http.EncodeJSONResponse, nats.EncodeJSONResponse, http.DefaultErrorEncoder, nats.DefaultErrorEncoder,...) but is designed to allow consumers to provide their implementation (maybe to use the latest features of the underlying systems).

?

peterbourgon commented 1 year ago

SGTM

mcosta74 commented 1 year ago

Can you suggest "where" to put this sentence in the documentation?