From 92dabfb1193ccd5054ff305af0e077ace222f721 Mon Sep 17 00:00:00 2001 From: ispiroglu Date: Fri, 20 Sep 2024 22:31:47 +0300 Subject: [PATCH 1/9] refactor: client err decoder - make factory.Get method accepts options to provide default err decoder without declaration - refactored defaul error decoder to provide more detailed information on error responses from clients --- modules/client/client.go | 15 +++++++-- modules/client/error.go | 71 +++++++++++++++++++++++++++++++++++++--- modules/client/option.go | 28 ++++++++++++++++ 3 files changed, 106 insertions(+), 8 deletions(-) create mode 100644 modules/client/option.go diff --git a/modules/client/client.go b/modules/client/client.go index 2cfa1e0..6d67cd7 100644 --- a/modules/client/client.go +++ b/modules/client/client.go @@ -24,12 +24,21 @@ func NewFactory(cfg *config.Config, wrappers []DriverWrapper) *Factory { } } -func (f *Factory) Get(name string, errDecoder ErrDecoder, driverWrappers ...DriverWrapper) *Base { +func (f *Factory) Get(name string, opts ...Option) *Base { + cOpts := &options{ + errDecoder: DefaultErrDecoder(name), + driverWrappers: []DriverWrapper{}, + } + + for _, opt := range opts { + opt.Apply(cOpts) + } + return &Base{ driver: newDriverBuilder(f.cfg.Of("client").Of(name)). - AddErrDecoder(errDecoder). + AddErrDecoder(cOpts.errDecoder). AddUpdaters(f.baseWrappers...). - AddUpdaters(driverWrappers...). + AddUpdaters(cOpts.driverWrappers...). build(), name: name, } diff --git a/modules/client/error.go b/modules/client/error.go index 7ea721c..b468406 100644 --- a/modules/client/error.go +++ b/modules/client/error.go @@ -2,16 +2,77 @@ package client import ( "context" + "encoding/json" + "fmt" + "strings" "github.com/go-resty/resty/v2" - "github.com/gofiber/fiber/v2" ) type ErrDecoder func(context.Context, *resty.Response) error -func DefaultErrDecoder(_ context.Context, res *resty.Response) error { - if res.StatusCode() > 300 { - return fiber.NewError(res.StatusCode(), "client error") +type GenericClientError struct { + ClientName string + StatusCode int + RawBody []byte + ParsedBody interface{} +} + +func (e GenericClientError) Error() string { + msg := fmt.Sprintf("Error on client %s (Status %d)", e.ClientName, e.StatusCode) + if details := e.extractErrorDetails(); details != "" { + msg += ": " + details + } + return msg +} + +func (e GenericClientError) extractErrorDetails() string { + var details []string + + var extract func(interface{}) + extract = func(v interface{}) { + switch value := v.(type) { + case string: + details = append(details, strings.TrimSpace(value)) + case map[string]interface{}: + for _, v := range value { + extract(v) + } + case []interface{}: + for _, v := range value { + extract(v) + } + } + } + + extract(e.ParsedBody) + + if len(details) == 0 && len(e.RawBody) > 0 { + return strings.TrimSpace(string(e.RawBody)) + } + + return strings.Join(details, "; ") +} + +func DefaultErrDecoder(name string) ErrDecoder { + return func(_ context.Context, res *resty.Response) error { + if res.IsSuccess() { + return nil + } + + apiErr := GenericClientError{ + ClientName: name, + StatusCode: res.StatusCode(), + RawBody: res.Body(), + } + + var jsonBody interface{} + if err := json.Unmarshal(res.Body(), &jsonBody); err == nil { + apiErr.ParsedBody = jsonBody + } else { + apiErr.ParsedBody = string(res.Body()) + } + + return apiErr } - return nil } diff --git a/modules/client/option.go b/modules/client/option.go new file mode 100644 index 0000000..d527f23 --- /dev/null +++ b/modules/client/option.go @@ -0,0 +1,28 @@ +package client + +type options struct { + errDecoder ErrDecoder + driverWrappers []DriverWrapper +} + +type Option interface { + Apply(*options) +} + +type withOption func(*options) + +func (wf withOption) Apply(opts *options) { + wf(opts) +} + +func WithErrDecoder(ed ErrDecoder) Option { + return withOption(func(o *options) { + o.errDecoder = ed + }) +} + +func WithDriverWrappers(wrappers ...DriverWrapper) Option { + return withOption(func(o *options) { + o.driverWrappers = append(o.driverWrappers, wrappers...) + }) +} From 1bbe30a78ee4d3f5c6e15baba17719f99afa3e07 Mon Sep 17 00:00:00 2001 From: ispiroglu Date: Fri, 20 Sep 2024 23:05:02 +0300 Subject: [PATCH 2/9] refactor: make logging on clients accessible from config --- .../client-server-with-otel/client/client.go | 2 +- example/client-server/client/client.go | 2 +- example/client-server/client/main.go | 3 +++ modules/client/driver.go | 19 +++++++++++++++++-- 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/example/client-server-with-otel/client/client.go b/example/client-server-with-otel/client/client.go index 3cda1ee..dcc2fb4 100644 --- a/example/client-server-with-otel/client/client.go +++ b/example/client-server-with-otel/client/client.go @@ -14,7 +14,7 @@ type exampleClient struct { func newClient(f *client.Factory) *exampleClient { return &exampleClient{ - Base: f.Get("example-client", client.DefaultErrDecoder), + Base: f.Get("example-client"), } } diff --git a/example/client-server/client/client.go b/example/client-server/client/client.go index 3cda1ee..dcc2fb4 100644 --- a/example/client-server/client/client.go +++ b/example/client-server/client/client.go @@ -14,7 +14,7 @@ type exampleClient struct { func newClient(f *client.Factory) *exampleClient { return &exampleClient{ - Base: f.Get("example-client", client.DefaultErrDecoder), + Base: f.Get("example-client"), } } diff --git a/example/client-server/client/main.go b/example/client-server/client/main.go index a124d2b..c879e02 100644 --- a/example/client-server/client/main.go +++ b/example/client-server/client/main.go @@ -9,6 +9,7 @@ import ( "github.com/Trendyol/chaki/modules/server" "github.com/Trendyol/chaki/modules/server/controller" "github.com/Trendyol/chaki/modules/server/route" + "github.com/Trendyol/chaki/modules/swagger" ) func main() { @@ -17,6 +18,8 @@ func main() { app.Use( client.Module(), server.Module(), + + swagger.Module(), ) app.Provide( diff --git a/modules/client/driver.go b/modules/client/driver.go index 5dd5d53..84dd113 100644 --- a/modules/client/driver.go +++ b/modules/client/driver.go @@ -15,9 +15,16 @@ type driverBuilder struct { } func newDriverBuilder(cfg *config.Config) *driverBuilder { + setDefaults(cfg) + + d := resty.New(). + SetBaseURL(cfg.GetString("baseurl")). + SetTimeout(cfg.GetDuration("timeout")). + // Debug mode provides a logging, but it's not in the same format with our logger. + SetDebug(cfg.GetBool("debug")) return &driverBuilder{ cfg: cfg, - d: resty.New().SetBaseURL(cfg.GetString("baseurl")), + d: d, } } @@ -32,7 +39,9 @@ func (b *driverBuilder) AddUpdaters(wrappers ...DriverWrapper) *driverBuilder { } func (b *driverBuilder) build() *resty.Client { - b.useLogging() + if b.cfg.GetBool("logging") { + b.useLogging() + } for _, upd := range b.updaters { b.d = upd(b.d) @@ -68,3 +77,9 @@ func (b *driverBuilder) useLogging() { return nil }) } + +func setDefaults(cfg *config.Config) { + cfg.SetDefault("timeout", "5s") + cfg.SetDefault("debug", false) + cfg.SetDefault("logging", false) +} From 938feee6fb0c1b54c51c27f3d68124f01bb4e52d Mon Sep 17 00:00:00 2001 From: ispiroglu Date: Fri, 20 Sep 2024 23:11:48 +0300 Subject: [PATCH 3/9] refactor: make logging on every request to server configurable from config --- modules/server/server.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/server/server.go b/modules/server/server.go index cd2b9d2..3649a22 100644 --- a/modules/server/server.go +++ b/modules/server/server.go @@ -115,7 +115,9 @@ func defaultFiber( app.Use(mw) } - app.Use(middlewares.Log()) + if serverCfg.GetBool("logging") { + app.Use(middlewares.Log()) + } for _, wrapper := range wrappers { app = wrapper(app) @@ -132,6 +134,7 @@ func setDefaultFiberConfigs(cfg *config.Config) { serverCfg.SetDefault("healthcheck.endpoints.readiness", "/__monitor/ready") serverCfg.SetDefault("readtimeout", "10s") serverCfg.SetDefault("writetimeout", "10s") + serverCfg.SetDefault("logging", false) } func getSwaggerDefs(rs []*registry) []swagger.EndpointDef { From cad7dafef0688d3188db79fc8507c6fbd18a4fb3 Mon Sep 17 00:00:00 2001 From: ispiroglu Date: Tue, 24 Sep 2024 23:03:17 +0300 Subject: [PATCH 4/9] refactor: change agentname key to x-agentname --- modules/common/ctxvaluer/valuer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/common/ctxvaluer/valuer.go b/modules/common/ctxvaluer/valuer.go index 2779ccd..8f8023a 100644 --- a/modules/common/ctxvaluer/valuer.go +++ b/modules/common/ctxvaluer/valuer.go @@ -13,7 +13,7 @@ const ( ExecutorUserKey = "x-executor-user" TraceIDKey = "trace-id" SpanIDKey = "span-id" - AgentNameKey = "x-agent-name" + AgentNameKey = "x-agentname" OwnerKey = "x-owner" ) From ed24cb41332a0ec8c87f9e987daedf31c0dae5eb Mon Sep 17 00:00:00 2001 From: ispiroglu Date: Wed, 25 Sep 2024 00:00:24 +0300 Subject: [PATCH 5/9] docs: add client documentation --- docs/modules/client.md | 149 ++++++++++++++++++ example/client-server/client/client.go | 4 +- .../client-server/client/client_wrapper.go | 30 ++++ example/client-server/client/error_decoder.go | 15 ++ modules/client/driver.go | 1 + modules/common/ctxvaluer/valuer.go | 5 +- 6 files changed, 201 insertions(+), 3 deletions(-) create mode 100644 docs/modules/client.md create mode 100644 example/client-server/client/client_wrapper.go create mode 100644 example/client-server/client/error_decoder.go diff --git a/docs/modules/client.md b/docs/modules/client.md new file mode 100644 index 0000000..dee6747 --- /dev/null +++ b/docs/modules/client.md @@ -0,0 +1,149 @@ +# Client + +The client module enables your application to interact with other web services over HTTP using the [Resty](https://github.com/go-resty/resty) library. + +This module simplifies the process of creating an HTTP client. It reads the base URL and timeout configurations from a config file. To create a client, you need to add the module to your application as follows: + +```go + + app := chaki.New() + + app.Use( + // ... + client.Module(), + // ... + ) + + //... +``` + +To create a client, you can use the following code: + +```go +type exampleClient struct { + *client.Base +} + +func newClient(f *client.Factory) *exampleClient { + return &exampleClient{ + Base: f.Get("example-client"), + } +} +``` + +the `example-client` name should match the name in the config file to configure the client from the config file: +```yaml +client: + example-client: + baseUrl: "http://super-duper-client-url.com" + timeout: 500ms +``` + +To create a request, you should use the following code. This ensures that tracing and other metadata are used on the request as all metadata is under context. + +```go +func (cl *exampleClient) SendHello(ctx context.Context) (string, error) { + resp := &response.Response[string]{} + + request := cl.Request(ctx) // this gives you an *resty.Request, to work with. + + if _, err := request. + SetResult(resp). + Get("/hello"); err != nil { + return "", err + } + + return resp.Data, nil +} +``` + +If you want to log every outgoing request and incoming response, you can simply set `logging` key to `true` on config. +```yaml + client: + example-client: + baseUrl: "http://super-duper-client-url.com" + timeout: 500ms + logging: true +``` +--- +## Error Handler + +By default, Chaki provides a built-in error handler to encapsulate incoming errors. The source code can be found in `modules/client/errors.go`. To avoid log chaos, error cases are not logged by default. + +To access the details of the errors, you can cast the error type into `GenericClientError` type as follows: +```go + + _, err := cl.SendSomeRequest() + genericError := client.GenericClientError{} + errors.As(err, genericError) + logger.From(ctx).Error(genericError.ClientName) + +``` + +### Providing error handler +You can provide a custom error handler to handle errors in a more specific way. The error handler function should accept a `context.Context` and a `*resty.Response` as parameters. +```go +func newClient(f *client.Factory) *exampleClient { + return &exampleClient{ + Base: f.Get("example-client", client.WithErrDecoder(customErrorDecoder)), + } +} + +func customErrorDecoder(_ context.Context, res *resty.Response) error { + if res.StatusCode() == 404 { + return fmt.Errorf("not found") + } + return nil +} +``` + +--- + +## Wrappers + +You can add wrappers to clients to extend their functionality. Chaki provides a default wrapper that adds the following headers to requests if the corresponding values are present in the context: +```go + CorrelationIDKey = "x-correlationId" + ExecutorUserKey = "x-executor-user" + AgentNameKey = "x-agentname" + OwnerKey = "x-owner" +``` + +### Providing an wrapper + +You can wrap the existing client as follows. +```go + + +type user struct { + publicUsername string + publicTag string +} + +func HeaderWrapper() client.DriverWrapper { + return func(restyClient *resty.Client) *resty.Client { + return restyClient.OnBeforeRequest(func(c *resty.Client, r *resty.Request) error { + ctx := r.Context() + + h := map[string]string{} + + if v := ctx.Value("user"); v != nil { + user := v.(user) + h["publicUsername"] = user.publicUsername + h["publicTag"] = user.publicTag + } + + r.SetHeaders(h) + return nil + }) + } +} + +func newClient(f *client.Factory) *exampleClient { + return &exampleClient{ + Base: f.Get("example-client", + client.WithDriverWrappers(HeaderWrapper())), + } +} + +``` diff --git a/example/client-server/client/client.go b/example/client-server/client/client.go index dcc2fb4..578ee58 100644 --- a/example/client-server/client/client.go +++ b/example/client-server/client/client.go @@ -14,7 +14,9 @@ type exampleClient struct { func newClient(f *client.Factory) *exampleClient { return &exampleClient{ - Base: f.Get("example-client"), + Base: f.Get("example-client", + client.WithErrDecoder(customErrorDecoder), + client.WithDriverWrappers(HeaderWrapper())), } } diff --git a/example/client-server/client/client_wrapper.go b/example/client-server/client/client_wrapper.go new file mode 100644 index 0000000..a40873f --- /dev/null +++ b/example/client-server/client/client_wrapper.go @@ -0,0 +1,30 @@ +package main + +import ( + "github.com/Trendyol/chaki/modules/client" + "github.com/go-resty/resty/v2" +) + +type user struct { + publicUsername string + publicTag string +} + +func HeaderWrapper() client.DriverWrapper { + return func(restyClient *resty.Client) *resty.Client { + return restyClient.OnBeforeRequest(func(c *resty.Client, r *resty.Request) error { + ctx := r.Context() + + h := map[string]string{} + + if v := ctx.Value("user"); v != nil { + user := v.(user) + h["publicUsername"] = user.publicUsername + h["publicTag"] = user.publicTag + } + + r.SetHeaders(h) + return nil + }) + } +} diff --git a/example/client-server/client/error_decoder.go b/example/client-server/client/error_decoder.go new file mode 100644 index 0000000..bd19de7 --- /dev/null +++ b/example/client-server/client/error_decoder.go @@ -0,0 +1,15 @@ +package main + +import ( + "context" + "fmt" + + "github.com/go-resty/resty/v2" +) + +func customErrorDecoder(_ context.Context, res *resty.Response) error { + if res.StatusCode() == 404 { + return fmt.Errorf("not found") + } + return nil +} diff --git a/modules/client/driver.go b/modules/client/driver.go index 84dd113..bffb8a4 100644 --- a/modules/client/driver.go +++ b/modules/client/driver.go @@ -20,6 +20,7 @@ func newDriverBuilder(cfg *config.Config) *driverBuilder { d := resty.New(). SetBaseURL(cfg.GetString("baseurl")). SetTimeout(cfg.GetDuration("timeout")). + // Debug mode provides a logging, but it's not in the same format with our logger. SetDebug(cfg.GetBool("debug")) return &driverBuilder{ diff --git a/modules/common/ctxvaluer/valuer.go b/modules/common/ctxvaluer/valuer.go index 8f8023a..97b1a96 100644 --- a/modules/common/ctxvaluer/valuer.go +++ b/modules/common/ctxvaluer/valuer.go @@ -11,10 +11,11 @@ import ( const ( CorrelationIDKey = "x-correlationId" ExecutorUserKey = "x-executor-user" - TraceIDKey = "trace-id" - SpanIDKey = "span-id" AgentNameKey = "x-agentname" OwnerKey = "x-owner" + + TraceIDKey = "trace-id" + SpanIDKey = "span-id" ) var ( From acd4d9d1de58d72d877da759fd95eace9407eda7 Mon Sep 17 00:00:00 2001 From: ispiroglu Date: Wed, 25 Sep 2024 14:25:02 +0300 Subject: [PATCH 6/9] feat: ignore '-' json tags in swagger. --- modules/swagger/defs.go | 2 +- modules/swagger/defs_test.go | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/modules/swagger/defs.go b/modules/swagger/defs.go index 55cf8e1..de642af 100644 --- a/modules/swagger/defs.go +++ b/modules/swagger/defs.go @@ -52,7 +52,7 @@ func buildModelDefinition(defs m, t reflect.Type, isReq bool) { buildModelDefinition(defs, ft.Elem(), isReq) } - if !isReq || f.Tag.Get("json") != "" { + if !isReq || f.Tag.Get("json") != "" && f.Tag.Get("json") != "-" { smp[getFieldName(f)] = getPropertyField(f.Type) if vts, ok := f.Tag.Lookup("validate"); isReq && ok { diff --git a/modules/swagger/defs_test.go b/modules/swagger/defs_test.go index efda066..76639dc 100644 --- a/modules/swagger/defs_test.go +++ b/modules/swagger/defs_test.go @@ -14,6 +14,7 @@ type testStruct struct { Field4 []anotherStruct `json:"field4"` Field5 *string `json:"field5"` Field6 map[string]int `json:"field6" validate:"required"` + Field7 string `json:"-"` } type anotherStruct struct { @@ -114,6 +115,10 @@ func TestBuildModelDefinition(t *testing.T) { t.Errorf("Expected field6 to be of type map") } } + + if _, ok := props.(m)["field7"]; ok { + t.Errorf("field7 is not expected in defs as it tagged with '-'") + } } else { t.Errorf("Expected properties to be a map, got %T", defs["testStruct"].(m)["properties"]) } From 47de00ed7a00bdeb181f52df804ed051fdae8c29 Mon Sep 17 00:00:00 2001 From: ispiroglu Date: Wed, 25 Sep 2024 14:41:53 +0300 Subject: [PATCH 7/9] refactor: does not wrap into response entity automatically in route. --- modules/server/route/route.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/server/route/route.go b/modules/server/route/route.go index 4af1ac2..1710f89 100644 --- a/modules/server/route/route.go +++ b/modules/server/route/route.go @@ -112,7 +112,7 @@ func build[Req, Res any](f HandlerFunc[Req, Res], defaultStatus ...int) fiber.Ha if rp, ok := any(res).(response.Responser); ok { resp = rp.ToResponse() } else { - resp = response.Success(res) + resp = res } if st, ok := any(res).(response.Stature); ok { From a98f43d087f3605856b935de68296e4486812f15 Mon Sep 17 00:00:00 2001 From: ispiroglu Date: Wed, 25 Sep 2024 16:25:03 +0300 Subject: [PATCH 8/9] feat: add uint and float support into swagger --- modules/swagger/defs_test.go | 31 +++++++++++++++++++++++++++++++ modules/swagger/json.go | 9 ++++++++- 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/modules/swagger/defs_test.go b/modules/swagger/defs_test.go index 76639dc..cf4ca0a 100644 --- a/modules/swagger/defs_test.go +++ b/modules/swagger/defs_test.go @@ -15,6 +15,8 @@ type testStruct struct { Field5 *string `json:"field5"` Field6 map[string]int `json:"field6" validate:"required"` Field7 string `json:"-"` + Field8 uint32 `json:"field8"` + Field9 float64 `json:"field9"` } type anotherStruct struct { @@ -119,6 +121,35 @@ func TestBuildModelDefinition(t *testing.T) { if _, ok := props.(m)["field7"]; ok { t.Errorf("field7 is not expected in defs as it tagged with '-'") } + + if p, ok := props.(m)["field8"]; !ok { + t.Errorf("Expected field8 in properties, not found") + } else { + if field, ok := p.(m); ok { + if field["type"] != "integer" { + t.Errorf("Expected field8 to be of type int") + } + + if field["format"] != "uint32" { + t.Errorf("Expected field8 to be of format uint32") + } + } + } + + if p, ok := props.(m)["field9"]; !ok { + t.Errorf("Expected field9 in properties, not found") + } else { + if field, ok := p.(m); ok { + if field["type"] != "number" { + t.Errorf("Expected field9 to be of type number") + } + + if field["format"] != "float64" { + t.Errorf("Expected field9 to be of format float64") + } + } + } + } else { t.Errorf("Expected properties to be a map, got %T", defs["testStruct"].(m)["properties"]) } diff --git a/modules/swagger/json.go b/modules/swagger/json.go index 742cde6..8d9cf83 100644 --- a/modules/swagger/json.go +++ b/modules/swagger/json.go @@ -74,13 +74,20 @@ func withDefinitionPrefix(s string) string { } func getPrimitiveType(t reflect.Type) m { - if kp := t.Kind().String(); strings.HasPrefix(kp, "int") { + if kp := t.Kind().String(); strings.HasPrefix(kp, "int") || strings.HasPrefix(kp, "uint") { return m{ "type": "integer", "format": kp, } } + if kp := t.Kind().String(); strings.HasPrefix(kp, "float") { + return m{ + "type": "number", + "format": kp, + } + } + k := t.Kind().String() if t.Kind() == reflect.Bool { From 3dea49422a55652d6840f187b21072ca9e7493e5 Mon Sep 17 00:00:00 2001 From: ispiroglu Date: Thu, 26 Sep 2024 10:37:16 +0300 Subject: [PATCH 9/9] chore: exclude test case from linter --- modules/swagger/defs_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/swagger/defs_test.go b/modules/swagger/defs_test.go index cf4ca0a..5b48a9a 100644 --- a/modules/swagger/defs_test.go +++ b/modules/swagger/defs_test.go @@ -51,7 +51,7 @@ func TestBuildDefinitions(t *testing.T) { } } -func TestBuildModelDefinition(t *testing.T) { +func TestBuildModelDefinition(t *testing.T) { //nolint:gocyclo mockType := reflect.TypeOf(testStruct{}) defs := make(m) @@ -149,7 +149,6 @@ func TestBuildModelDefinition(t *testing.T) { } } } - } else { t.Errorf("Expected properties to be a map, got %T", defs["testStruct"].(m)["properties"]) }