From e0446b096227733ccdef33b2cd223d530fc00d70 Mon Sep 17 00:00:00 2001 From: Piotr Piotrowski Date: Fri, 9 Dec 2022 19:29:34 +0100 Subject: [PATCH 01/14] Service API improvements --- services/errors.go | 8 +- services/service.go | 394 +++++++++++++++++++++++---------------- services/service_test.go | 309 +++++++++++++++++++++++++++--- 3 files changed, 522 insertions(+), 189 deletions(-) diff --git a/services/errors.go b/services/errors.go index 9ad1c5452..29f0d2979 100644 --- a/services/errors.go +++ b/services/errors.go @@ -15,11 +15,11 @@ package services import "fmt" -type ServiceAPIError struct { - ErrorCode int +type ResponseError struct { + ErrorCode string Description string } -func (e *ServiceAPIError) Error() string { - return fmt.Sprintf("%d %s", e.ErrorCode, e.Description) +func (e *ResponseError) Error() string { + return fmt.Sprintf("%s %s", e.ErrorCode, e.Description) } diff --git a/services/service.go b/services/service.go index e9e3a652e..e32bfbf09 100644 --- a/services/service.go +++ b/services/service.go @@ -17,6 +17,7 @@ import ( "encoding/json" "errors" "fmt" + "regexp" "strings" "sync" "time" @@ -38,35 +39,42 @@ type ( Name() string Description() string Version() string - Stats() ServiceStats + Stats() Stats Reset() - Stop() + Stop() error } - // A request handler. - // TODO (could make error more and return more info to user automatically?) - ServiceHandler func(svc Service, req *nats.Msg) error + // RequestHandler is a function used as a Handler for a service + RequestHandler func(*nats.Msg) + + ErrHandler func(Service, *Error) + + DoneHandler func(Service) // Clients can request as well. - ServiceStats struct { - Name string `json:"name"` - ID string `json:"id"` - Version string `json:"version"` - Started time.Time `json:"started"` - Endpoints []Stats `json:"stats"` + Stats struct { + Name string `json:"name"` + ID string `json:"id"` + Version string `json:"version"` + Endpoints []EndpointStats `json:"stats"` } - Stats struct { - Name string `json:"name"` - NumRequests int `json:"num_requests"` - NumErrors int `json:"num_errors"` - TotalLatency time.Duration `json:"total_latency"` - AverageLatency time.Duration `json:"average_latency"` - Data interface{} `json:"data"` + EndpointStats struct { + Name string `json:"name"` + NumRequests int `json:"num_requests"` + NumErrors int `json:"num_errors"` + TotalProcessingTime time.Duration `json:"total_processing_time"` + AverageProcessingTime time.Duration `json:"average_processing_time"` + Data interface{} `json:"data"` } - // ServiceInfo is the basic information about a service type - ServiceInfo struct { + Ping struct { + Name string `json:"name"` + ID string `json:"id"` + } + + // Info is the basic information about a service type + Info struct { Name string `json:"name"` ID string `json:"id"` Description string `json:"description"` @@ -74,90 +82,109 @@ type ( Subject string `json:"subject"` } - ServiceSchema struct { + Schema struct { Request string `json:"request"` Response string `json:"response"` } Endpoint struct { Subject string `json:"subject"` - Handler ServiceHandler + Handler RequestHandler } - InternalEndpoint struct { - Name string - Handler nats.MsgHandler - } + Verb int64 - ServiceVerb int64 + Config struct { + Name string `json:"name"` + Version string `json:"version"` + Description string `json:"description"` + Schema Schema `json:"schema"` + Endpoint Endpoint `json:"endpoint"` + StatsHandler func(Endpoint) interface{} + DoneHandler DoneHandler + ErrorHandler ErrHandler + } - ServiceConfig struct { - Name string `json:"name"` - Description string `json:"description"` - Version string `json:"version"` - Schema ServiceSchema `json:"schema"` - Endpoint Endpoint `json:"endpoint"` - StatusHandler func(Endpoint) interface{} + Error struct { + Subject string + Description string } // service is the internal implementation of a Service service struct { sync.Mutex - ServiceConfig - id string - // subs - reqSub *nats.Subscription - internal map[string]*nats.Subscription - statuses map[string]*Stats - stats *ServiceStats - conn *nats.Conn + Config + id string + reqSub *nats.Subscription + verbSubs map[string]*nats.Subscription + endpointStats map[string]*EndpointStats + conn *nats.Conn + natsHandlers handlers + stopped bool + } + + handlers struct { + closed nats.ConnHandler + asyncErr nats.ErrHandler } ) const ( - // We can fix this, as versions will be on separate subjects and use account mapping to roll requests to new versions etc. - QG = "svc" + // Queue Group name used across all services + QG = "q" - // ServiceApiPrefix is the root of all control subjects - ServiceApiPrefix = "$SRV" + // APIPrefix is the root of all control subjects + APIPrefix = "$SRV" +) - ServiceErrorHeader = "Nats-Service-Error" +// Service Error headers +const ( + ErrorHeader = "Nats-Service-Error" + ErrorCodeHeader = "Nats-Service-Error-Code" ) const ( - SrvPing ServiceVerb = iota - SrvStatus - SrvInfo - SrvSchema + PingVerb Verb = iota + StatsVerb + InfoVerb + SchemaVerb +) + +var ( + serviceNameRegexp = regexp.MustCompile(`^[A-Za-z0-9\-_]+$`) ) -func (s *ServiceConfig) Valid() error { - if s.Name == "" { - return errors.New("name is required") +var ( + ErrConfigValidation = errors.New("validation") + ErrVerbNotSupported = errors.New("unsupported verb") +) + +func (s *Config) Valid() error { + if !serviceNameRegexp.MatchString(s.Name) { + return fmt.Errorf("%w: service name: name should not be empty and should consist of alphanumerical charactest, dashes and underscores", ErrConfigValidation) } return s.Endpoint.Valid() } func (e *Endpoint) Valid() error { - s := strings.TrimSpace(e.Subject) - if len(s) == 0 { - return errors.New("subject is required") + if e.Subject == "" { + return fmt.Errorf("%w: endpoint: subject is required", ErrConfigValidation) } if e.Handler == nil { - return errors.New("handler is required") + return fmt.Errorf("%w: endpoint: handler is required", ErrConfigValidation) } return nil } -func (s ServiceVerb) String() string { +func (s Verb) String() string { switch s { - case SrvPing: + case PingVerb: return "PING" - case SrvStatus: - return "STATUS" - case SrvInfo: + case StatsVerb: + return "STATS" + case InfoVerb: return "INFO" - case SrvSchema: + case SchemaVerb: return "SCHEMA" default: return "" @@ -165,32 +192,28 @@ func (s ServiceVerb) String() string { } // Add adds a microservice. -// NOTE we can do an OpenAPI version as well, but looking at it it was very involved. So I think keep simple version and -// also have a version that talkes full blown OpenAPI spec and we can pull these things out. -func Add(nc *nats.Conn, config ServiceConfig) (Service, error) { +// It will enable internal common services (PING, STATS, INFO and SCHEMA) as well as +// the actual service handler on the subject provided in config.Endpoint +// A service name and Endpoint configuration are required to add a service. +// Add returns a [Service] interface, allowing service menagement. +// Each service is assigned a unique ID. +func Add(nc *nats.Conn, config Config) (Service, error) { if err := config.Valid(); err != nil { return nil, err } id := nuid.Next() svc := &service{ - ServiceConfig: config, - conn: nc, - id: id, + Config: config, + conn: nc, + id: id, } - svc.internal = make(map[string]*nats.Subscription) - svc.statuses = make(map[string]*Stats) - svc.statuses[""] = &Stats{ + svc.verbSubs = make(map[string]*nats.Subscription) + svc.endpointStats = make(map[string]*EndpointStats) + svc.endpointStats[""] = &EndpointStats{ Name: config.Name, } - svc.stats = &ServiceStats{ - Name: config.Name, - ID: id, - Version: config.Version, - Started: time.Now(), - } - // Setup internal subscriptions. var err error @@ -201,7 +224,7 @@ func Add(nc *nats.Conn, config ServiceConfig) (Service, error) { return nil, err } - info := &ServiceInfo{ + info := &Info{ Name: config.Name, ID: id, Description: config.Description, @@ -209,51 +232,98 @@ func Add(nc *nats.Conn, config ServiceConfig) (Service, error) { Subject: config.Endpoint.Subject, } + ping := &Ping{ + Name: config.Name, + ID: id, + } + infoHandler := func(m *nats.Msg) { - response, _ := json.MarshalIndent(info, "", " ") + response, _ := json.Marshal(info) m.Respond(response) } pingHandler := func(m *nats.Msg) { - infoHandler(m) + response, _ := json.Marshal(ping) + m.Respond(response) } statusHandler := func(m *nats.Msg) { - response, _ := json.MarshalIndent(svc.Stats(), "", " ") + response, _ := json.Marshal(svc.Stats()) m.Respond(response) } schemaHandler := func(m *nats.Msg) { - response, _ := json.MarshalIndent(svc.ServiceConfig.Schema, "", " ") + response, _ := json.Marshal(svc.Config.Schema) m.Respond(response) } - if err := svc.addInternalHandlerGroup(nc, SrvInfo, infoHandler); err != nil { + if err := svc.verbHandlers(nc, InfoVerb, infoHandler); err != nil { return nil, err } - if err := svc.addInternalHandlerGroup(nc, SrvPing, pingHandler); err != nil { + if err := svc.verbHandlers(nc, PingVerb, pingHandler); err != nil { return nil, err } - if err := svc.addInternalHandlerGroup(nc, SrvStatus, statusHandler); err != nil { + if err := svc.verbHandlers(nc, StatsVerb, statusHandler); err != nil { return nil, err } - if svc.ServiceConfig.Schema.Request != "" || svc.ServiceConfig.Schema.Response != "" { - if err := svc.addInternalHandlerGroup(nc, SrvSchema, schemaHandler); err != nil { + if svc.Config.Schema.Request != "" || svc.Config.Schema.Response != "" { + if err := svc.verbHandlers(nc, SchemaVerb, schemaHandler); err != nil { return nil, err } } - svc.stats.ID = id - svc.stats.Started = time.Now() + svc.natsHandlers.closed = nc.Opts.ClosedCB + if nc.Opts.ClosedCB != nil { + nc.SetClosedHandler(func(c *nats.Conn) { + svc.Stop() + if config.DoneHandler != nil { + config.DoneHandler(svc) + } + svc.natsHandlers.closed(c) + }) + } else { + nc.SetClosedHandler(func(c *nats.Conn) { + if err := svc.Stop(); err != nil { + } + if config.DoneHandler != nil { + config.DoneHandler(svc) + } + }) + } + + svc.natsHandlers.asyncErr = nc.Opts.AsyncErrorCB + if nc.Opts.AsyncErrorCB != nil { + nc.SetErrorHandler(func(c *nats.Conn, s *nats.Subscription, err error) { + if config.ErrorHandler != nil { + config.ErrorHandler(svc, &Error{ + Description: err.Error(), + Subject: s.Subject, + }) + } + svc.Stop() + svc.natsHandlers.asyncErr(c, s, err) + }) + } else { + nc.SetErrorHandler(func(c *nats.Conn, s *nats.Subscription, err error) { + if config.ErrorHandler != nil { + config.ErrorHandler(svc, &Error{ + Description: err.Error(), + Subject: s.Subject, + }) + } + svc.Stop() + }) + } + return svc, nil } -// addInternalHandlerGroup generates control handlers for a specific verb +// verbHandlers generates control handlers for a specific verb // each request generates 3 subscriptions, one for the general verb // affecting all services written with the framework, one that handles // all services of a particular kind, and finally a specific service. -func (svc *service) addInternalHandlerGroup(nc *nats.Conn, verb ServiceVerb, handler nats.MsgHandler) error { +func (svc *service) verbHandlers(nc *nats.Conn, verb Verb, handler nats.MsgHandler) error { name := fmt.Sprintf("%s-all", verb.String()) if err := svc.addInternalHandler(nc, verb, "", "", name, handler); err != nil { return err @@ -266,31 +336,30 @@ func (svc *service) addInternalHandlerGroup(nc *nats.Conn, verb ServiceVerb, han } // addInternalHandler registers a control subject handler -func (svc *service) addInternalHandler(nc *nats.Conn, verb ServiceVerb, kind, id, name string, handler nats.MsgHandler) error { - subj, err := SvcControlSubject(verb, kind, id) +func (svc *service) addInternalHandler(nc *nats.Conn, verb Verb, kind, id, name string, handler nats.MsgHandler) error { + subj, err := ControlSubject(verb, kind, id) if err != nil { svc.Stop() return err } - svc.internal[name], err = nc.Subscribe(subj, func(msg *nats.Msg) { + svc.verbSubs[name], err = nc.Subscribe(subj, func(msg *nats.Msg) { start := time.Now() - defer func() { - svc.Lock() - stats := svc.statuses[name] - stats.NumRequests++ - stats.TotalLatency += time.Since(start) - stats.AverageLatency = stats.TotalLatency / time.Duration(stats.NumRequests) - svc.Unlock() - }() handler(msg) + + svc.Lock() + stats := svc.endpointStats[name] + stats.NumRequests++ + stats.TotalProcessingTime += time.Since(start) + stats.AverageProcessingTime = stats.TotalProcessingTime / time.Duration(stats.NumRequests) + svc.Unlock() }) if err != nil { svc.Stop() return err } - svc.statuses[name] = &Stats{ + svc.endpointStats[name] = &EndpointStats{ Name: name, } return nil @@ -299,48 +368,48 @@ func (svc *service) addInternalHandler(nc *nats.Conn, verb ServiceVerb, kind, id // reqHandler itself func (svc *service) reqHandler(req *nats.Msg) { start := time.Now() - defer func() { - svc.Lock() - stats := svc.statuses[""] - stats.NumRequests++ - stats.TotalLatency += time.Since(start) - stats.AverageLatency = stats.TotalLatency / time.Duration(stats.NumRequests) - svc.Unlock() - }() + svc.Config.Endpoint.Handler(req) + svc.Lock() + stats := svc.endpointStats[""] + stats.NumRequests++ + stats.TotalProcessingTime += time.Since(start) + stats.AverageProcessingTime = stats.TotalProcessingTime / time.Duration(stats.NumRequests) - if err := svc.ServiceConfig.Endpoint.Handler(svc, req); err != nil { - hdr := make(nats.Header) - apiErr := &ServiceAPIError{} - if ok := errors.As(err, &apiErr); !ok { - hdr[ServiceErrorHeader] = []string{fmt.Sprintf("%d %s", 500, err.Error())} - } else { - hdr[ServiceErrorHeader] = []string{apiErr.Error()} - } - svc.Lock() - stats := svc.statuses[""] + if req.Header.Get(ErrorHeader) != "" { + stats := svc.endpointStats[""] stats.NumErrors++ - svc.Unlock() - - svc.conn.PublishMsg(&nats.Msg{ - Subject: req.Reply, - Header: hdr, - }) } + svc.Unlock() } -func (svc *service) Stop() { +func (svc *service) Stop() error { + if svc.stopped { + return nil + } if svc.reqSub != nil { - svc.reqSub.Drain() + if err := svc.reqSub.Drain(); err != nil { + return fmt.Errorf("draining subsctioption for request handler: %w", err) + } svc.reqSub = nil } var keys []string - for key, sub := range svc.internal { + for key, sub := range svc.verbSubs { keys = append(keys, key) - sub.Drain() + if err := sub.Drain(); err != nil { + return fmt.Errorf("draining subsctioption for subject %q: %w", sub.Subject, err) + } } for _, key := range keys { - delete(svc.internal, key) + delete(svc.verbSubs, key) } + restoreAsyncHandlers(svc.conn, svc.natsHandlers) + svc.stopped = true + return nil +} + +func restoreAsyncHandlers(nc *nats.Conn, handlers handlers) { + nc.SetClosedHandler(handlers.closed) + nc.SetErrorHandler(handlers.asyncErr) } func (svc *service) ID() string { @@ -348,57 +417,70 @@ func (svc *service) ID() string { } func (svc *service) Name() string { - return svc.ServiceConfig.Name + return svc.Config.Name } func (svc *service) Description() string { - return svc.ServiceConfig.Description + return svc.Config.Description } func (svc *service) Version() string { - return svc.ServiceConfig.Version + return svc.Config.Version } -func (svc *service) Stats() ServiceStats { +func (svc *service) Stats() Stats { svc.Lock() defer func() { svc.Unlock() }() - if svc.ServiceConfig.StatusHandler != nil { - stats := svc.statuses[""] - stats.Data = svc.ServiceConfig.StatusHandler(svc.Endpoint) + if svc.Config.StatsHandler != nil { + stats := svc.endpointStats[""] + stats.Data = svc.Config.StatsHandler(svc.Endpoint) } idx := 0 - v := make([]Stats, len(svc.statuses)) - for _, se := range svc.statuses { + v := make([]EndpointStats, len(svc.endpointStats)) + for _, se := range svc.endpointStats { v[idx] = *se idx++ } - svc.stats.Endpoints = v - return *svc.stats + return Stats{ + Name: svc.Name(), + ID: svc.ID(), + Version: svc.Version(), + Endpoints: v, + } } func (svc *service) Reset() { - for _, se := range svc.statuses { + for _, se := range svc.endpointStats { se.NumRequests = 0 - se.TotalLatency = 0 + se.TotalProcessingTime = 0 se.NumErrors = 0 se.Data = nil } } -// SvcControlSubject returns monitoring subjects used by the ServiceImpl -func SvcControlSubject(verb ServiceVerb, kind, id string) (string, error) { - sverb := verb.String() - if sverb == "" { - return "", fmt.Errorf("unsupported service verb") +// ControlSubject returns monitoring subjects used by the Service +func ControlSubject(verb Verb, kind, id string) (string, error) { + verbStr := verb.String() + if verbStr == "" { + return "", fmt.Errorf("%w: %q", ErrVerbNotSupported, verbStr) } kind = strings.ToUpper(kind) if kind == "" && id == "" { - return fmt.Sprintf("%s.%s", ServiceApiPrefix, sverb), nil + return fmt.Sprintf("%s.%s", APIPrefix, verbStr), nil } if id == "" { - return fmt.Sprintf("%s.%s.%s", ServiceApiPrefix, sverb, kind), nil + return fmt.Sprintf("%s.%s.%s", APIPrefix, verbStr, kind), nil + } + return fmt.Sprintf("%s.%s.%s.%s", APIPrefix, verbStr, kind, id), nil +} + +func SendError(req *nats.Msg, err ResponseError) { + if req.Header == nil { + req.Header = nats.Header{} } - return fmt.Sprintf("%s.%s.%s.%s", ServiceApiPrefix, sverb, kind, id), nil + req.Header.Add(ErrorHeader, err.Description) + req.Header.Add(ErrorCodeHeader, err.ErrorCode) + req.RespondMsg(req) } diff --git a/services/service_test.go b/services/service_test.go index 532be8915..7bf1437a7 100644 --- a/services/service_test.go +++ b/services/service_test.go @@ -15,6 +15,7 @@ package services import ( "encoding/json" + "errors" "fmt" "math/rand" "testing" @@ -36,23 +37,24 @@ func TestServiceBasics(t *testing.T) { defer nc.Close() // Stub service. - doAdd := func(svc Service, req *nats.Msg) error { + doAdd := func(req *nats.Msg) { if rand.Intn(10) == 0 { - return fmt.Errorf("Unexpected Error!") + SendError(req, ResponseError{"500", "Unexpected error!"}) + return } // Happy Path. // Random delay between 5-10ms time.Sleep(5*time.Millisecond + time.Duration(rand.Intn(5))*time.Millisecond) if err := req.Respond([]byte("42")); err != nil { - return err + SendError(req, ResponseError{"500", "Unexpected error!"}) + return } - return nil } var svcs []Service // Create 5 service responders. - config := ServiceConfig{ + config := Config{ Name: "CoolAddService", Version: "v0.1", Description: "Add things together", @@ -60,7 +62,7 @@ func TestServiceBasics(t *testing.T) { Subject: "svc.add", Handler: doAdd, }, - Schema: ServiceSchema{Request: "", Response: ""}, + Schema: Schema{Request: "", Response: ""}, } for i := 0; i < 5; i++ { @@ -91,7 +93,7 @@ func TestServiceBasics(t *testing.T) { // Make sure we can request info, 1 response. // This could be exported as well as main ServiceImpl. - subj, err := SvcControlSubject(SrvInfo, "CoolAddService", "") + subj, err := ControlSubject(InfoVerb, "CoolAddService", "") if err != nil { t.Fatalf("Failed to building info subject %v", err) } @@ -99,7 +101,7 @@ func TestServiceBasics(t *testing.T) { if err != nil { t.Fatalf("Expected a response, got %v", err) } - var inf ServiceInfo + var inf Info if err := json.Unmarshal(info.Data, &inf); err != nil { t.Fatalf("Unexpected error: %v", err) } @@ -108,13 +110,12 @@ func TestServiceBasics(t *testing.T) { } // Ping all services. Multiple responses. - // could do STATZ too? inbox := nats.NewInbox() sub, err := nc.SubscribeSync(inbox) if err != nil { t.Fatalf("subscribe failed: %s", err) } - pingSubject, err := SvcControlSubject(SrvPing, "CoolAddService", "") + pingSubject, err := ControlSubject(PingVerb, "CoolAddService", "") if err != nil { t.Fatalf("Unexpected error: %v", err) } @@ -140,7 +141,7 @@ func TestServiceBasics(t *testing.T) { if err != nil { t.Fatalf("subscribe failed: %s", err) } - statsSubject, err := SvcControlSubject(SrvStatus, "CoolAddService", "") + statsSubject, err := ControlSubject(StatsVerb, "CoolAddService", "") if err != nil { t.Fatalf("Unexpected error: %v", err) } @@ -148,14 +149,14 @@ func TestServiceBasics(t *testing.T) { t.Fatalf("Unexpected error: %v", err) } - stats := make([]ServiceStats, 0) + stats := make([]Stats, 0) var requestsNum int for { resp, err := sub.NextMsg(250 * time.Millisecond) if err != nil { break } - var srvStats ServiceStats + var srvStats Stats if err := json.Unmarshal(resp.Data, &srvStats); err != nil { t.Fatalf("Unexpected error: %v", err) } @@ -179,26 +180,272 @@ func TestServiceBasics(t *testing.T) { } } +func TestAddService(t *testing.T) { + testHandler := func(*nats.Msg) {} + var errNats, errService, closedNats, doneService chan struct{} + + tests := []struct { + name string + givenConfig Config + natsClosedHandler nats.ConnHandler + natsErrorHandler nats.ErrHandler + expectedPing Ping + withError error + }{ + { + name: "minimal config", + givenConfig: Config{ + Name: "test_service", + Endpoint: Endpoint{ + Subject: "test.sub", + Handler: testHandler, + }, + }, + expectedPing: Ping{ + Name: "test_service", + }, + }, + { + name: "with done handler, no handlers on nats connection", + givenConfig: Config{ + Name: "test_service", + Endpoint: Endpoint{ + Subject: "test.sub", + Handler: testHandler, + }, + DoneHandler: func(s Service) { + doneService <- struct{}{} + }, + }, + expectedPing: Ping{ + Name: "test_service", + }, + }, + { + name: "with error handler, no handlers on nats connection", + givenConfig: Config{ + Name: "test_service", + Endpoint: Endpoint{ + Subject: "test.sub", + Handler: testHandler, + }, + ErrorHandler: func(s Service, err *Error) { + errService <- struct{}{} + }, + }, + expectedPing: Ping{ + Name: "test_service", + }, + }, + { + name: "with done handler, append to nats handlers", + givenConfig: Config{ + Name: "test_service", + Endpoint: Endpoint{ + Subject: "test.sub", + Handler: testHandler, + }, + DoneHandler: func(s Service) { + doneService <- struct{}{} + }, + }, + natsClosedHandler: func(c *nats.Conn) { + closedNats <- struct{}{} + }, + natsErrorHandler: func(*nats.Conn, *nats.Subscription, error) { + errNats <- struct{}{} + }, + expectedPing: Ping{ + Name: "test_service", + }, + }, + { + name: "with error handler, append to nats handlers", + givenConfig: Config{ + Name: "test_service", + Endpoint: Endpoint{ + Subject: "test.sub", + Handler: testHandler, + }, + DoneHandler: func(s Service) { + doneService <- struct{}{} + }, + }, + natsClosedHandler: func(c *nats.Conn) { + closedNats <- struct{}{} + }, + natsErrorHandler: func(*nats.Conn, *nats.Subscription, error) { + errNats <- struct{}{} + }, + expectedPing: Ping{ + Name: "test_service", + }, + }, + { + name: "validation error, invalid service name", + givenConfig: Config{ + Name: "test_service!", + Endpoint: Endpoint{ + Subject: "test.sub", + Handler: testHandler, + }, + }, + withError: ErrConfigValidation, + }, + { + name: "validation error, empty subject", + givenConfig: Config{ + Name: "test_service", + Endpoint: Endpoint{ + Subject: "", + Handler: testHandler, + }, + }, + withError: ErrConfigValidation, + }, + { + name: "validation error, no handler", + givenConfig: Config{ + Name: "test_service", + Endpoint: Endpoint{ + Subject: "test_subject", + Handler: nil, + }, + }, + withError: ErrConfigValidation, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + s := RunServerOnPort(-1) + defer s.Shutdown() + + nc, err := nats.Connect(s.ClientURL(), + nats.ErrorHandler(test.natsErrorHandler), + nats.ClosedHandler(test.natsClosedHandler), + ) + if err != nil { + t.Fatalf("Expected to connect to server, got %v", err) + } + defer nc.Close() + + errNats = make(chan struct{}) + errService = make(chan struct{}) + closedNats = make(chan struct{}) + doneService = make(chan struct{}) + + srv, err := Add(nc, test.givenConfig) + if test.withError != nil { + if !errors.Is(err, test.withError) { + t.Fatalf("Expected error: %v; got: %v", test.withError, err) + } + return + } + + pingSubject, err := ControlSubject(PingVerb, srv.Name(), srv.ID()) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + pingResp, err := nc.Request(pingSubject, nil, 1*time.Second) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + var ping Ping + if err := json.Unmarshal(pingResp.Data, &ping); err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + test.expectedPing.ID = srv.ID() + if test.expectedPing != ping { + t.Fatalf("Invalid ping response; want: %+v; got: %+v", test.expectedPing, ping) + } + + if test.givenConfig.DoneHandler != nil { + go nc.Opts.ClosedCB(nc) + select { + case <-doneService: + case <-time.After(1 * time.Second): + t.Fatalf("Timeout on DoneHandler") + } + if test.natsClosedHandler != nil { + select { + case <-closedNats: + case <-time.After(1 * time.Second): + t.Fatalf("Timeout on ClosedHandler") + } + } + } + + if test.givenConfig.ErrorHandler != nil { + go nc.Opts.AsyncErrorCB(nc, &nats.Subscription{Subject: "test.sub"}, fmt.Errorf("oops")) + select { + case <-errService: + case <-time.After(1 * time.Second): + t.Fatalf("Timeout on ErrorHandler") + } + if test.natsErrorHandler != nil { + select { + case <-errNats: + case <-time.After(1 * time.Second): + t.Fatalf("Timeout on AsyncErrHandler") + } + } + } + + srv.Stop() + if test.natsClosedHandler != nil { + go nc.Opts.ClosedCB(nc) + select { + case <-doneService: + t.Fatalf("Expected to restore nats closed handler") + case <-time.After(50 * time.Millisecond): + } + select { + case <-closedNats: + case <-time.After(1 * time.Second): + t.Fatalf("Timeout on ClosedHandler") + } + } + if test.natsErrorHandler != nil { + go nc.Opts.AsyncErrorCB(nc, &nats.Subscription{Subject: "test.sub"}, fmt.Errorf("oops")) + select { + case <-errService: + t.Fatalf("Expected to restore nats error handler") + case <-time.After(50 * time.Millisecond): + } + select { + case <-errNats: + case <-time.After(1 * time.Second): + t.Fatalf("Timeout on AsyncErrHandler") + } + } + }) + } +} + func TestServiceErrors(t *testing.T) { tests := []struct { name string - handlerResponse error - expectedStatus string + handlerResponse *ResponseError + expectedMessage string + expectedCode string }{ { name: "generic error", - handlerResponse: fmt.Errorf("oops"), - expectedStatus: "500 oops", + handlerResponse: &ResponseError{ErrorCode: "500", Description: "oops"}, + expectedMessage: "oops", + expectedCode: "500", }, { name: "api error", - handlerResponse: &ServiceAPIError{ErrorCode: 400, Description: "oops"}, - expectedStatus: "400 oops", + handlerResponse: &ResponseError{ErrorCode: "400", Description: "oops"}, + expectedMessage: "oops", + expectedCode: "400", }, { name: "no error", handlerResponse: nil, - expectedStatus: "", }, } @@ -214,16 +461,16 @@ func TestServiceErrors(t *testing.T) { defer nc.Close() // Stub service. - handler := func(svc Service, req *nats.Msg) error { + handler := func(req *nats.Msg) { if test.handlerResponse == nil { - if err := req.Respond([]byte("ok")); err != nil { - return err - } + req.Respond([]byte("ok")) + return } - return test.handlerResponse + + SendError(req, *test.handlerResponse) } - svc, err := Add(nc, ServiceConfig{ + svc, err := Add(nc, Config{ Name: "CoolService", Description: "Erroring service", Endpoint: Endpoint{ @@ -241,9 +488,13 @@ func TestServiceErrors(t *testing.T) { t.Fatalf("request error") } - status := resp.Header.Get("Nats-Service-Error") - if status != test.expectedStatus { - t.Fatalf("Invalid response status; want: %q; got: %q", test.expectedStatus, status) + description := resp.Header.Get("Nats-Service-Error") + if description != test.expectedMessage { + t.Fatalf("Invalid response message; want: %q; got: %q", test.expectedMessage, description) + } + code := resp.Header.Get("Nats-Service-Error-Code") + if code != test.expectedCode { + t.Fatalf("Invalid response code; want: %q; got: %q", test.expectedCode, code) } }) } From 11dd38614651a5695bde106da743623de0dd955b Mon Sep 17 00:00:00 2001 From: Piotr Piotrowski Date: Mon, 12 Dec 2022 13:10:08 +0100 Subject: [PATCH 02/14] Add examples and documentation --- service/example_package_test.go | 70 ++++++++++++ service/example_test.go | 66 +++++++++++ service/request.go | 61 ++++++++++ {services => service}/service.go | 159 ++++++++++++++++---------- {services => service}/service_test.go | 155 ++++++++++++++++++++----- services/errors.go | 25 ---- 6 files changed, 419 insertions(+), 117 deletions(-) create mode 100644 service/example_package_test.go create mode 100644 service/example_test.go create mode 100644 service/request.go rename {services => service}/service.go (71%) rename {services => service}/service_test.go (74%) delete mode 100644 services/errors.go diff --git a/service/example_package_test.go b/service/example_package_test.go new file mode 100644 index 000000000..a5019a7b5 --- /dev/null +++ b/service/example_package_test.go @@ -0,0 +1,70 @@ +package service + +import ( + "fmt" + "log" + "strconv" + "time" + + "github.com/nats-io/nats.go" +) + +func Example() { + s := RunServerOnPort(-1) + defer s.Shutdown() + + nc, err := nats.Connect(s.ClientURL()) + if err != nil { + log.Fatal(err) + } + defer nc.Close() + + // Service handler is a function which takes *service.Request as argument. + // req.Respond or req.Error should be used to respond to the request. + incrementHandler := func(req *Request) { + val, err := strconv.Atoi(string(req.Data)) + if err != nil { + req.Error("400", "request data should be a number") + return + } + + responseData := val + 1 + req.Respond([]byte(strconv.Itoa(responseData))) + } + + config := Config{ + Name: "IncrementService", + Version: "v0.1.0", + Description: "Increment numbers", + Endpoint: Endpoint{ + // service handler + Handler: incrementHandler, + // a unique subject serving as a service endpoint + Subject: "numbers.increment", + }, + } + // Multiple instances of the servcice with the same name can be created. + // Requests to a service with the same name will be load-balanced. + for i := 0; i < 5; i++ { + svc, err := Add(nc, config) + if err != nil { + log.Fatal(err) + } + defer svc.Stop() + } + + // send a request to a service + resp, err := nc.Request("numbers.increment", []byte("3"), 1*time.Second) + if err != nil { + log.Fatal(err) + } + responseVal, err := strconv.Atoi(string(resp.Data)) + if err != nil { + log.Fatal(err) + } + fmt.Println(responseVal) + + // + // Output: 4 + // +} diff --git a/service/example_test.go b/service/example_test.go new file mode 100644 index 000000000..715e84a20 --- /dev/null +++ b/service/example_test.go @@ -0,0 +1,66 @@ +package service + +import ( + "fmt" + "log" + + "github.com/nats-io/nats.go" +) + +func ExampleAdd() { + nc, err := nats.Connect("127.0.0.1:4222") + if err != nil { + log.Fatal(err) + } + defer nc.Close() + + echoHandler := func(req *Request) { + req.Respond(req.Data) + } + + config := Config{ + Name: "EchoService", + Version: "v1.0.0", + Description: "Send back what you receive", + Endpoint: Endpoint{ + Subject: "echo", + Handler: echoHandler, + }, + + // DoneHandler can be set to customize behavior on stopping a service. + DoneHandler: func(srv Service) { + fmt.Printf("stopped service %q with ID %q\n", srv.Name(), srv.ID()) + }, + + // ErrorHandler can be used to customize behavior on service execution error. + ErrorHandler: func(srv Service, err *NATSError) { + fmt.Printf("Service %q returned an error on subject %q: %s", srv.Name(), err.Subject, err.Description) + }, + } + + srv, err := Add(nc, config) + if err != nil { + log.Fatal(err) + } + defer srv.Stop() +} + +func ExampleControlSubject() { + + // subject used to get PING from all services + subjectPINGAll, _ := ControlSubject(PingVerb, "", "") + fmt.Println(subjectPINGAll) + + // subject used to get PING from services with provided name + subjectPINGName, _ := ControlSubject(PingVerb, "CoolService", "") + fmt.Println(subjectPINGName) + + // subject used to get PING from a service with provided name and ID + subjectPINGInstance, _ := ControlSubject(PingVerb, "CoolService", "123") + fmt.Println(subjectPINGInstance) + + // Output: + // $SRV.PING + // $SRV.PING.COOLSERVICE + // $SRV.PING.COOLSERVICE.123 +} diff --git a/service/request.go b/service/request.go new file mode 100644 index 000000000..72cc7f345 --- /dev/null +++ b/service/request.go @@ -0,0 +1,61 @@ +package service + +import ( + "encoding/json" + "errors" + "fmt" + + "github.com/nats-io/nats.go" +) + +type ( + Request struct { + *nats.Msg + errResponse bool + } + + // RequestHandler is a function used as a Handler for a service. + RequestHandler func(*Request) +) + +var ( + ErrRespond = errors.New("NATS error when sending response") + ErrMarshalResponse = errors.New("marshaling response") + ErrArgRequired = errors.New("argument required") +) + +func (r *Request) Respond(response []byte) error { + if err := r.Msg.Respond(response); err != nil { + return fmt.Errorf("%w: %s", ErrRespond, err) + } + + return nil +} + +func (r *Request) RespondJSON(response interface{}) error { + resp, err := json.Marshal(response) + if err != nil { + return ErrMarshalResponse + } + + return r.Respond(resp) +} + +// Error prepares and publishes error response from a handler. +// A response error should be set containing an error code and description. +func (r *Request) Error(code, description string) error { + if code == "" { + return fmt.Errorf("%w: error code", ErrArgRequired) + } + if description == "" { + return fmt.Errorf("%w: description", ErrArgRequired) + } + response := &nats.Msg{ + Header: nats.Header{ + ErrorHeader: []string{description}, + ErrorCodeHeader: []string{code}, + }, + } + r.errResponse = true + return r.RespondMsg(response) +} diff --git a/services/service.go b/service/service.go similarity index 71% rename from services/service.go rename to service/service.go index e32bfbf09..9ce5dbae6 100644 --- a/services/service.go +++ b/service/service.go @@ -11,7 +11,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package services +package service import ( "encoding/json" @@ -35,23 +35,42 @@ type ( // Service is an interface for service management. // It exposes methods to stop/reset a service, as well as get information on a service. Service interface { + // ID returns the service instance's unique ID. ID() string + + // Name returns the name of the service. + // It can be shared between multiple service instances. Name() string + + // Description returns the service description. Description() string + + // Version returns the service version. Version() string + + // Stats returns statisctics for the service endpoint and all monitoring endpoints. Stats() Stats + + // Reset resets all statistics on a service instance. Reset() + + // Stop drains the endpoint subscriptions and marks the service as stopped. Stop() error } - // RequestHandler is a function used as a Handler for a service - RequestHandler func(*nats.Msg) - - ErrHandler func(Service, *Error) + // ErrHandler is a function used to configure a custom error handler for a service, + ErrHandler func(Service, *NATSError) + // DoneHandler is a function used to configure a custom done handler for a service. DoneHandler func(Service) - // Clients can request as well. + // StatsHandleris a function used to configure a custom STATS endpoint. + // It should return a value which can be serialized to JSON. + StatsHandler func(Endpoint) interface{} + + // Stats is the type returned by STATS monitoring endpoint. + // It contains a slice of [EndpointStats], providing stats + // for both the service handler as well as all monitoring subjects. Stats struct { Name string `json:"name"` ID string `json:"id"` @@ -59,6 +78,8 @@ type ( Endpoints []EndpointStats `json:"stats"` } + // EndpointStats are stats for a specific endpoint (either request handler or monitoring enpoints). + // It contains general statisctics for an endpoint, as well as a custom value returned by optional [StatsHandler]. EndpointStats struct { Name string `json:"name"` NumRequests int `json:"num_requests"` @@ -68,12 +89,13 @@ type ( Data interface{} `json:"data"` } + // Ping is the response type for PING monitoring endpoint. Ping struct { Name string `json:"name"` ID string `json:"id"` } - // Info is the basic information about a service type + // Info is the basic information about a service type. Info struct { Name string `json:"name"` ID string `json:"id"` @@ -82,30 +104,38 @@ type ( Subject string `json:"subject"` } + // Schema can be used to configure a schema for a service. + // It is olso returned by the SCHEMA monitoring service (if set). Schema struct { Request string `json:"request"` Response string `json:"response"` } + // Endpoint is used to configure a subject and handler for a service. Endpoint struct { Subject string `json:"subject"` Handler RequestHandler } + // Verb represents a name of the monitoring service. Verb int64 + // Config is a configuration of a service. Config struct { Name string `json:"name"` Version string `json:"version"` Description string `json:"description"` Schema Schema `json:"schema"` Endpoint Endpoint `json:"endpoint"` - StatsHandler func(Endpoint) interface{} + StatsHandler StatsHandler DoneHandler DoneHandler ErrorHandler ErrHandler } - Error struct { + // NATSError represents an error returned by a NATS Subscription. + // It contains a subject on which the subscription failed, so that + // it can be linked with a specific service endpoint. + NATSError struct { Subject string Description string } @@ -143,6 +173,7 @@ const ( ErrorCodeHeader = "Nats-Service-Error-Code" ) +// Verbs being used to set up a specific control subject. const ( PingVerb Verb = iota StatsVerb @@ -154,28 +185,15 @@ var ( serviceNameRegexp = regexp.MustCompile(`^[A-Za-z0-9\-_]+$`) ) +// Common errors returned by the Service framework. var ( + // ErrConfigValidation is returned when service configuration is invalid ErrConfigValidation = errors.New("validation") + + // ErrVerbNotSupported is returned when invalid [Verb] is used (PING, SCHEMA, INFO, STATS) ErrVerbNotSupported = errors.New("unsupported verb") ) -func (s *Config) Valid() error { - if !serviceNameRegexp.MatchString(s.Name) { - return fmt.Errorf("%w: service name: name should not be empty and should consist of alphanumerical charactest, dashes and underscores", ErrConfigValidation) - } - return s.Endpoint.Valid() -} - -func (e *Endpoint) Valid() error { - if e.Subject == "" { - return fmt.Errorf("%w: endpoint: subject is required", ErrConfigValidation) - } - if e.Handler == nil { - return fmt.Errorf("%w: endpoint: handler is required", ErrConfigValidation) - } - return nil -} - func (s Verb) String() string { switch s { case PingVerb: @@ -198,7 +216,7 @@ func (s Verb) String() string { // Add returns a [Service] interface, allowing service menagement. // Each service is assigned a unique ID. func Add(nc *nats.Conn, config Config) (Service, error) { - if err := config.Valid(); err != nil { + if err := config.valid(); err != nil { return nil, err } @@ -218,7 +236,7 @@ func Add(nc *nats.Conn, config Config) (Service, error) { var err error svc.reqSub, err = nc.QueueSubscribe(config.Endpoint.Subject, QG, func(m *nats.Msg) { - svc.reqHandler(m) + svc.reqHandler(&Request{Msg: m}) }) if err != nil { return nil, err @@ -277,18 +295,11 @@ func Add(nc *nats.Conn, config Config) (Service, error) { if nc.Opts.ClosedCB != nil { nc.SetClosedHandler(func(c *nats.Conn) { svc.Stop() - if config.DoneHandler != nil { - config.DoneHandler(svc) - } svc.natsHandlers.closed(c) }) } else { nc.SetClosedHandler(func(c *nats.Conn) { - if err := svc.Stop(); err != nil { - } - if config.DoneHandler != nil { - config.DoneHandler(svc) - } + svc.Stop() }) } @@ -296,7 +307,7 @@ func Add(nc *nats.Conn, config Config) (Service, error) { if nc.Opts.AsyncErrorCB != nil { nc.SetErrorHandler(func(c *nats.Conn, s *nats.Subscription, err error) { if config.ErrorHandler != nil { - config.ErrorHandler(svc, &Error{ + config.ErrorHandler(svc, &NATSError{ Description: err.Error(), Subject: s.Subject, }) @@ -307,7 +318,7 @@ func Add(nc *nats.Conn, config Config) (Service, error) { } else { nc.SetErrorHandler(func(c *nats.Conn, s *nats.Subscription, err error) { if config.ErrorHandler != nil { - config.ErrorHandler(svc, &Error{ + config.ErrorHandler(svc, &NATSError{ Description: err.Error(), Subject: s.Subject, }) @@ -319,10 +330,27 @@ func Add(nc *nats.Conn, config Config) (Service, error) { return svc, nil } -// verbHandlers generates control handlers for a specific verb -// each request generates 3 subscriptions, one for the general verb +func (s *Config) valid() error { + if !serviceNameRegexp.MatchString(s.Name) { + return fmt.Errorf("%w: service name: name should not be empty and should consist of alphanumerical charactest, dashes and underscores", ErrConfigValidation) + } + return s.Endpoint.valid() +} + +func (e *Endpoint) valid() error { + if e.Subject == "" { + return fmt.Errorf("%w: endpoint: subject is required", ErrConfigValidation) + } + if e.Handler == nil { + return fmt.Errorf("%w: endpoint: handler is required", ErrConfigValidation) + } + return nil +} + +// verbHandlers generates control handlers for a specific verb. +// Each request generates 3 subscriptions, one for the general verb // affecting all services written with the framework, one that handles -// all services of a particular kind, and finally a specific service. +// all services of a particular kind, and finally a specific service instance. func (svc *service) verbHandlers(nc *nats.Conn, verb Verb, handler nats.MsgHandler) error { name := fmt.Sprintf("%s-all", verb.String()) if err := svc.addInternalHandler(nc, verb, "", "", name, handler); err != nil { @@ -335,7 +363,7 @@ func (svc *service) verbHandlers(nc *nats.Conn, verb Verb, handler nats.MsgHandl return svc.addInternalHandler(nc, verb, svc.Name(), svc.ID(), verb.String(), handler) } -// addInternalHandler registers a control subject handler +// addInternalHandler registers a control subject handler. func (svc *service) addInternalHandler(nc *nats.Conn, verb Verb, kind, id, name string, handler nats.MsgHandler) error { subj, err := ControlSubject(verb, kind, id) if err != nil { @@ -366,7 +394,7 @@ func (svc *service) addInternalHandler(nc *nats.Conn, verb Verb, kind, id, name } // reqHandler itself -func (svc *service) reqHandler(req *nats.Msg) { +func (svc *service) reqHandler(req *Request) { start := time.Now() svc.Config.Endpoint.Handler(req) svc.Lock() @@ -375,20 +403,21 @@ func (svc *service) reqHandler(req *nats.Msg) { stats.TotalProcessingTime += time.Since(start) stats.AverageProcessingTime = stats.TotalProcessingTime / time.Duration(stats.NumRequests) - if req.Header.Get(ErrorHeader) != "" { + if req.errResponse { stats := svc.endpointStats[""] stats.NumErrors++ } svc.Unlock() } +// Stop drains the endpoint subscriptions and marks the service as stopped. func (svc *service) Stop() error { if svc.stopped { return nil } if svc.reqSub != nil { if err := svc.reqSub.Drain(); err != nil { - return fmt.Errorf("draining subsctioption for request handler: %w", err) + return fmt.Errorf("draining subscription for request handler: %w", err) } svc.reqSub = nil } @@ -396,7 +425,7 @@ func (svc *service) Stop() error { for key, sub := range svc.verbSubs { keys = append(keys, key) if err := sub.Drain(); err != nil { - return fmt.Errorf("draining subsctioption for subject %q: %w", sub.Subject, err) + return fmt.Errorf("draining subscription for subject %q: %w", sub.Subject, err) } } for _, key := range keys { @@ -404,6 +433,9 @@ func (svc *service) Stop() error { } restoreAsyncHandlers(svc.conn, svc.natsHandlers) svc.stopped = true + if svc.Config.DoneHandler != nil { + svc.Config.DoneHandler(svc) + } return nil } @@ -412,22 +444,28 @@ func restoreAsyncHandlers(nc *nats.Conn, handlers handlers) { nc.SetErrorHandler(handlers.asyncErr) } +// ID returns the service instance's unique ID. func (svc *service) ID() string { return svc.id } +// Name returns the name of the service. +// It can be shared between multiple service instances. func (svc *service) Name() string { return svc.Config.Name } +// Description returns the service description. func (svc *service) Description() string { return svc.Config.Description } +// Version returns the service version. func (svc *service) Version() string { return svc.Config.Version } +// Stats returns statisctics for the service endpoint and all monitoring endpoints. func (svc *service) Stats() Stats { svc.Lock() defer func() { @@ -451,36 +489,35 @@ func (svc *service) Stats() Stats { } } +// Reset resets all statistics on a service instance. func (svc *service) Reset() { for _, se := range svc.endpointStats { se.NumRequests = 0 se.TotalProcessingTime = 0 + se.AverageProcessingTime = 0 + se.Data = nil se.NumErrors = 0 se.Data = nil } } -// ControlSubject returns monitoring subjects used by the Service -func ControlSubject(verb Verb, kind, id string) (string, error) { +// ControlSubject returns monitoring subjects used by the Service. +// Providing a verb is mandatory (it should be one of Ping, Schema, Info or Stats). +// Depending on whether kind and id are provided, ControlSubject will return one of the following: +// - verb only: subject used to monitor all available services +// - verb and kind: subject used to monitor services with the provided name +// - verb, name and id: subject used to monitor an instance of a service with the provided ID +func ControlSubject(verb Verb, name, id string) (string, error) { verbStr := verb.String() if verbStr == "" { return "", fmt.Errorf("%w: %q", ErrVerbNotSupported, verbStr) } - kind = strings.ToUpper(kind) - if kind == "" && id == "" { + name = strings.ToUpper(name) + if name == "" && id == "" { return fmt.Sprintf("%s.%s", APIPrefix, verbStr), nil } if id == "" { - return fmt.Sprintf("%s.%s.%s", APIPrefix, verbStr, kind), nil - } - return fmt.Sprintf("%s.%s.%s.%s", APIPrefix, verbStr, kind, id), nil -} - -func SendError(req *nats.Msg, err ResponseError) { - if req.Header == nil { - req.Header = nats.Header{} + return fmt.Sprintf("%s.%s.%s", APIPrefix, verbStr, name), nil } - req.Header.Add(ErrorHeader, err.Description) - req.Header.Add(ErrorCodeHeader, err.ErrorCode) - req.RespondMsg(req) + return fmt.Sprintf("%s.%s.%s.%s", APIPrefix, verbStr, name, id), nil } diff --git a/services/service_test.go b/service/service_test.go similarity index 74% rename from services/service_test.go rename to service/service_test.go index 7bf1437a7..ea6afeb4a 100644 --- a/services/service_test.go +++ b/service/service_test.go @@ -11,9 +11,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -package services +package service import ( + "bytes" "encoding/json" "errors" "fmt" @@ -37,16 +38,20 @@ func TestServiceBasics(t *testing.T) { defer nc.Close() // Stub service. - doAdd := func(req *nats.Msg) { + doAdd := func(req *Request) { if rand.Intn(10) == 0 { - SendError(req, ResponseError{"500", "Unexpected error!"}) + if err := req.Error("500", "Unexpected error!"); err != nil { + t.Fatalf("Unexpected error when sending error response: %v", err) + } return } // Happy Path. // Random delay between 5-10ms time.Sleep(5*time.Millisecond + time.Duration(rand.Intn(5))*time.Millisecond) if err := req.Respond([]byte("42")); err != nil { - SendError(req, ResponseError{"500", "Unexpected error!"}) + if err := req.Error("500", "Unexpected error!"); err != nil { + t.Fatalf("Unexpected error when sending error response: %v", err) + } return } } @@ -178,10 +183,18 @@ func TestServiceBasics(t *testing.T) { if requestsNum != 50 { t.Fatalf("Expected a total fo 50 requests processed, got: %d", requestsNum) } + // Reset stats for a service + svcs[0].Reset() + for _, e := range svcs[0].Stats().Endpoints { + emptyStats := EndpointStats{Name: e.Name} + if e != emptyStats { + t.Fatalf("Expected empty stats after reset; got: %+v", e) + } + } } func TestAddService(t *testing.T) { - testHandler := func(*nats.Msg) {} + testHandler := func(*Request) {} var errNats, errService, closedNats, doneService chan struct{} tests := []struct { @@ -229,7 +242,7 @@ func TestAddService(t *testing.T) { Subject: "test.sub", Handler: testHandler, }, - ErrorHandler: func(s Service, err *Error) { + ErrorHandler: func(s Service, err *NATSError) { errService <- struct{}{} }, }, @@ -393,7 +406,9 @@ func TestAddService(t *testing.T) { } } - srv.Stop() + if err := srv.Stop(); err != nil { + t.Fatalf("Unexpected error when stopping the service: %v", err) + } if test.natsClosedHandler != nil { go nc.Opts.ClosedCB(nc) select { @@ -424,28 +439,58 @@ func TestAddService(t *testing.T) { } } -func TestServiceErrors(t *testing.T) { +func TestRequestRespond(t *testing.T) { + type x struct { + A string `json:"a"` + B int `json:"b"` + } + tests := []struct { - name string - handlerResponse *ResponseError - expectedMessage string - expectedCode string + name string + respondData interface{} + errDescription string + errCode string + expectedMessage string + expectedCode string + expectedResponse []byte + withRespondError error }{ + { + name: "byte response", + respondData: []byte("OK"), + expectedResponse: []byte("OK"), + }, + { + name: "byte response, connection closed", + respondData: []byte("OK"), + withRespondError: ErrRespond, + }, + { + name: "struct response", + respondData: x{"abc", 5}, + expectedResponse: []byte(`{"a":"abc","b":5}`), + }, + { + name: "invalid response data", + respondData: func() {}, + withRespondError: ErrMarshalResponse, + }, { name: "generic error", - handlerResponse: &ResponseError{ErrorCode: "500", Description: "oops"}, + errDescription: "oops", + errCode: "500", expectedMessage: "oops", expectedCode: "500", }, { - name: "api error", - handlerResponse: &ResponseError{ErrorCode: "400", Description: "oops"}, - expectedMessage: "oops", - expectedCode: "400", + name: "missing error code", + errDescription: "oops", + withRespondError: ErrArgRequired, }, { - name: "no error", - handlerResponse: nil, + name: "missing error description", + errCode: "500", + withRespondError: ErrArgRequired, }, } @@ -461,13 +506,47 @@ func TestServiceErrors(t *testing.T) { defer nc.Close() // Stub service. - handler := func(req *nats.Msg) { - if test.handlerResponse == nil { - req.Respond([]byte("ok")) + handler := func(req *Request) { + if errors.Is(test.withRespondError, ErrRespond) { + nc.Close() + } + if test.errCode == "" && test.errDescription == "" { + if resp, ok := test.respondData.([]byte); ok { + err := req.Respond(resp) + if test.withRespondError != nil { + if !errors.Is(err, test.withRespondError) { + t.Fatalf("Expected error: %v; got: %v", test.withRespondError, err) + } + return + } + if err != nil { + t.Fatalf("Unexpected error when sending response: %v", err) + } + } else { + err := req.RespondJSON(test.respondData) + if test.withRespondError != nil { + if !errors.Is(err, test.withRespondError) { + t.Fatalf("Expected error: %v; got: %v", test.withRespondError, err) + } + return + } + if err != nil { + t.Fatalf("Unexpected error when sending response: %v", err) + } + } return } - SendError(req, *test.handlerResponse) + err := req.Error(test.errCode, test.errDescription) + if test.withRespondError != nil { + if !errors.Is(err, test.withRespondError) { + t.Fatalf("Expected error: %v; got: %v", test.withRespondError, err) + } + return + } + if err != nil { + t.Fatalf("Unexpected error when sending response: %v", err) + } } svc, err := Add(nc, Config{ @@ -483,18 +562,32 @@ func TestServiceErrors(t *testing.T) { } defer svc.Stop() - resp, err := nc.Request("svc.fail", nil, 1*time.Second) + resp, err := nc.Request("svc.fail", nil, 50*time.Millisecond) + if test.withRespondError != nil { + return + } if err != nil { - t.Fatalf("request error") + t.Fatalf("request error: %v", err) } - description := resp.Header.Get("Nats-Service-Error") - if description != test.expectedMessage { - t.Fatalf("Invalid response message; want: %q; got: %q", test.expectedMessage, description) + if test.errCode != "" { + description := resp.Header.Get("Nats-Service-Error") + if description != test.expectedMessage { + t.Fatalf("Invalid response message; want: %q; got: %q", test.expectedMessage, description) + } + code := resp.Header.Get("Nats-Service-Error-Code") + if code != test.expectedCode { + t.Fatalf("Invalid response code; want: %q; got: %q", test.expectedCode, code) + } + return + } + + if err != nil { + t.Fatalf("Unexpected error: %v", err) } - code := resp.Header.Get("Nats-Service-Error-Code") - if code != test.expectedCode { - t.Fatalf("Invalid response code; want: %q; got: %q", test.expectedCode, code) + + if !bytes.Equal(bytes.TrimSpace(resp.Data), bytes.TrimSpace(test.expectedResponse)) { + t.Fatalf("Invalid response; want: %s; got: %s", string(test.expectedResponse), string(resp.Data)) } }) } diff --git a/services/errors.go b/services/errors.go deleted file mode 100644 index 29f0d2979..000000000 --- a/services/errors.go +++ /dev/null @@ -1,25 +0,0 @@ -// Copyright 2022 The NATS Authors -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package services - -import "fmt" - -type ResponseError struct { - ErrorCode string - Description string -} - -func (e *ResponseError) Error() string { - return fmt.Sprintf("%s %s", e.ErrorCode, e.Description) -} From 11f29989f3d692de147381912a213a35617c9e9f Mon Sep 17 00:00:00 2001 From: Piotr Piotrowski Date: Mon, 12 Dec 2022 13:46:21 +0100 Subject: [PATCH 03/14] Change Service implementation to a struct --- js.go | 4 +- service/example_test.go | 183 +++++++++++++++++++++++++++++++++++++++- service/service.go | 170 +++++++++++++++---------------------- service/service_test.go | 18 ++-- 4 files changed, 257 insertions(+), 118 deletions(-) diff --git a/js.go b/js.go index 362d75117..a249266e3 100644 --- a/js.go +++ b/js.go @@ -1528,13 +1528,11 @@ func (js *js) subscribe(subj, queue string, cb MsgHandler, ch chan *Msg, isSync, } // Find the stream mapped to the subject if not bound to a stream already. - if o.stream == _EMPTY_ { + if stream == _EMPTY_ { stream, err = js.StreamNameBySubject(subj) if err != nil { return nil, err } - } else { - stream = o.stream } // With an explicit durable name, we can lookup the consumer first diff --git a/service/example_test.go b/service/example_test.go index 715e84a20..4c472cc92 100644 --- a/service/example_test.go +++ b/service/example_test.go @@ -28,13 +28,13 @@ func ExampleAdd() { }, // DoneHandler can be set to customize behavior on stopping a service. - DoneHandler: func(srv Service) { - fmt.Printf("stopped service %q with ID %q\n", srv.Name(), srv.ID()) + DoneHandler: func(srv *Service) { + fmt.Printf("stopped service %q with ID %q\n", srv.Name, srv.ID()) }, // ErrorHandler can be used to customize behavior on service execution error. - ErrorHandler: func(srv Service, err *NATSError) { - fmt.Printf("Service %q returned an error on subject %q: %s", srv.Name(), err.Subject, err.Description) + ErrorHandler: func(srv *Service, err *NATSError) { + fmt.Printf("Service %q returned an error on subject %q: %s", srv.Name, err.Subject, err.Description) }, } @@ -45,6 +45,140 @@ func ExampleAdd() { defer srv.Stop() } +func ExampleService_ID() { + nc, err := nats.Connect("127.0.0.1:4222") + if err != nil { + log.Fatal(err) + } + defer nc.Close() + + config := Config{ + Name: "EchoService", + Endpoint: Endpoint{ + Subject: "echo", + Handler: func(*Request) {}, + }, + } + + srv, _ := Add(nc, config) + + // unique service ID + id := srv.ID() + fmt.Println(id) +} + +func ExampleService_Stats() { + nc, err := nats.Connect("127.0.0.1:4222") + if err != nil { + log.Fatal(err) + } + defer nc.Close() + + config := Config{ + Name: "EchoService", + Endpoint: Endpoint{ + Subject: "echo", + Handler: func(*Request) {}, + }, + } + + srv, _ := Add(nc, config) + + // stats of a service instance + stats := srv.Stats() + + for _, e := range stats.Endpoints { + fmt.Println(e.AverageProcessingTime) + } + +} + +func ExampleService_Stop() { + nc, err := nats.Connect("127.0.0.1:4222") + if err != nil { + log.Fatal(err) + } + defer nc.Close() + + config := Config{ + Name: "EchoService", + Endpoint: Endpoint{ + Subject: "echo", + Handler: func(*Request) {}, + }, + } + + srv, _ := Add(nc, config) + + // stop a service + err = srv.Stop() + if err != nil { + log.Fatal(err) + } + + // stop is idempotent so multiple executions will not return an error + err = srv.Stop() + if err != nil { + log.Fatal(err) + } +} + +func ExampleService_Stopped() { + nc, err := nats.Connect("127.0.0.1:4222") + if err != nil { + log.Fatal(err) + } + defer nc.Close() + + config := Config{ + Name: "EchoService", + Endpoint: Endpoint{ + Subject: "echo", + Handler: func(*Request) {}, + }, + } + + srv, _ := Add(nc, config) + + // stop a service + err = srv.Stop() + if err != nil { + log.Fatal(err) + } + + if srv.Stopped() { + fmt.Println("service stopped") + } +} + +func ExampleService_Reset() { + nc, err := nats.Connect("127.0.0.1:4222") + if err != nil { + log.Fatal(err) + } + defer nc.Close() + + config := Config{ + Name: "EchoService", + Endpoint: Endpoint{ + Subject: "echo", + Handler: func(*Request) {}, + }, + } + + srv, _ := Add(nc, config) + + // reset endpoint stats on this service + srv.Reset() + + empty := EndpointStats{Name: "EchoService"} + for _, e := range srv.Stats().Endpoints { + if e != empty { + log.Fatal("Expected endpoint stats to be empty") + } + } +} + func ExampleControlSubject() { // subject used to get PING from all services @@ -64,3 +198,44 @@ func ExampleControlSubject() { // $SRV.PING.COOLSERVICE // $SRV.PING.COOLSERVICE.123 } + +func ExampleRequest_Respond() { + handler := func(req *Request) { + // respond to the request + if err := req.Respond(req.Data); err != nil { + log.Fatal(err) + } + } + + fmt.Printf("%T", handler) +} + +func ExampleRequest_RespondJSON() { + type Point struct { + X int `json:"x"` + Y int `json:"y"` + } + + handler := func(req *Request) { + resp := Point{5, 10} + // respond to the request + // response will be serialized to {"x":5,"y":10} + if err := req.RespondJSON(resp); err != nil { + log.Fatal(err) + } + } + + fmt.Printf("%T", handler) +} + +func ExampleRequest_Error() { + handler := func(req *Request) { + // respond with an error + // Error sets Nats-Service-Error and Nats-Service-Error-Code headers in the response + if err := req.Error("400", "bad request"); err != nil { + log.Fatal(err) + } + } + + fmt.Printf("%T", handler) +} diff --git a/service/service.go b/service/service.go index 9ce5dbae6..66b9eeedd 100644 --- a/service/service.go +++ b/service/service.go @@ -31,38 +31,28 @@ import ( // This functionality is EXPERIMENTAL and may be changed in later releases. type ( + // Service represents a configured NATS service. + // It should be created using [Add] in order to configure the appropriate NATS subscriptions + // for request handler and monitoring. + Service struct { + // Config contains a configuration of the service + Config - // Service is an interface for service management. - // It exposes methods to stop/reset a service, as well as get information on a service. - Service interface { - // ID returns the service instance's unique ID. - ID() string - - // Name returns the name of the service. - // It can be shared between multiple service instances. - Name() string - - // Description returns the service description. - Description() string - - // Version returns the service version. - Version() string - - // Stats returns statisctics for the service endpoint and all monitoring endpoints. - Stats() Stats - - // Reset resets all statistics on a service instance. - Reset() - - // Stop drains the endpoint subscriptions and marks the service as stopped. - Stop() error + m sync.Mutex + id string + reqSub *nats.Subscription + verbSubs map[string]*nats.Subscription + endpointStats map[string]*EndpointStats + conn *nats.Conn + natsHandlers handlers + stopped bool } // ErrHandler is a function used to configure a custom error handler for a service, - ErrHandler func(Service, *NATSError) + ErrHandler func(*Service, *NATSError) // DoneHandler is a function used to configure a custom done handler for a service. - DoneHandler func(Service) + DoneHandler func(*Service) // StatsHandleris a function used to configure a custom STATS endpoint. // It should return a value which can be serialized to JSON. @@ -140,19 +130,6 @@ type ( Description string } - // service is the internal implementation of a Service - service struct { - sync.Mutex - Config - id string - reqSub *nats.Subscription - verbSubs map[string]*nats.Subscription - endpointStats map[string]*EndpointStats - conn *nats.Conn - natsHandlers handlers - stopped bool - } - handlers struct { closed nats.ConnHandler asyncErr nats.ErrHandler @@ -215,13 +192,13 @@ func (s Verb) String() string { // A service name and Endpoint configuration are required to add a service. // Add returns a [Service] interface, allowing service menagement. // Each service is assigned a unique ID. -func Add(nc *nats.Conn, config Config) (Service, error) { +func Add(nc *nats.Conn, config Config) (*Service, error) { if err := config.valid(); err != nil { return nil, err } id := nuid.Next() - svc := &service{ + svc := &Service{ Config: config, conn: nc, id: id, @@ -271,7 +248,7 @@ func Add(nc *nats.Conn, config Config) (Service, error) { } schemaHandler := func(m *nats.Msg) { - response, _ := json.Marshal(svc.Config.Schema) + response, _ := json.Marshal(svc.Schema) m.Respond(response) } @@ -285,7 +262,7 @@ func Add(nc *nats.Conn, config Config) (Service, error) { return nil, err } - if svc.Config.Schema.Request != "" || svc.Config.Schema.Response != "" { + if svc.Schema.Request != "" || svc.Schema.Response != "" { if err := svc.verbHandlers(nc, SchemaVerb, schemaHandler); err != nil { return nil, err } @@ -351,90 +328,90 @@ func (e *Endpoint) valid() error { // Each request generates 3 subscriptions, one for the general verb // affecting all services written with the framework, one that handles // all services of a particular kind, and finally a specific service instance. -func (svc *service) verbHandlers(nc *nats.Conn, verb Verb, handler nats.MsgHandler) error { +func (svc *Service) verbHandlers(nc *nats.Conn, verb Verb, handler nats.MsgHandler) error { name := fmt.Sprintf("%s-all", verb.String()) if err := svc.addInternalHandler(nc, verb, "", "", name, handler); err != nil { return err } name = fmt.Sprintf("%s-kind", verb.String()) - if err := svc.addInternalHandler(nc, verb, svc.Name(), "", name, handler); err != nil { + if err := svc.addInternalHandler(nc, verb, svc.Name, "", name, handler); err != nil { return err } - return svc.addInternalHandler(nc, verb, svc.Name(), svc.ID(), verb.String(), handler) + return svc.addInternalHandler(nc, verb, svc.Name, svc.ID(), verb.String(), handler) } // addInternalHandler registers a control subject handler. -func (svc *service) addInternalHandler(nc *nats.Conn, verb Verb, kind, id, name string, handler nats.MsgHandler) error { +func (s *Service) addInternalHandler(nc *nats.Conn, verb Verb, kind, id, name string, handler nats.MsgHandler) error { subj, err := ControlSubject(verb, kind, id) if err != nil { - svc.Stop() + s.Stop() return err } - svc.verbSubs[name], err = nc.Subscribe(subj, func(msg *nats.Msg) { + s.verbSubs[name], err = nc.Subscribe(subj, func(msg *nats.Msg) { start := time.Now() handler(msg) - svc.Lock() - stats := svc.endpointStats[name] + s.m.Lock() + stats := s.endpointStats[name] stats.NumRequests++ stats.TotalProcessingTime += time.Since(start) stats.AverageProcessingTime = stats.TotalProcessingTime / time.Duration(stats.NumRequests) - svc.Unlock() + s.m.Unlock() }) if err != nil { - svc.Stop() + s.Stop() return err } - svc.endpointStats[name] = &EndpointStats{ + s.endpointStats[name] = &EndpointStats{ Name: name, } return nil } // reqHandler itself -func (svc *service) reqHandler(req *Request) { +func (s *Service) reqHandler(req *Request) { start := time.Now() - svc.Config.Endpoint.Handler(req) - svc.Lock() - stats := svc.endpointStats[""] + s.Endpoint.Handler(req) + s.m.Lock() + stats := s.endpointStats[""] stats.NumRequests++ stats.TotalProcessingTime += time.Since(start) stats.AverageProcessingTime = stats.TotalProcessingTime / time.Duration(stats.NumRequests) if req.errResponse { - stats := svc.endpointStats[""] + stats := s.endpointStats[""] stats.NumErrors++ } - svc.Unlock() + s.m.Unlock() } // Stop drains the endpoint subscriptions and marks the service as stopped. -func (svc *service) Stop() error { - if svc.stopped { +func (s *Service) Stop() error { + if s.stopped { return nil } - if svc.reqSub != nil { - if err := svc.reqSub.Drain(); err != nil { + if s.reqSub != nil { + if err := s.reqSub.Drain(); err != nil { return fmt.Errorf("draining subscription for request handler: %w", err) } - svc.reqSub = nil + s.reqSub = nil } var keys []string - for key, sub := range svc.verbSubs { + for key, sub := range s.verbSubs { keys = append(keys, key) if err := sub.Drain(); err != nil { return fmt.Errorf("draining subscription for subject %q: %w", sub.Subject, err) } } for _, key := range keys { - delete(svc.verbSubs, key) + delete(s.verbSubs, key) } - restoreAsyncHandlers(svc.conn, svc.natsHandlers) - svc.stopped = true - if svc.Config.DoneHandler != nil { - svc.Config.DoneHandler(svc) + restoreAsyncHandlers(s.conn, s.natsHandlers) + s.stopped = true + if s.DoneHandler != nil { + s.DoneHandler(s) } return nil } @@ -445,53 +422,37 @@ func restoreAsyncHandlers(nc *nats.Conn, handlers handlers) { } // ID returns the service instance's unique ID. -func (svc *service) ID() string { - return svc.id -} - -// Name returns the name of the service. -// It can be shared between multiple service instances. -func (svc *service) Name() string { - return svc.Config.Name -} - -// Description returns the service description. -func (svc *service) Description() string { - return svc.Config.Description -} - -// Version returns the service version. -func (svc *service) Version() string { - return svc.Config.Version +func (s *Service) ID() string { + return s.id } // Stats returns statisctics for the service endpoint and all monitoring endpoints. -func (svc *service) Stats() Stats { - svc.Lock() +func (s *Service) Stats() Stats { + s.m.Lock() defer func() { - svc.Unlock() + s.m.Unlock() }() - if svc.Config.StatsHandler != nil { - stats := svc.endpointStats[""] - stats.Data = svc.Config.StatsHandler(svc.Endpoint) + if s.StatsHandler != nil { + stats := s.endpointStats[""] + stats.Data = s.StatsHandler(s.Endpoint) } idx := 0 - v := make([]EndpointStats, len(svc.endpointStats)) - for _, se := range svc.endpointStats { + v := make([]EndpointStats, len(s.endpointStats)) + for _, se := range s.endpointStats { v[idx] = *se idx++ } return Stats{ - Name: svc.Name(), - ID: svc.ID(), - Version: svc.Version(), + Name: s.Name, + ID: s.ID(), + Version: s.Version, Endpoints: v, } } // Reset resets all statistics on a service instance. -func (svc *service) Reset() { - for _, se := range svc.endpointStats { +func (s *Service) Reset() { + for _, se := range s.endpointStats { se.NumRequests = 0 se.TotalProcessingTime = 0 se.AverageProcessingTime = 0 @@ -501,6 +462,11 @@ func (svc *service) Reset() { } } +// Stopped informs whether [Stop] was executed on the service. +func (s *Service) Stopped() bool { + return s.stopped +} + // ControlSubject returns monitoring subjects used by the Service. // Providing a verb is mandatory (it should be one of Ping, Schema, Info or Stats). // Depending on whether kind and id are provided, ControlSubject will return one of the following: diff --git a/service/service_test.go b/service/service_test.go index ea6afeb4a..55a652390 100644 --- a/service/service_test.go +++ b/service/service_test.go @@ -56,7 +56,7 @@ func TestServiceBasics(t *testing.T) { } } - var svcs []Service + var svcs []*Service // Create 5 service responders. config := Config{ @@ -88,10 +88,10 @@ func TestServiceBasics(t *testing.T) { } for _, svc := range svcs { - if svc.Name() != "CoolAddService" { - t.Fatalf("Expected %q, got %q", "CoolAddService", svc.Name()) + if svc.Name != "CoolAddService" { + t.Fatalf("Expected %q, got %q", "CoolAddService", svc.Name) } - if len(svc.Description()) == 0 || len(svc.Version()) == 0 { + if len(svc.Description) == 0 || len(svc.Version) == 0 { t.Fatalf("Expected non empty description and version") } } @@ -226,7 +226,7 @@ func TestAddService(t *testing.T) { Subject: "test.sub", Handler: testHandler, }, - DoneHandler: func(s Service) { + DoneHandler: func(*Service) { doneService <- struct{}{} }, }, @@ -242,7 +242,7 @@ func TestAddService(t *testing.T) { Subject: "test.sub", Handler: testHandler, }, - ErrorHandler: func(s Service, err *NATSError) { + ErrorHandler: func(*Service, *NATSError) { errService <- struct{}{} }, }, @@ -258,7 +258,7 @@ func TestAddService(t *testing.T) { Subject: "test.sub", Handler: testHandler, }, - DoneHandler: func(s Service) { + DoneHandler: func(*Service) { doneService <- struct{}{} }, }, @@ -280,7 +280,7 @@ func TestAddService(t *testing.T) { Subject: "test.sub", Handler: testHandler, }, - DoneHandler: func(s Service) { + DoneHandler: func(*Service) { doneService <- struct{}{} }, }, @@ -356,7 +356,7 @@ func TestAddService(t *testing.T) { return } - pingSubject, err := ControlSubject(PingVerb, srv.Name(), srv.ID()) + pingSubject, err := ControlSubject(PingVerb, srv.Name, srv.ID()) if err != nil { t.Fatalf("Unexpected error: %v", err) } From 2ae257842054b2ac9face3d49206c64eadfe329f Mon Sep 17 00:00:00 2001 From: Piotr Piotrowski Date: Mon, 12 Dec 2022 14:05:34 +0100 Subject: [PATCH 04/14] Fix data races --- service/service.go | 10 +++++++--- service/service_test.go | 40 +++++++++++++++++++++------------------- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/service/service.go b/service/service.go index 66b9eeedd..1ac4b5be0 100644 --- a/service/service.go +++ b/service/service.go @@ -389,9 +389,11 @@ func (s *Service) reqHandler(req *Request) { // Stop drains the endpoint subscriptions and marks the service as stopped. func (s *Service) Stop() error { + s.m.Lock() if s.stopped { return nil } + defer s.m.Unlock() if s.reqSub != nil { if err := s.reqSub.Drain(); err != nil { return fmt.Errorf("draining subscription for request handler: %w", err) @@ -429,9 +431,7 @@ func (s *Service) ID() string { // Stats returns statisctics for the service endpoint and all monitoring endpoints. func (s *Service) Stats() Stats { s.m.Lock() - defer func() { - s.m.Unlock() - }() + defer s.m.Unlock() if s.StatsHandler != nil { stats := s.endpointStats[""] stats.Data = s.StatsHandler(s.Endpoint) @@ -452,6 +452,7 @@ func (s *Service) Stats() Stats { // Reset resets all statistics on a service instance. func (s *Service) Reset() { + s.m.Lock() for _, se := range s.endpointStats { se.NumRequests = 0 se.TotalProcessingTime = 0 @@ -460,10 +461,13 @@ func (s *Service) Reset() { se.NumErrors = 0 se.Data = nil } + s.m.Unlock() } // Stopped informs whether [Stop] was executed on the service. func (s *Service) Stopped() bool { + s.m.Lock() + defer s.m.Unlock() return s.stopped } diff --git a/service/service_test.go b/service/service_test.go index 55a652390..ecb35ac08 100644 --- a/service/service_test.go +++ b/service/service_test.go @@ -195,7 +195,10 @@ func TestServiceBasics(t *testing.T) { func TestAddService(t *testing.T) { testHandler := func(*Request) {} - var errNats, errService, closedNats, doneService chan struct{} + errNats := make(chan struct{}) + errService := make(chan struct{}) + closedNats := make(chan struct{}) + doneService := make(chan struct{}) tests := []struct { name string @@ -343,11 +346,6 @@ func TestAddService(t *testing.T) { } defer nc.Close() - errNats = make(chan struct{}) - errService = make(chan struct{}) - closedNats = make(chan struct{}) - doneService = make(chan struct{}) - srv, err := Add(nc, test.givenConfig) if test.withError != nil { if !errors.Is(err, test.withError) { @@ -505,17 +503,21 @@ func TestRequestRespond(t *testing.T) { } defer nc.Close() + respData := test.respondData + respError := test.withRespondError + errCode := test.errCode + errDesc := test.errDescription // Stub service. handler := func(req *Request) { if errors.Is(test.withRespondError, ErrRespond) { nc.Close() } - if test.errCode == "" && test.errDescription == "" { - if resp, ok := test.respondData.([]byte); ok { + if errCode == "" && errDesc == "" { + if resp, ok := respData.([]byte); ok { err := req.Respond(resp) - if test.withRespondError != nil { - if !errors.Is(err, test.withRespondError) { - t.Fatalf("Expected error: %v; got: %v", test.withRespondError, err) + if respError != nil { + if !errors.Is(err, respError) { + t.Fatalf("Expected error: %v; got: %v", respError, err) } return } @@ -523,10 +525,10 @@ func TestRequestRespond(t *testing.T) { t.Fatalf("Unexpected error when sending response: %v", err) } } else { - err := req.RespondJSON(test.respondData) - if test.withRespondError != nil { - if !errors.Is(err, test.withRespondError) { - t.Fatalf("Expected error: %v; got: %v", test.withRespondError, err) + err := req.RespondJSON(respData) + if respError != nil { + if !errors.Is(err, respError) { + t.Fatalf("Expected error: %v; got: %v", respError, err) } return } @@ -537,10 +539,10 @@ func TestRequestRespond(t *testing.T) { return } - err := req.Error(test.errCode, test.errDescription) - if test.withRespondError != nil { - if !errors.Is(err, test.withRespondError) { - t.Fatalf("Expected error: %v; got: %v", test.withRespondError, err) + err := req.Error(errCode, errDesc) + if respError != nil { + if !errors.Is(err, respError) { + t.Fatalf("Expected error: %v; got: %v", respError, err) } return } From 7d5cf4a5ef708b4e6f0af033b256ecc37d4b2405 Mon Sep 17 00:00:00 2001 From: Piotr Piotrowski Date: Wed, 14 Dec 2022 15:13:57 +0100 Subject: [PATCH 05/14] Revert to using service interface, rename package to micro --- {service => micro}/example_package_test.go | 6 +- {service => micro}/example_test.go | 24 +- {service => micro}/request.go | 2 +- {service => micro}/service.go | 208 +++++++--- {service => micro}/service_test.go | 436 ++++++++++++++++++++- 5 files changed, 584 insertions(+), 92 deletions(-) rename {service => micro}/example_package_test.go (91%) rename {service => micro}/example_test.go (90%) rename {service => micro}/request.go (98%) rename {service => micro}/service.go (71%) rename {service => micro}/service_test.go (58%) diff --git a/service/example_package_test.go b/micro/example_package_test.go similarity index 91% rename from service/example_package_test.go rename to micro/example_package_test.go index a5019a7b5..1e919aa84 100644 --- a/service/example_package_test.go +++ b/micro/example_package_test.go @@ -1,4 +1,4 @@ -package service +package micro import ( "fmt" @@ -19,7 +19,7 @@ func Example() { } defer nc.Close() - // Service handler is a function which takes *service.Request as argument. + // Service handler is a function which takes Service.Request as argument. // req.Respond or req.Error should be used to respond to the request. incrementHandler := func(req *Request) { val, err := strconv.Atoi(string(req.Data)) @@ -46,7 +46,7 @@ func Example() { // Multiple instances of the servcice with the same name can be created. // Requests to a service with the same name will be load-balanced. for i := 0; i < 5; i++ { - svc, err := Add(nc, config) + svc, err := AddService(nc, config) if err != nil { log.Fatal(err) } diff --git a/service/example_test.go b/micro/example_test.go similarity index 90% rename from service/example_test.go rename to micro/example_test.go index 4c472cc92..df657cfaa 100644 --- a/service/example_test.go +++ b/micro/example_test.go @@ -1,4 +1,4 @@ -package service +package micro import ( "fmt" @@ -7,7 +7,7 @@ import ( "github.com/nats-io/nats.go" ) -func ExampleAdd() { +func ExampleAddService() { nc, err := nats.Connect("127.0.0.1:4222") if err != nil { log.Fatal(err) @@ -28,17 +28,17 @@ func ExampleAdd() { }, // DoneHandler can be set to customize behavior on stopping a service. - DoneHandler: func(srv *Service) { - fmt.Printf("stopped service %q with ID %q\n", srv.Name, srv.ID()) + DoneHandler: func(srv Service) { + fmt.Printf("stopped service %q with ID %q\n", srv.Name(), srv.ID()) }, // ErrorHandler can be used to customize behavior on service execution error. - ErrorHandler: func(srv *Service, err *NATSError) { - fmt.Printf("Service %q returned an error on subject %q: %s", srv.Name, err.Subject, err.Description) + ErrorHandler: func(srv Service, err *NATSError) { + fmt.Printf("Service %q returned an error on subject %q: %s", srv.Name(), err.Subject, err.Description) }, } - srv, err := Add(nc, config) + srv, err := AddService(nc, config) if err != nil { log.Fatal(err) } @@ -60,7 +60,7 @@ func ExampleService_ID() { }, } - srv, _ := Add(nc, config) + srv, _ := AddService(nc, config) // unique service ID id := srv.ID() @@ -82,7 +82,7 @@ func ExampleService_Stats() { }, } - srv, _ := Add(nc, config) + srv, _ := AddService(nc, config) // stats of a service instance stats := srv.Stats() @@ -108,7 +108,7 @@ func ExampleService_Stop() { }, } - srv, _ := Add(nc, config) + srv, _ := AddService(nc, config) // stop a service err = srv.Stop() @@ -138,7 +138,7 @@ func ExampleService_Stopped() { }, } - srv, _ := Add(nc, config) + srv, _ := AddService(nc, config) // stop a service err = srv.Stop() @@ -166,7 +166,7 @@ func ExampleService_Reset() { }, } - srv, _ := Add(nc, config) + srv, _ := AddService(nc, config) // reset endpoint stats on this service srv.Reset() diff --git a/service/request.go b/micro/request.go similarity index 98% rename from service/request.go rename to micro/request.go index 72cc7f345..d391e8af1 100644 --- a/service/request.go +++ b/micro/request.go @@ -1,4 +1,4 @@ -package service +package micro import ( "encoding/json" diff --git a/service/service.go b/micro/service.go similarity index 71% rename from service/service.go rename to micro/service.go index 1ac4b5be0..dcab61d48 100644 --- a/service/service.go +++ b/micro/service.go @@ -11,7 +11,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package service +package micro import ( "encoding/json" @@ -31,10 +31,37 @@ import ( // This functionality is EXPERIMENTAL and may be changed in later releases. type ( - // Service represents a configured NATS service. + Service interface { + // ID returns the service instance's unique ID. + ID() string + + // Name returns the name of the service. + // It can be shared between multiple service instances. + Name() string + + // Description returns the service description. + Description() string + + // Version returns the service version. + Version() string + + // Stats returns statisctics for the service endpoint and all monitoring endpoints. + Stats() Stats + + // Reset resets all statistics on a service instance. + Reset() + + // Stop drains the endpoint subscriptions and marks the service as stopped. + Stop() error + + // Stopped informs whether [Stop] was executed on the service. + Stopped() bool + } + + // service represents a configured NATS service. // It should be created using [Add] in order to configure the appropriate NATS subscriptions // for request handler and monitoring. - Service struct { + service struct { // Config contains a configuration of the service Config @@ -49,10 +76,10 @@ type ( } // ErrHandler is a function used to configure a custom error handler for a service, - ErrHandler func(*Service, *NATSError) + ErrHandler func(Service, *NATSError) // DoneHandler is a function used to configure a custom done handler for a service. - DoneHandler func(*Service) + DoneHandler func(Service) // StatsHandleris a function used to configure a custom STATS endpoint. // It should return a value which can be serialized to JSON. @@ -169,6 +196,9 @@ var ( // ErrVerbNotSupported is returned when invalid [Verb] is used (PING, SCHEMA, INFO, STATS) ErrVerbNotSupported = errors.New("unsupported verb") + + // ErrServiceNameRequired is returned when attempting to generate control subject with ID but empty name + ErrServiceNameRequired = errors.New("service name is required to generate ID control subject") ) func (s Verb) String() string { @@ -186,19 +216,19 @@ func (s Verb) String() string { } } -// Add adds a microservice. +// AddService adds a microservice. // It will enable internal common services (PING, STATS, INFO and SCHEMA) as well as // the actual service handler on the subject provided in config.Endpoint // A service name and Endpoint configuration are required to add a service. -// Add returns a [Service] interface, allowing service menagement. +// AddService returns a [Service] interface, allowing service menagement. // Each service is assigned a unique ID. -func Add(nc *nats.Conn, config Config) (*Service, error) { +func AddService(nc *nats.Conn, config Config) (Service, error) { if err := config.valid(); err != nil { return nil, err } id := nuid.Next() - svc := &Service{ + svc := &service{ Config: config, conn: nc, id: id, @@ -209,6 +239,8 @@ func Add(nc *nats.Conn, config Config) (*Service, error) { Name: config.Name, } + svc.setupAsyncCallbacks() + // Setup internal subscriptions. var err error @@ -232,24 +264,40 @@ func Add(nc *nats.Conn, config Config) (*Service, error) { ID: id, } - infoHandler := func(m *nats.Msg) { + infoHandler := func(req *Request) { response, _ := json.Marshal(info) - m.Respond(response) + if err := req.Respond(response); err != nil { + if err := req.Error("500", fmt.Sprintf("Error handling INFO request: %s", err)); err != nil && config.ErrorHandler != nil { + go config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) + } + } } - pingHandler := func(m *nats.Msg) { + pingHandler := func(req *Request) { response, _ := json.Marshal(ping) - m.Respond(response) + if err := req.Respond(response); err != nil { + if err := req.Error("500", fmt.Sprintf("Error handling PING request: %s", err)); err != nil && config.ErrorHandler != nil { + go config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) + } + } } - statusHandler := func(m *nats.Msg) { + statsHandler := func(req *Request) { response, _ := json.Marshal(svc.Stats()) - m.Respond(response) + if err := req.Respond(response); err != nil { + if err := req.Error("500", fmt.Sprintf("Error handling STATS request: %s", err)); err != nil && config.ErrorHandler != nil { + go config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) + } + } } - schemaHandler := func(m *nats.Msg) { + schemaHandler := func(req *Request) { response, _ := json.Marshal(svc.Schema) - m.Respond(response) + if err := req.Respond(response); err != nil { + if err := req.Error("500", fmt.Sprintf("Error handling SCHEMA request: %s", err)); err != nil && config.ErrorHandler != nil { + go config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) + } + } } if err := svc.verbHandlers(nc, InfoVerb, infoHandler); err != nil { @@ -258,7 +306,7 @@ func Add(nc *nats.Conn, config Config) (*Service, error) { if err := svc.verbHandlers(nc, PingVerb, pingHandler); err != nil { return nil, err } - if err := svc.verbHandlers(nc, StatsVerb, statusHandler); err != nil { + if err := svc.verbHandlers(nc, StatsVerb, statsHandler); err != nil { return nil, err } @@ -268,80 +316,100 @@ func Add(nc *nats.Conn, config Config) (*Service, error) { } } - svc.natsHandlers.closed = nc.Opts.ClosedCB - if nc.Opts.ClosedCB != nil { - nc.SetClosedHandler(func(c *nats.Conn) { + return svc, nil +} + +func (s *Config) valid() error { + if !serviceNameRegexp.MatchString(s.Name) { + return fmt.Errorf("%w: service name: name should not be empty and should consist of alphanumerical charactest, dashes and underscores", ErrConfigValidation) + } + return s.Endpoint.valid() +} + +func (e *Endpoint) valid() error { + if e.Subject == "" { + return fmt.Errorf("%w: endpoint: subject is required", ErrConfigValidation) + } + if e.Handler == nil { + return fmt.Errorf("%w: endpoint: handler is required", ErrConfigValidation) + } + return nil +} + +func (svc *service) setupAsyncCallbacks() { + svc.natsHandlers.closed = svc.conn.Opts.ClosedCB + if svc.conn.Opts.ClosedCB != nil { + svc.conn.SetClosedHandler(func(c *nats.Conn) { svc.Stop() svc.natsHandlers.closed(c) }) } else { - nc.SetClosedHandler(func(c *nats.Conn) { + svc.conn.SetClosedHandler(func(c *nats.Conn) { svc.Stop() }) } - svc.natsHandlers.asyncErr = nc.Opts.AsyncErrorCB - if nc.Opts.AsyncErrorCB != nil { - nc.SetErrorHandler(func(c *nats.Conn, s *nats.Subscription, err error) { - if config.ErrorHandler != nil { - config.ErrorHandler(svc, &NATSError{ - Description: err.Error(), + svc.natsHandlers.asyncErr = svc.conn.Opts.AsyncErrorCB + if svc.conn.Opts.AsyncErrorCB != nil { + svc.conn.SetErrorHandler(func(c *nats.Conn, s *nats.Subscription, err error) { + if !svc.matchSubscriptionSubject(s.Subject) { + svc.natsHandlers.asyncErr(c, s, err) + } + if svc.Config.ErrorHandler != nil { + svc.Config.ErrorHandler(svc, &NATSError{ Subject: s.Subject, + Description: err.Error(), }) } svc.Stop() svc.natsHandlers.asyncErr(c, s, err) }) } else { - nc.SetErrorHandler(func(c *nats.Conn, s *nats.Subscription, err error) { - if config.ErrorHandler != nil { - config.ErrorHandler(svc, &NATSError{ - Description: err.Error(), + svc.conn.SetErrorHandler(func(c *nats.Conn, s *nats.Subscription, err error) { + if !svc.matchSubscriptionSubject(s.Subject) { + return + } + if svc.Config.ErrorHandler != nil { + svc.Config.ErrorHandler(svc, &NATSError{ Subject: s.Subject, + Description: err.Error(), }) } svc.Stop() }) } - - return svc, nil } -func (s *Config) valid() error { - if !serviceNameRegexp.MatchString(s.Name) { - return fmt.Errorf("%w: service name: name should not be empty and should consist of alphanumerical charactest, dashes and underscores", ErrConfigValidation) +func (svc *service) matchSubscriptionSubject(subj string) bool { + if svc.reqSub.Subject == subj { + return true } - return s.Endpoint.valid() -} - -func (e *Endpoint) valid() error { - if e.Subject == "" { - return fmt.Errorf("%w: endpoint: subject is required", ErrConfigValidation) - } - if e.Handler == nil { - return fmt.Errorf("%w: endpoint: handler is required", ErrConfigValidation) + for _, verbSub := range svc.verbSubs { + if verbSub.Subject == subj { + return true + } } - return nil + return false } // verbHandlers generates control handlers for a specific verb. // Each request generates 3 subscriptions, one for the general verb // affecting all services written with the framework, one that handles // all services of a particular kind, and finally a specific service instance. -func (svc *Service) verbHandlers(nc *nats.Conn, verb Verb, handler nats.MsgHandler) error { +func (svc *service) verbHandlers(nc *nats.Conn, verb Verb, handler RequestHandler) error { name := fmt.Sprintf("%s-all", verb.String()) if err := svc.addInternalHandler(nc, verb, "", "", name, handler); err != nil { return err } name = fmt.Sprintf("%s-kind", verb.String()) - if err := svc.addInternalHandler(nc, verb, svc.Name, "", name, handler); err != nil { + if err := svc.addInternalHandler(nc, verb, svc.Config.Name, "", name, handler); err != nil { return err } - return svc.addInternalHandler(nc, verb, svc.Name, svc.ID(), verb.String(), handler) + return svc.addInternalHandler(nc, verb, svc.Config.Name, svc.ID(), verb.String(), handler) } // addInternalHandler registers a control subject handler. -func (s *Service) addInternalHandler(nc *nats.Conn, verb Verb, kind, id, name string, handler nats.MsgHandler) error { +func (s *service) addInternalHandler(nc *nats.Conn, verb Verb, kind, id, name string, handler RequestHandler) error { subj, err := ControlSubject(verb, kind, id) if err != nil { s.Stop() @@ -350,7 +418,7 @@ func (s *Service) addInternalHandler(nc *nats.Conn, verb Verb, kind, id, name st s.verbSubs[name], err = nc.Subscribe(subj, func(msg *nats.Msg) { start := time.Now() - handler(msg) + handler(&Request{Msg: msg}) s.m.Lock() stats := s.endpointStats[name] @@ -371,7 +439,7 @@ func (s *Service) addInternalHandler(nc *nats.Conn, verb Verb, kind, id, name st } // reqHandler itself -func (s *Service) reqHandler(req *Request) { +func (s *service) reqHandler(req *Request) { start := time.Now() s.Endpoint.Handler(req) s.m.Lock() @@ -388,7 +456,7 @@ func (s *Service) reqHandler(req *Request) { } // Stop drains the endpoint subscriptions and marks the service as stopped. -func (s *Service) Stop() error { +func (s *service) Stop() error { s.m.Lock() if s.stopped { return nil @@ -413,7 +481,7 @@ func (s *Service) Stop() error { restoreAsyncHandlers(s.conn, s.natsHandlers) s.stopped = true if s.DoneHandler != nil { - s.DoneHandler(s) + go s.DoneHandler(s) } return nil } @@ -424,12 +492,27 @@ func restoreAsyncHandlers(nc *nats.Conn, handlers handlers) { } // ID returns the service instance's unique ID. -func (s *Service) ID() string { +func (s *service) ID() string { return s.id } +// ID returns the service instance's unique ID. +func (s *service) Name() string { + return s.Config.Name +} + +// ID returns the service instance's unique ID. +func (s *service) Version() string { + return s.Config.Version +} + +// ID returns the service instance's unique ID. +func (s *service) Description() string { + return s.Config.Description +} + // Stats returns statisctics for the service endpoint and all monitoring endpoints. -func (s *Service) Stats() Stats { +func (s *service) Stats() Stats { s.m.Lock() defer s.m.Unlock() if s.StatsHandler != nil { @@ -443,15 +526,15 @@ func (s *Service) Stats() Stats { idx++ } return Stats{ - Name: s.Name, + Name: s.Config.Name, ID: s.ID(), - Version: s.Version, + Version: s.Config.Version, Endpoints: v, } } // Reset resets all statistics on a service instance. -func (s *Service) Reset() { +func (s *service) Reset() { s.m.Lock() for _, se := range s.endpointStats { se.NumRequests = 0 @@ -465,7 +548,7 @@ func (s *Service) Reset() { } // Stopped informs whether [Stop] was executed on the service. -func (s *Service) Stopped() bool { +func (s *service) Stopped() bool { s.m.Lock() defer s.m.Unlock() return s.stopped @@ -482,6 +565,9 @@ func ControlSubject(verb Verb, name, id string) (string, error) { if verbStr == "" { return "", fmt.Errorf("%w: %q", ErrVerbNotSupported, verbStr) } + if name == "" && id != "" { + return "", ErrServiceNameRequired + } name = strings.ToUpper(name) if name == "" && id == "" { return fmt.Sprintf("%s.%s", APIPrefix, verbStr), nil diff --git a/service/service_test.go b/micro/service_test.go similarity index 58% rename from service/service_test.go rename to micro/service_test.go index ecb35ac08..768591a70 100644 --- a/service/service_test.go +++ b/micro/service_test.go @@ -11,7 +11,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package service +package micro import ( "bytes" @@ -19,6 +19,7 @@ import ( "errors" "fmt" "math/rand" + "reflect" "testing" "time" @@ -56,7 +57,7 @@ func TestServiceBasics(t *testing.T) { } } - var svcs []*Service + var svcs []Service // Create 5 service responders. config := Config{ @@ -71,7 +72,7 @@ func TestServiceBasics(t *testing.T) { } for i := 0; i < 5; i++ { - svc, err := Add(nc, config) + svc, err := AddService(nc, config) if err != nil { t.Fatalf("Expected to create Service, got %v", err) } @@ -88,10 +89,10 @@ func TestServiceBasics(t *testing.T) { } for _, svc := range svcs { - if svc.Name != "CoolAddService" { - t.Fatalf("Expected %q, got %q", "CoolAddService", svc.Name) + if svc.Name() != "CoolAddService" { + t.Fatalf("Expected %q, got %q", "CoolAddService", svc.Name()) } - if len(svc.Description) == 0 || len(svc.Version) == 0 { + if len(svc.Description()) == 0 || len(svc.Version()) == 0 { t.Fatalf("Expected non empty description and version") } } @@ -205,6 +206,7 @@ func TestAddService(t *testing.T) { givenConfig Config natsClosedHandler nats.ConnHandler natsErrorHandler nats.ErrHandler + asyncErrorSubject string expectedPing Ping withError error }{ @@ -229,7 +231,7 @@ func TestAddService(t *testing.T) { Subject: "test.sub", Handler: testHandler, }, - DoneHandler: func(*Service) { + DoneHandler: func(Service) { doneService <- struct{}{} }, }, @@ -245,13 +247,31 @@ func TestAddService(t *testing.T) { Subject: "test.sub", Handler: testHandler, }, - ErrorHandler: func(*Service, *NATSError) { + ErrorHandler: func(Service, *NATSError) { errService <- struct{}{} }, }, expectedPing: Ping{ Name: "test_service", }, + asyncErrorSubject: "test.sub", + }, + { + name: "with error handler, no handlers on nats connection, error on monitoring subject", + givenConfig: Config{ + Name: "test_service", + Endpoint: Endpoint{ + Subject: "test.sub", + Handler: testHandler, + }, + ErrorHandler: func(Service, *NATSError) { + errService <- struct{}{} + }, + }, + expectedPing: Ping{ + Name: "test_service", + }, + asyncErrorSubject: "$SVC.PING.TEST_SERVICE", }, { name: "with done handler, append to nats handlers", @@ -261,7 +281,7 @@ func TestAddService(t *testing.T) { Subject: "test.sub", Handler: testHandler, }, - DoneHandler: func(*Service) { + DoneHandler: func(Service) { doneService <- struct{}{} }, }, @@ -274,6 +294,7 @@ func TestAddService(t *testing.T) { expectedPing: Ping{ Name: "test_service", }, + asyncErrorSubject: "test.sub", }, { name: "with error handler, append to nats handlers", @@ -283,7 +304,7 @@ func TestAddService(t *testing.T) { Subject: "test.sub", Handler: testHandler, }, - DoneHandler: func(*Service) { + DoneHandler: func(Service) { doneService <- struct{}{} }, }, @@ -297,6 +318,29 @@ func TestAddService(t *testing.T) { Name: "test_service", }, }, + { + name: "with error handler, append to nats handlers, error on monitoring subject", + givenConfig: Config{ + Name: "test_service", + Endpoint: Endpoint{ + Subject: "test.sub", + Handler: testHandler, + }, + DoneHandler: func(Service) { + doneService <- struct{}{} + }, + }, + natsClosedHandler: func(c *nats.Conn) { + closedNats <- struct{}{} + }, + natsErrorHandler: func(*nats.Conn, *nats.Subscription, error) { + errNats <- struct{}{} + }, + expectedPing: Ping{ + Name: "test_service", + }, + asyncErrorSubject: "$SVC.PING.TEST_SERVICE", + }, { name: "validation error, invalid service name", givenConfig: Config{ @@ -346,7 +390,7 @@ func TestAddService(t *testing.T) { } defer nc.Close() - srv, err := Add(nc, test.givenConfig) + srv, err := AddService(nc, test.givenConfig) if test.withError != nil { if !errors.Is(err, test.withError) { t.Fatalf("Expected error: %v; got: %v", test.withError, err) @@ -354,7 +398,7 @@ func TestAddService(t *testing.T) { return } - pingSubject, err := ControlSubject(PingVerb, srv.Name, srv.ID()) + pingSubject, err := ControlSubject(PingVerb, srv.Name(), srv.ID()) if err != nil { t.Fatalf("Unexpected error: %v", err) } @@ -389,7 +433,7 @@ func TestAddService(t *testing.T) { } if test.givenConfig.ErrorHandler != nil { - go nc.Opts.AsyncErrorCB(nc, &nats.Subscription{Subject: "test.sub"}, fmt.Errorf("oops")) + go nc.Opts.AsyncErrorCB(nc, &nats.Subscription{Subject: test.asyncErrorSubject}, fmt.Errorf("oops")) select { case <-errService: case <-time.After(1 * time.Second): @@ -421,7 +465,7 @@ func TestAddService(t *testing.T) { } } if test.natsErrorHandler != nil { - go nc.Opts.AsyncErrorCB(nc, &nats.Subscription{Subject: "test.sub"}, fmt.Errorf("oops")) + go nc.Opts.AsyncErrorCB(nc, &nats.Subscription{Subject: test.asyncErrorSubject}, fmt.Errorf("oops")) select { case <-errService: t.Fatalf("Expected to restore nats error handler") @@ -437,6 +481,308 @@ func TestAddService(t *testing.T) { } } +func TestMonitoringHandlers(t *testing.T) { + s := RunServerOnPort(-1) + defer s.Shutdown() + + nc, err := nats.Connect(s.ClientURL()) + if err != nil { + t.Fatalf("Expected to connect to server, got %v", err) + } + defer nc.Close() + + asyncErr := make(chan struct{}) + errHandler := func(s Service, n *NATSError) { + asyncErr <- struct{}{} + } + + config := Config{ + Name: "test_service", + Endpoint: Endpoint{ + Subject: "test.sub", + Handler: func(*Request) {}, + }, + Schema: Schema{ + Request: "some_schema", + }, + ErrorHandler: errHandler, + } + srv, err := AddService(nc, config) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + defer func() { + srv.Stop() + if !srv.Stopped() { + t.Fatalf("Expected service to be stopped") + } + }() + + tests := []struct { + name string + subject string + withError bool + expectedResponse interface{} + }{ + { + name: "PING all", + subject: "$SRV.PING", + expectedResponse: Ping{ + Name: "test_service", + ID: srv.ID(), + }, + }, + { + name: "PING name", + subject: "$SRV.PING.TEST_SERVICE", + expectedResponse: Ping{ + Name: "test_service", + ID: srv.ID(), + }, + }, + { + name: "PING ID", + subject: fmt.Sprintf("$SRV.PING.TEST_SERVICE.%s", srv.ID()), + expectedResponse: Ping{ + Name: "test_service", + ID: srv.ID(), + }, + }, + { + name: "INFO all", + subject: "$SRV.INFO", + expectedResponse: Info{ + Name: "test_service", + ID: srv.ID(), + Subject: "test.sub", + }, + }, + { + name: "INFO name", + subject: "$SRV.INFO.TEST_SERVICE", + expectedResponse: Info{ + Name: "test_service", + ID: srv.ID(), + Subject: "test.sub", + }, + }, + { + name: "INFO ID", + subject: fmt.Sprintf("$SRV.INFO.TEST_SERVICE.%s", srv.ID()), + expectedResponse: Info{ + Name: "test_service", + ID: srv.ID(), + Subject: "test.sub", + }, + }, + { + name: "SCHEMA all", + subject: "$SRV.SCHEMA", + expectedResponse: Schema{ + Request: "some_schema", + }, + }, + { + name: "SCHEMA name", + subject: "$SRV.SCHEMA.TEST_SERVICE", + expectedResponse: Schema{ + Request: "some_schema", + }, + }, + { + name: "SCHEMA ID", + subject: fmt.Sprintf("$SRV.SCHEMA.TEST_SERVICE.%s", srv.ID()), + expectedResponse: Schema{ + Request: "some_schema", + }, + }, + { + name: "PING error", + subject: "$SRV.PING", + withError: true, + }, + { + name: "INFO error", + subject: "$SRV.INFO", + withError: true, + }, + { + name: "STATS error", + subject: "$SRV.STATS", + withError: true, + }, + { + name: "SCHEMA error", + subject: "$SRV.SCHEMA", + withError: true, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if test.withError { + // use publish instead of request, so Respond will fail inside the handler + if err := nc.Publish(test.subject, nil); err != nil { + t.Fatalf("Unexpected error: %v", err) + } + select { + case <-asyncErr: + return + case <-time.After(1 * time.Second): + t.Fatalf("Timeout waiting for async error") + } + return + } + + resp, err := nc.Request(test.subject, nil, 1*time.Second) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + respMap := make(map[string]interface{}) + if err := json.Unmarshal(resp.Data, &respMap); err != nil { + t.Fatalf("Unexpected error: %v", err) + } + expectedResponseJSON, err := json.Marshal(test.expectedResponse) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + expectedRespMap := make(map[string]interface{}) + if err := json.Unmarshal(expectedResponseJSON, &expectedRespMap); err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if !reflect.DeepEqual(respMap, expectedRespMap) { + t.Fatalf("Invalid response; want: %+v; got: %+v", expectedRespMap, respMap) + } + }) + } +} + +func TestServiceStats(t *testing.T) { + tests := []struct { + name string + config Config + expectedEndpointsLen int + expectedStats map[string]interface{} + }{ + { + name: "without schema or stats handler", + config: Config{ + Name: "test_service", + Endpoint: Endpoint{ + Subject: "test.sub", + Handler: func(*Request) {}, + }, + }, + expectedEndpointsLen: 10, + }, + { + name: "with stats handler", + config: Config{ + Name: "test_service", + Endpoint: Endpoint{ + Subject: "test.sub", + Handler: func(*Request) {}, + }, + StatsHandler: func(e Endpoint) interface{} { + return map[string]interface{}{ + "key": "val", + } + }, + }, + expectedEndpointsLen: 10, + expectedStats: map[string]interface{}{ + "key": "val", + }, + }, + { + name: "with schema", + config: Config{ + Name: "test_service", + Endpoint: Endpoint{ + Subject: "test.sub", + Handler: func(*Request) {}, + }, + Schema: Schema{ + Request: "some_schema", + }, + }, + expectedEndpointsLen: 13, + }, + { + name: "with schema and stats handler", + config: Config{ + Name: "test_service", + Endpoint: Endpoint{ + Subject: "test.sub", + Handler: func(*Request) {}, + }, + Schema: Schema{ + Request: "some_schema", + }, + StatsHandler: func(e Endpoint) interface{} { + return map[string]interface{}{ + "key": "val", + } + }, + }, + expectedEndpointsLen: 13, + expectedStats: map[string]interface{}{ + "key": "val", + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + s := RunServerOnPort(-1) + defer s.Shutdown() + + nc, err := nats.Connect(s.ClientURL()) + if err != nil { + t.Fatalf("Expected to connect to server, got %v", err) + } + defer nc.Close() + + srv, err := AddService(nc, test.config) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + defer srv.Stop() + + resp, err := nc.Request(fmt.Sprintf("$SRV.STATS.TEST_SERVICE.%s", srv.ID()), nil, 1*time.Second) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + var stats Stats + if err := json.Unmarshal(resp.Data, &stats); err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + if len(stats.Endpoints) != test.expectedEndpointsLen { + t.Errorf("Unexpected endpoint count; want: %d; got: %d", test.expectedEndpointsLen, len(stats.Endpoints)) + } + if stats.Name != srv.Name() { + t.Errorf("Unexpected service name; want: %s; got: %s", srv.Name(), stats.Name) + } + if stats.ID != srv.ID() { + t.Errorf("Unexpected service name; want: %s; got: %s", srv.ID(), stats.ID) + } + if test.expectedStats != nil { + for _, e := range stats.Endpoints { + if e.Name != "test_service" { + continue + } + if val, ok := e.Data.(map[string]interface{}); !ok || !reflect.DeepEqual(val, test.expectedStats) { + t.Fatalf("Invalid data from stats handler; want: %v; got: %v", test.expectedStats, val) + } + } + } + }) + } +} + func TestRequestRespond(t *testing.T) { type x struct { A string `json:"a"` @@ -551,7 +897,7 @@ func TestRequestRespond(t *testing.T) { } } - svc, err := Add(nc, Config{ + svc, err := AddService(nc, Config{ Name: "CoolService", Description: "Erroring service", Endpoint: Endpoint{ @@ -604,3 +950,63 @@ func RunServerOnPort(port int) *server.Server { func RunServerWithOptions(opts *server.Options) *server.Server { return natsserver.RunServer(opts) } + +func TestControlSubject(t *testing.T) { + tests := []struct { + name string + verb Verb + srvName string + id string + expectedSubject string + withError error + }{ + { + name: "PING ALL", + verb: PingVerb, + expectedSubject: "$SRV.PING", + }, + { + name: "PING name", + verb: PingVerb, + srvName: "test", + expectedSubject: "$SRV.PING.TEST", + }, + { + name: "PING id", + verb: PingVerb, + srvName: "test", + id: "123", + expectedSubject: "$SRV.PING.TEST.123", + }, + { + name: "invalid verb", + verb: Verb(100), + withError: ErrVerbNotSupported, + }, + { + name: "name not provided", + verb: PingVerb, + srvName: "", + id: "123", + withError: ErrServiceNameRequired, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + res, err := ControlSubject(test.verb, test.srvName, test.id) + if test.withError != nil { + if !errors.Is(err, test.withError) { + t.Fatalf("Expected error: %v; got: %v", test.withError, err) + } + return + } + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if res != test.expectedSubject { + t.Errorf("Invalid subject; want: %q; got: %q", test.expectedSubject, res) + } + }) + } +} From 972dcd256821b9afdce77c4707992913b81c44f3 Mon Sep 17 00:00:00 2001 From: Piotr Piotrowski Date: Wed, 14 Dec 2022 18:54:34 +0100 Subject: [PATCH 06/14] Adopt ADR changes --- micro/example_package_test.go | 2 +- micro/example_test.go | 46 ++++--- micro/service.go | 177 ++++++++++++--------------- micro/service_test.go | 221 +++++++++++++++++++++------------- 4 files changed, 244 insertions(+), 202 deletions(-) diff --git a/micro/example_package_test.go b/micro/example_package_test.go index 1e919aa84..b5b27d8fb 100644 --- a/micro/example_package_test.go +++ b/micro/example_package_test.go @@ -34,7 +34,7 @@ func Example() { config := Config{ Name: "IncrementService", - Version: "v0.1.0", + Version: "0.1.0", Description: "Increment numbers", Endpoint: Endpoint{ // service handler diff --git a/micro/example_test.go b/micro/example_test.go index df657cfaa..366b7eb70 100644 --- a/micro/example_test.go +++ b/micro/example_test.go @@ -29,12 +29,14 @@ func ExampleAddService() { // DoneHandler can be set to customize behavior on stopping a service. DoneHandler: func(srv Service) { - fmt.Printf("stopped service %q with ID %q\n", srv.Name(), srv.ID()) + info := srv.Info() + fmt.Printf("stopped service %q with ID %q\n", info.Name, info.ID) }, // ErrorHandler can be used to customize behavior on service execution error. ErrorHandler: func(srv Service, err *NATSError) { - fmt.Printf("Service %q returned an error on subject %q: %s", srv.Name(), err.Subject, err.Description) + info := srv.Info() + fmt.Printf("Service %q returned an error on subject %q: %s", info.Name, err.Subject, err.Description) }, } @@ -45,7 +47,7 @@ func ExampleAddService() { defer srv.Stop() } -func ExampleService_ID() { +func ExampleService_Info() { nc, err := nats.Connect("127.0.0.1:4222") if err != nil { log.Fatal(err) @@ -62,9 +64,14 @@ func ExampleService_ID() { srv, _ := AddService(nc, config) - // unique service ID - id := srv.ID() - fmt.Println(id) + // service info + info := srv.Info() + + fmt.Println(info.ID) + fmt.Println(info.Name) + fmt.Println(info.Description) + fmt.Println(info.Version) + fmt.Println(info.Subject) } func ExampleService_Stats() { @@ -75,7 +82,8 @@ func ExampleService_Stats() { defer nc.Close() config := Config{ - Name: "EchoService", + Name: "EchoService", + Version: "0.1.0", Endpoint: Endpoint{ Subject: "echo", Handler: func(*Request) {}, @@ -87,9 +95,8 @@ func ExampleService_Stats() { // stats of a service instance stats := srv.Stats() - for _, e := range stats.Endpoints { - fmt.Println(e.AverageProcessingTime) - } + fmt.Println(stats.AverageProcessingTime) + fmt.Println(stats.TotalProcessingTime) } @@ -101,7 +108,8 @@ func ExampleService_Stop() { defer nc.Close() config := Config{ - Name: "EchoService", + Name: "EchoService", + Version: "0.1.0", Endpoint: Endpoint{ Subject: "echo", Handler: func(*Request) {}, @@ -131,7 +139,8 @@ func ExampleService_Stopped() { defer nc.Close() config := Config{ - Name: "EchoService", + Name: "EchoService", + Version: "0.1.0", Endpoint: Endpoint{ Subject: "echo", Handler: func(*Request) {}, @@ -159,7 +168,8 @@ func ExampleService_Reset() { defer nc.Close() config := Config{ - Name: "EchoService", + Name: "EchoService", + Version: "0.1.0", Endpoint: Endpoint{ Subject: "echo", Handler: func(*Request) {}, @@ -171,11 +181,11 @@ func ExampleService_Reset() { // reset endpoint stats on this service srv.Reset() - empty := EndpointStats{Name: "EchoService"} - for _, e := range srv.Stats().Endpoints { - if e != empty { - log.Fatal("Expected endpoint stats to be empty") - } + empty := Stats{ + ServiceIdentity: srv.Info().ServiceIdentity, + } + if srv.Stats() != empty { + log.Fatal("Expected endpoint stats to be empty") } } diff --git a/micro/service.go b/micro/service.go index dcab61d48..a1a18a3f3 100644 --- a/micro/service.go +++ b/micro/service.go @@ -32,18 +32,8 @@ import ( type ( Service interface { - // ID returns the service instance's unique ID. - ID() string - - // Name returns the name of the service. - // It can be shared between multiple service instances. - Name() string - - // Description returns the service description. - Description() string - - // Version returns the service version. - Version() string + // Info returns the service info. + Info() Info // Stats returns statisctics for the service endpoint and all monitoring endpoints. Stats() Stats @@ -65,14 +55,14 @@ type ( // Config contains a configuration of the service Config - m sync.Mutex - id string - reqSub *nats.Subscription - verbSubs map[string]*nats.Subscription - endpointStats map[string]*EndpointStats - conn *nats.Conn - natsHandlers handlers - stopped bool + m sync.Mutex + id string + reqSub *nats.Subscription + verbSubs map[string]*nats.Subscription + stats *Stats + conn *nats.Conn + natsHandlers handlers + stopped bool } // ErrHandler is a function used to configure a custom error handler for a service, @@ -85,42 +75,44 @@ type ( // It should return a value which can be serialized to JSON. StatsHandler func(Endpoint) interface{} + // ServiceIdentity contains fields helping to identidy a service instance. + ServiceIdentity struct { + Name string `json:"name"` + ID string `json:"id"` + Version string `json:"version"` + } + // Stats is the type returned by STATS monitoring endpoint. // It contains a slice of [EndpointStats], providing stats // for both the service handler as well as all monitoring subjects. - Stats struct { - Name string `json:"name"` - ID string `json:"id"` - Version string `json:"version"` - Endpoints []EndpointStats `json:"stats"` - } - // EndpointStats are stats for a specific endpoint (either request handler or monitoring enpoints). // It contains general statisctics for an endpoint, as well as a custom value returned by optional [StatsHandler]. - EndpointStats struct { - Name string `json:"name"` + Stats struct { + ServiceIdentity NumRequests int `json:"num_requests"` NumErrors int `json:"num_errors"` TotalProcessingTime time.Duration `json:"total_processing_time"` AverageProcessingTime time.Duration `json:"average_processing_time"` - Data interface{} `json:"data"` + Started string `json:"started"` + Data interface{} `json:"data,omitempty"` } // Ping is the response type for PING monitoring endpoint. - Ping struct { - Name string `json:"name"` - ID string `json:"id"` - } + Ping ServiceIdentity // Info is the basic information about a service type. Info struct { - Name string `json:"name"` - ID string `json:"id"` + ServiceIdentity Description string `json:"description"` - Version string `json:"version"` Subject string `json:"subject"` } + // SchemaResp is the response value for SCHEMA requests. + SchemaResp struct { + ServiceIdentity + Schema + } + // Schema can be used to configure a schema for a service. // It is olso returned by the SCHEMA monitoring service (if set). Schema struct { @@ -186,6 +178,8 @@ const ( ) var ( + // this regular expression is suggested regexp for semver validation: https://semver.org/ + semVerRegexp = regexp.MustCompile(`^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$`) serviceNameRegexp = regexp.MustCompile(`^[A-Za-z0-9\-_]+$`) ) @@ -234,9 +228,12 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { id: id, } svc.verbSubs = make(map[string]*nats.Subscription) - svc.endpointStats = make(map[string]*EndpointStats) - svc.endpointStats[""] = &EndpointStats{ - Name: config.Name, + svc.stats = &Stats{ + ServiceIdentity: ServiceIdentity{ + Name: config.Name, + ID: id, + Version: config.Version, + }, } svc.setupAsyncCallbacks() @@ -251,21 +248,14 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { return nil, err } - info := &Info{ - Name: config.Name, - ID: id, - Description: config.Description, - Version: config.Version, - Subject: config.Endpoint.Subject, - } - ping := &Ping{ - Name: config.Name, - ID: id, + Name: config.Name, + ID: id, + Version: config.Version, } infoHandler := func(req *Request) { - response, _ := json.Marshal(info) + response, _ := json.Marshal(svc.Info()) if err := req.Respond(response); err != nil { if err := req.Error("500", fmt.Sprintf("Error handling INFO request: %s", err)); err != nil && config.ErrorHandler != nil { go config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) @@ -315,6 +305,7 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { return nil, err } } + svc.stats.Started = time.Now().Format(time.RFC3339) return svc, nil } @@ -323,6 +314,9 @@ func (s *Config) valid() error { if !serviceNameRegexp.MatchString(s.Name) { return fmt.Errorf("%w: service name: name should not be empty and should consist of alphanumerical charactest, dashes and underscores", ErrConfigValidation) } + if !semVerRegexp.MatchString(s.Version) { + return fmt.Errorf("%w: version: version should not be empty should match the SemVer format", ErrConfigValidation) + } return s.Endpoint.valid() } @@ -405,7 +399,7 @@ func (svc *service) verbHandlers(nc *nats.Conn, verb Verb, handler RequestHandle if err := svc.addInternalHandler(nc, verb, svc.Config.Name, "", name, handler); err != nil { return err } - return svc.addInternalHandler(nc, verb, svc.Config.Name, svc.ID(), verb.String(), handler) + return svc.addInternalHandler(nc, verb, svc.Config.Name, svc.id, verb.String(), handler) } // addInternalHandler registers a control subject handler. @@ -417,24 +411,12 @@ func (s *service) addInternalHandler(nc *nats.Conn, verb Verb, kind, id, name st } s.verbSubs[name], err = nc.Subscribe(subj, func(msg *nats.Msg) { - start := time.Now() handler(&Request{Msg: msg}) - - s.m.Lock() - stats := s.endpointStats[name] - stats.NumRequests++ - stats.TotalProcessingTime += time.Since(start) - stats.AverageProcessingTime = stats.TotalProcessingTime / time.Duration(stats.NumRequests) - s.m.Unlock() }) if err != nil { s.Stop() return err } - - s.endpointStats[name] = &EndpointStats{ - Name: name, - } return nil } @@ -443,14 +425,13 @@ func (s *service) reqHandler(req *Request) { start := time.Now() s.Endpoint.Handler(req) s.m.Lock() - stats := s.endpointStats[""] - stats.NumRequests++ - stats.TotalProcessingTime += time.Since(start) - stats.AverageProcessingTime = stats.TotalProcessingTime / time.Duration(stats.NumRequests) + s.stats.NumRequests++ + s.stats.TotalProcessingTime += time.Since(start) + avgProcessingTime := s.stats.TotalProcessingTime.Nanoseconds() / int64(s.stats.NumRequests) + s.stats.AverageProcessingTime = time.Duration(avgProcessingTime) if req.errResponse { - stats := s.endpointStats[""] - stats.NumErrors++ + s.stats.NumErrors++ } s.m.Unlock() } @@ -492,18 +473,16 @@ func restoreAsyncHandlers(nc *nats.Conn, handlers handlers) { } // ID returns the service instance's unique ID. -func (s *service) ID() string { - return s.id -} - -// ID returns the service instance's unique ID. -func (s *service) Name() string { - return s.Config.Name -} - -// ID returns the service instance's unique ID. -func (s *service) Version() string { - return s.Config.Version +func (s *service) Info() Info { + return Info{ + ServiceIdentity: ServiceIdentity{ + Name: s.Config.Name, + ID: s.id, + Version: s.Config.Version, + }, + Description: s.Config.Description, + Subject: s.Config.Endpoint.Subject, + } } // ID returns the service instance's unique ID. @@ -516,33 +495,29 @@ func (s *service) Stats() Stats { s.m.Lock() defer s.m.Unlock() if s.StatsHandler != nil { - stats := s.endpointStats[""] - stats.Data = s.StatsHandler(s.Endpoint) - } - idx := 0 - v := make([]EndpointStats, len(s.endpointStats)) - for _, se := range s.endpointStats { - v[idx] = *se - idx++ + s.stats.Data = s.StatsHandler(s.Endpoint) } + info := s.Info() return Stats{ - Name: s.Config.Name, - ID: s.ID(), - Version: s.Config.Version, - Endpoints: v, + ServiceIdentity: ServiceIdentity{ + Name: info.Name, + ID: info.ID, + Version: info.Version, + }, + NumRequests: s.stats.NumRequests, + NumErrors: s.stats.NumErrors, + TotalProcessingTime: s.stats.TotalProcessingTime, + AverageProcessingTime: s.stats.AverageProcessingTime, + Started: s.stats.Started, + Data: s.stats.Data, } } // Reset resets all statistics on a service instance. func (s *service) Reset() { s.m.Lock() - for _, se := range s.endpointStats { - se.NumRequests = 0 - se.TotalProcessingTime = 0 - se.AverageProcessingTime = 0 - se.Data = nil - se.NumErrors = 0 - se.Data = nil + s.stats = &Stats{ + ServiceIdentity: s.Info().ServiceIdentity, } s.m.Unlock() } diff --git a/micro/service_test.go b/micro/service_test.go index 768591a70..27c647bb8 100644 --- a/micro/service_test.go +++ b/micro/service_test.go @@ -62,7 +62,7 @@ func TestServiceBasics(t *testing.T) { // Create 5 service responders. config := Config{ Name: "CoolAddService", - Version: "v0.1", + Version: "0.1.0", Description: "Add things together", Endpoint: Endpoint{ Subject: "svc.add", @@ -89,10 +89,11 @@ func TestServiceBasics(t *testing.T) { } for _, svc := range svcs { - if svc.Name() != "CoolAddService" { - t.Fatalf("Expected %q, got %q", "CoolAddService", svc.Name()) + info := svc.Info() + if info.Name != "CoolAddService" { + t.Fatalf("Expected %q, got %q", "CoolAddService", info.Name) } - if len(svc.Description()) == 0 || len(svc.Version()) == 0 { + if len(info.Description) == 0 || len(info.Version) == 0 { t.Fatalf("Expected non empty description and version") } } @@ -166,14 +167,7 @@ func TestServiceBasics(t *testing.T) { if err := json.Unmarshal(resp.Data, &srvStats); err != nil { t.Fatalf("Unexpected error: %v", err) } - if len(srvStats.Endpoints) != 10 { - t.Fatalf("Expected 10 endpoints on a serivce, got: %d", len(srvStats.Endpoints)) - } - for _, e := range srvStats.Endpoints { - if e.Name == "CoolAddService" { - requestsNum += e.NumRequests - } - } + requestsNum += srvStats.NumRequests stats = append(stats, srvStats) } if len(stats) != 5 { @@ -186,12 +180,14 @@ func TestServiceBasics(t *testing.T) { } // Reset stats for a service svcs[0].Reset() - for _, e := range svcs[0].Stats().Endpoints { - emptyStats := EndpointStats{Name: e.Name} - if e != emptyStats { - t.Fatalf("Expected empty stats after reset; got: %+v", e) - } + emptyStats := Stats{ + ServiceIdentity: svcs[0].Info().ServiceIdentity, } + + if svcs[0].Stats() != emptyStats { + t.Fatalf("Expected empty stats after reset; got: %+v", svcs[0].Stats()) + } + } func TestAddService(t *testing.T) { @@ -213,20 +209,23 @@ func TestAddService(t *testing.T) { { name: "minimal config", givenConfig: Config{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", Endpoint: Endpoint{ Subject: "test.sub", Handler: testHandler, }, }, expectedPing: Ping{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", }, }, { name: "with done handler, no handlers on nats connection", givenConfig: Config{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", Endpoint: Endpoint{ Subject: "test.sub", Handler: testHandler, @@ -236,13 +235,15 @@ func TestAddService(t *testing.T) { }, }, expectedPing: Ping{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", }, }, { name: "with error handler, no handlers on nats connection", givenConfig: Config{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", Endpoint: Endpoint{ Subject: "test.sub", Handler: testHandler, @@ -252,14 +253,16 @@ func TestAddService(t *testing.T) { }, }, expectedPing: Ping{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", }, asyncErrorSubject: "test.sub", }, { name: "with error handler, no handlers on nats connection, error on monitoring subject", givenConfig: Config{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", Endpoint: Endpoint{ Subject: "test.sub", Handler: testHandler, @@ -269,14 +272,16 @@ func TestAddService(t *testing.T) { }, }, expectedPing: Ping{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", }, asyncErrorSubject: "$SVC.PING.TEST_SERVICE", }, { name: "with done handler, append to nats handlers", givenConfig: Config{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", Endpoint: Endpoint{ Subject: "test.sub", Handler: testHandler, @@ -292,14 +297,16 @@ func TestAddService(t *testing.T) { errNats <- struct{}{} }, expectedPing: Ping{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", }, asyncErrorSubject: "test.sub", }, { name: "with error handler, append to nats handlers", givenConfig: Config{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", Endpoint: Endpoint{ Subject: "test.sub", Handler: testHandler, @@ -315,13 +322,15 @@ func TestAddService(t *testing.T) { errNats <- struct{}{} }, expectedPing: Ping{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", }, }, { name: "with error handler, append to nats handlers, error on monitoring subject", givenConfig: Config{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", Endpoint: Endpoint{ Subject: "test.sub", Handler: testHandler, @@ -337,14 +346,28 @@ func TestAddService(t *testing.T) { errNats <- struct{}{} }, expectedPing: Ping{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", }, asyncErrorSubject: "$SVC.PING.TEST_SERVICE", }, { name: "validation error, invalid service name", givenConfig: Config{ - Name: "test_service!", + Name: "test_service!", + Version: "0.1.0", + Endpoint: Endpoint{ + Subject: "test.sub", + Handler: testHandler, + }, + }, + withError: ErrConfigValidation, + }, + { + name: "validation error, invalid version", + givenConfig: Config{ + Name: "test_service!", + Version: "abc", Endpoint: Endpoint{ Subject: "test.sub", Handler: testHandler, @@ -355,7 +378,8 @@ func TestAddService(t *testing.T) { { name: "validation error, empty subject", givenConfig: Config{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", Endpoint: Endpoint{ Subject: "", Handler: testHandler, @@ -366,7 +390,8 @@ func TestAddService(t *testing.T) { { name: "validation error, no handler", givenConfig: Config{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", Endpoint: Endpoint{ Subject: "test_subject", Handler: nil, @@ -397,8 +422,12 @@ func TestAddService(t *testing.T) { } return } + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } - pingSubject, err := ControlSubject(PingVerb, srv.Name(), srv.ID()) + info := srv.Info() + pingSubject, err := ControlSubject(PingVerb, info.Name, info.ID) if err != nil { t.Fatalf("Unexpected error: %v", err) } @@ -411,7 +440,7 @@ func TestAddService(t *testing.T) { t.Fatalf("Unexpected error: %v", err) } - test.expectedPing.ID = srv.ID() + test.expectedPing.ID = info.ID if test.expectedPing != ping { t.Fatalf("Invalid ping response; want: %+v; got: %+v", test.expectedPing, ping) } @@ -497,7 +526,8 @@ func TestMonitoringHandlers(t *testing.T) { } config := Config{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", Endpoint: Endpoint{ Subject: "test.sub", Handler: func(*Request) {}, @@ -518,6 +548,8 @@ func TestMonitoringHandlers(t *testing.T) { } }() + info := srv.Info() + tests := []struct { name string subject string @@ -528,32 +560,38 @@ func TestMonitoringHandlers(t *testing.T) { name: "PING all", subject: "$SRV.PING", expectedResponse: Ping{ - Name: "test_service", - ID: srv.ID(), + Name: "test_service", + Version: "0.1.0", + ID: info.ID, }, }, { name: "PING name", subject: "$SRV.PING.TEST_SERVICE", expectedResponse: Ping{ - Name: "test_service", - ID: srv.ID(), + Name: "test_service", + Version: "0.1.0", + ID: info.ID, }, }, { name: "PING ID", - subject: fmt.Sprintf("$SRV.PING.TEST_SERVICE.%s", srv.ID()), + subject: fmt.Sprintf("$SRV.PING.TEST_SERVICE.%s", info.ID), expectedResponse: Ping{ - Name: "test_service", - ID: srv.ID(), + Name: "test_service", + Version: "0.1.0", + ID: info.ID, }, }, { name: "INFO all", subject: "$SRV.INFO", expectedResponse: Info{ - Name: "test_service", - ID: srv.ID(), + ServiceIdentity: ServiceIdentity{ + Name: "test_service", + Version: "0.1.0", + ID: info.ID, + }, Subject: "test.sub", }, }, @@ -561,17 +599,23 @@ func TestMonitoringHandlers(t *testing.T) { name: "INFO name", subject: "$SRV.INFO.TEST_SERVICE", expectedResponse: Info{ - Name: "test_service", - ID: srv.ID(), + ServiceIdentity: ServiceIdentity{ + Name: "test_service", + Version: "0.1.0", + ID: info.ID, + }, Subject: "test.sub", }, }, { name: "INFO ID", - subject: fmt.Sprintf("$SRV.INFO.TEST_SERVICE.%s", srv.ID()), + subject: fmt.Sprintf("$SRV.INFO.TEST_SERVICE.%s", info.ID), expectedResponse: Info{ - Name: "test_service", - ID: srv.ID(), + ServiceIdentity: ServiceIdentity{ + Name: "test_service", + Version: "0.1.0", + ID: info.ID, + }, Subject: "test.sub", }, }, @@ -591,7 +635,7 @@ func TestMonitoringHandlers(t *testing.T) { }, { name: "SCHEMA ID", - subject: fmt.Sprintf("$SRV.SCHEMA.TEST_SERVICE.%s", srv.ID()), + subject: fmt.Sprintf("$SRV.SCHEMA.TEST_SERVICE.%s", info.ID), expectedResponse: Schema{ Request: "some_schema", }, @@ -659,30 +703,36 @@ func TestMonitoringHandlers(t *testing.T) { } func TestServiceStats(t *testing.T) { + handler := func(r *Request) { + if bytes.Equal(r.Data, []byte("err")) { + r.Error("400", "bad request") + } + r.Respond([]byte("ok")) + } tests := []struct { - name string - config Config - expectedEndpointsLen int - expectedStats map[string]interface{} + name string + config Config + expectedStats map[string]interface{} }{ { name: "without schema or stats handler", config: Config{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", Endpoint: Endpoint{ Subject: "test.sub", - Handler: func(*Request) {}, + Handler: handler, }, }, - expectedEndpointsLen: 10, }, { name: "with stats handler", config: Config{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", Endpoint: Endpoint{ Subject: "test.sub", - Handler: func(*Request) {}, + Handler: handler, }, StatsHandler: func(e Endpoint) interface{} { return map[string]interface{}{ @@ -690,7 +740,6 @@ func TestServiceStats(t *testing.T) { } }, }, - expectedEndpointsLen: 10, expectedStats: map[string]interface{}{ "key": "val", }, @@ -698,24 +747,25 @@ func TestServiceStats(t *testing.T) { { name: "with schema", config: Config{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", Endpoint: Endpoint{ Subject: "test.sub", - Handler: func(*Request) {}, + Handler: handler, }, Schema: Schema{ Request: "some_schema", }, }, - expectedEndpointsLen: 13, }, { name: "with schema and stats handler", config: Config{ - Name: "test_service", + Name: "test_service", + Version: "0.1.0", Endpoint: Endpoint{ Subject: "test.sub", - Handler: func(*Request) {}, + Handler: handler, }, Schema: Schema{ Request: "some_schema", @@ -726,7 +776,6 @@ func TestServiceStats(t *testing.T) { } }, }, - expectedEndpointsLen: 13, expectedStats: map[string]interface{}{ "key": "val", }, @@ -749,8 +798,17 @@ func TestServiceStats(t *testing.T) { t.Fatalf("Unexpected error: %v", err) } defer srv.Stop() + for i := 0; i < 10; i++ { + if _, err := nc.Request(srv.Info().Subject, []byte("msg"), time.Second); err != nil { + t.Fatalf("Unexpected error: %v", err) + } + } + if _, err := nc.Request(srv.Info().Subject, []byte("err"), time.Second); err != nil { + t.Fatalf("Unexpected error: %v", err) + } - resp, err := nc.Request(fmt.Sprintf("$SRV.STATS.TEST_SERVICE.%s", srv.ID()), nil, 1*time.Second) + info := srv.Info() + resp, err := nc.Request(fmt.Sprintf("$SRV.STATS.TEST_SERVICE.%s", info.ID), nil, 1*time.Second) if err != nil { t.Fatalf("Unexpected error: %v", err) } @@ -760,23 +818,21 @@ func TestServiceStats(t *testing.T) { t.Fatalf("Unexpected error: %v", err) } - if len(stats.Endpoints) != test.expectedEndpointsLen { - t.Errorf("Unexpected endpoint count; want: %d; got: %d", test.expectedEndpointsLen, len(stats.Endpoints)) + if stats.Name != info.Name { + t.Errorf("Unexpected service name; want: %s; got: %s", info.Name, stats.Name) } - if stats.Name != srv.Name() { - t.Errorf("Unexpected service name; want: %s; got: %s", srv.Name(), stats.Name) + if stats.ID != info.ID { + t.Errorf("Unexpected service name; want: %s; got: %s", info.ID, stats.ID) } - if stats.ID != srv.ID() { - t.Errorf("Unexpected service name; want: %s; got: %s", srv.ID(), stats.ID) + if stats.NumRequests != 11 { + t.Errorf("Unexpected num_requests; want: 11; got: %d", stats.NumRequests) + } + if stats.NumErrors != 1 { + t.Errorf("Unexpected num_requests; want: 11; got: %d", stats.NumErrors) } if test.expectedStats != nil { - for _, e := range stats.Endpoints { - if e.Name != "test_service" { - continue - } - if val, ok := e.Data.(map[string]interface{}); !ok || !reflect.DeepEqual(val, test.expectedStats) { - t.Fatalf("Invalid data from stats handler; want: %v; got: %v", test.expectedStats, val) - } + if val, ok := stats.Data.(map[string]interface{}); !ok || !reflect.DeepEqual(val, test.expectedStats) { + t.Fatalf("Invalid data from stats handler; want: %v; got: %v", test.expectedStats, val) } } }) @@ -899,6 +955,7 @@ func TestRequestRespond(t *testing.T) { svc, err := AddService(nc, Config{ Name: "CoolService", + Version: "0.1.0", Description: "Erroring service", Endpoint: Endpoint{ Subject: "svc.fail", From d3ae7e572cac1487bc1b778234ba27e2a1a69478 Mon Sep 17 00:00:00 2001 From: Piotr Piotrowski Date: Wed, 14 Dec 2022 18:57:28 +0100 Subject: [PATCH 07/14] Use getters to retrieve async handlers --- micro/service.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/micro/service.go b/micro/service.go index a1a18a3f3..ad0bb9dc4 100644 --- a/micro/service.go +++ b/micro/service.go @@ -331,8 +331,8 @@ func (e *Endpoint) valid() error { } func (svc *service) setupAsyncCallbacks() { - svc.natsHandlers.closed = svc.conn.Opts.ClosedCB - if svc.conn.Opts.ClosedCB != nil { + svc.natsHandlers.closed = svc.conn.ClosedHandler() + if svc.natsHandlers.closed != nil { svc.conn.SetClosedHandler(func(c *nats.Conn) { svc.Stop() svc.natsHandlers.closed(c) @@ -343,8 +343,8 @@ func (svc *service) setupAsyncCallbacks() { }) } - svc.natsHandlers.asyncErr = svc.conn.Opts.AsyncErrorCB - if svc.conn.Opts.AsyncErrorCB != nil { + svc.natsHandlers.asyncErr = svc.conn.ErrorHandler() + if svc.natsHandlers.asyncErr != nil { svc.conn.SetErrorHandler(func(c *nats.Conn, s *nats.Subscription, err error) { if !svc.matchSubscriptionSubject(s.Subject) { svc.natsHandlers.asyncErr(c, s, err) From 5353379a76aff53e2b57b7be5e8e64227d4884a3 Mon Sep 17 00:00:00 2001 From: Piotr Piotrowski Date: Thu, 15 Dec 2022 12:19:06 +0100 Subject: [PATCH 08/14] Add callbacks dispatcher --- micro/service.go | 95 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 64 insertions(+), 31 deletions(-) diff --git a/micro/service.go b/micro/service.go index ad0bb9dc4..8bc434607 100644 --- a/micro/service.go +++ b/micro/service.go @@ -48,23 +48,6 @@ type ( Stopped() bool } - // service represents a configured NATS service. - // It should be created using [Add] in order to configure the appropriate NATS subscriptions - // for request handler and monitoring. - service struct { - // Config contains a configuration of the service - Config - - m sync.Mutex - id string - reqSub *nats.Subscription - verbSubs map[string]*nats.Subscription - stats *Stats - conn *nats.Conn - natsHandlers handlers - stopped bool - } - // ErrHandler is a function used to configure a custom error handler for a service, ErrHandler func(Service, *NATSError) @@ -83,10 +66,7 @@ type ( } // Stats is the type returned by STATS monitoring endpoint. - // It contains a slice of [EndpointStats], providing stats - // for both the service handler as well as all monitoring subjects. - // EndpointStats are stats for a specific endpoint (either request handler or monitoring enpoints). - // It contains general statisctics for an endpoint, as well as a custom value returned by optional [StatsHandler]. + // It contains stats for a specific endpoint (either request handler or monitoring enpoints). Stats struct { ServiceIdentity NumRequests int `json:"num_requests"` @@ -149,10 +129,33 @@ type ( Description string } + // service represents a configured NATS service. + // It should be created using [Add] in order to configure the appropriate NATS subscriptions + // for request handler and monitoring. + service struct { + // Config contains a configuration of the service + Config + + m sync.Mutex + id string + reqSub *nats.Subscription + verbSubs map[string]*nats.Subscription + stats *Stats + conn *nats.Conn + natsHandlers handlers + stopped bool + + asyncDispatcher asyncCallbacksHandler + } + handlers struct { closed nats.ConnHandler asyncErr nats.ErrHandler } + + asyncCallbacksHandler struct { + cbQueue chan func() + } ) const ( @@ -226,6 +229,9 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { Config: config, conn: nc, id: id, + asyncDispatcher: asyncCallbacksHandler{ + cbQueue: make(chan func(), 100), + }, } svc.verbSubs = make(map[string]*nats.Subscription) svc.stats = &Stats{ @@ -238,6 +244,8 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { svc.setupAsyncCallbacks() + go svc.asyncDispatcher.asyncCBDispatcher() + // Setup internal subscriptions. var err error @@ -245,6 +253,7 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { svc.reqHandler(&Request{Msg: m}) }) if err != nil { + svc.asyncDispatcher.close() return nil, err } @@ -258,7 +267,7 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { response, _ := json.Marshal(svc.Info()) if err := req.Respond(response); err != nil { if err := req.Error("500", fmt.Sprintf("Error handling INFO request: %s", err)); err != nil && config.ErrorHandler != nil { - go config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) + svc.asyncDispatcher.push(func() { config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) }) } } } @@ -267,7 +276,7 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { response, _ := json.Marshal(ping) if err := req.Respond(response); err != nil { if err := req.Error("500", fmt.Sprintf("Error handling PING request: %s", err)); err != nil && config.ErrorHandler != nil { - go config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) + svc.asyncDispatcher.push(func() { config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) }) } } } @@ -276,7 +285,7 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { response, _ := json.Marshal(svc.Stats()) if err := req.Respond(response); err != nil { if err := req.Error("500", fmt.Sprintf("Error handling STATS request: %s", err)); err != nil && config.ErrorHandler != nil { - go config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) + svc.asyncDispatcher.push(func() { config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) }) } } } @@ -285,23 +294,27 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { response, _ := json.Marshal(svc.Schema) if err := req.Respond(response); err != nil { if err := req.Error("500", fmt.Sprintf("Error handling SCHEMA request: %s", err)); err != nil && config.ErrorHandler != nil { - go config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) + svc.asyncDispatcher.push(func() { config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) }) } } } if err := svc.verbHandlers(nc, InfoVerb, infoHandler); err != nil { + svc.asyncDispatcher.close() return nil, err } if err := svc.verbHandlers(nc, PingVerb, pingHandler); err != nil { + svc.asyncDispatcher.close() return nil, err } if err := svc.verbHandlers(nc, StatsVerb, statsHandler); err != nil { + svc.asyncDispatcher.close() return nil, err } if svc.Schema.Request != "" || svc.Schema.Response != "" { if err := svc.verbHandlers(nc, SchemaVerb, schemaHandler); err != nil { + svc.asyncDispatcher.close() return nil, err } } @@ -310,6 +323,26 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { return svc, nil } +// dispatch is responsible for calling any async callbacks +func (ac *asyncCallbacksHandler) asyncCBDispatcher() { + for { + f := <-ac.cbQueue + if f == nil { + return + } + f() + } +} + +// dispatch is responsible for calling any async callbacks +func (ac *asyncCallbacksHandler) push(f func()) { + ac.cbQueue <- f +} + +func (ac *asyncCallbacksHandler) close() { + close(ac.cbQueue) +} + func (s *Config) valid() error { if !serviceNameRegexp.MatchString(s.Name) { return fmt.Errorf("%w: service name: name should not be empty and should consist of alphanumerical charactest, dashes and underscores", ErrConfigValidation) @@ -462,7 +495,8 @@ func (s *service) Stop() error { restoreAsyncHandlers(s.conn, s.natsHandlers) s.stopped = true if s.DoneHandler != nil { - go s.DoneHandler(s) + s.asyncDispatcher.push(func() { s.DoneHandler(s) }) + s.asyncDispatcher.close() } return nil } @@ -485,11 +519,6 @@ func (s *service) Info() Info { } } -// ID returns the service instance's unique ID. -func (s *service) Description() string { - return s.Config.Description -} - // Stats returns statisctics for the service endpoint and all monitoring endpoints. func (s *service) Stats() Stats { s.m.Lock() @@ -552,3 +581,7 @@ func ControlSubject(verb Verb, name, id string) (string, error) { } return fmt.Sprintf("%s.%s.%s.%s", APIPrefix, verbStr, name, id), nil } + +func (e *NATSError) Error() string { + return fmt.Sprintf("%q: %s", e.Subject, e.Description) +} From 7ae28345c956f95f6c40ebd1f9d25bd48a931bc1 Mon Sep 17 00:00:00 2001 From: Piotr Piotrowski Date: Thu, 15 Dec 2022 16:31:59 +0100 Subject: [PATCH 09/14] Add licence --- micro/example_package_test.go | 13 +++++++++++++ micro/example_test.go | 13 +++++++++++++ micro/request.go | 13 +++++++++++++ netchan.go | 2 +- parser.go | 2 +- timer.go | 2 +- timer_test.go | 2 +- ws.go | 2 +- ws_test.go | 2 +- 9 files changed, 45 insertions(+), 6 deletions(-) diff --git a/micro/example_package_test.go b/micro/example_package_test.go index b5b27d8fb..4c45f62b0 100644 --- a/micro/example_package_test.go +++ b/micro/example_package_test.go @@ -1,3 +1,16 @@ +// Copyright 2022 The NATS Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package micro import ( diff --git a/micro/example_test.go b/micro/example_test.go index 366b7eb70..5672bfad1 100644 --- a/micro/example_test.go +++ b/micro/example_test.go @@ -1,3 +1,16 @@ +// Copyright 2022 The NATS Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package micro import ( diff --git a/micro/request.go b/micro/request.go index d391e8af1..be6e72c6d 100644 --- a/micro/request.go +++ b/micro/request.go @@ -1,3 +1,16 @@ +// Copyright 2022 The NATS Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package micro import ( diff --git a/netchan.go b/netchan.go index 3f2a33e60..a1af9e06e 100644 --- a/netchan.go +++ b/netchan.go @@ -1,4 +1,4 @@ -// Copyright 2013-2018 The NATS Authors +// Copyright 2013-2022 The NATS Authors // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at diff --git a/parser.go b/parser.go index 4540f5c1a..a0f0cca2e 100644 --- a/parser.go +++ b/parser.go @@ -1,4 +1,4 @@ -// Copyright 2012-2020 The NATS Authors +// Copyright 2012-2122 The NATS Authors // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at diff --git a/timer.go b/timer.go index 1216762d4..4fb02ecb4 100644 --- a/timer.go +++ b/timer.go @@ -1,4 +1,4 @@ -// Copyright 2017-2018 The NATS Authors +// Copyright 2017-2022 The NATS Authors // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at diff --git a/timer_test.go b/timer_test.go index d561f9675..7b216b598 100644 --- a/timer_test.go +++ b/timer_test.go @@ -1,4 +1,4 @@ -// Copyright 2017-2018 The NATS Authors +// Copyright 2017-2022 The NATS Authors // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at diff --git a/ws.go b/ws.go index d3732e371..c4919a3ae 100644 --- a/ws.go +++ b/ws.go @@ -1,4 +1,4 @@ -// Copyright 2021 The NATS Authors +// Copyright 2021-2022 The NATS Authors // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at diff --git a/ws_test.go b/ws_test.go index eafe7b67f..1de473b60 100644 --- a/ws_test.go +++ b/ws_test.go @@ -1,4 +1,4 @@ -// Copyright 2021 The NATS Authors +// Copyright 2021-2022 The NATS Authors // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at From 65046c9905038801e22537a9d0d4e7fb934aedaf Mon Sep 17 00:00:00 2001 From: Piotr Piotrowski Date: Thu, 15 Dec 2022 17:08:07 +0100 Subject: [PATCH 10/14] Add setting error payload --- micro/example_package_test.go | 2 +- micro/example_test.go | 2 +- micro/request.go | 4 +++- micro/service.go | 8 ++++---- micro/service_test.go | 21 +++++++++++++++++---- 5 files changed, 26 insertions(+), 11 deletions(-) diff --git a/micro/example_package_test.go b/micro/example_package_test.go index 4c45f62b0..4c9cfd689 100644 --- a/micro/example_package_test.go +++ b/micro/example_package_test.go @@ -37,7 +37,7 @@ func Example() { incrementHandler := func(req *Request) { val, err := strconv.Atoi(string(req.Data)) if err != nil { - req.Error("400", "request data should be a number") + req.Error("400", "request data should be a number", nil) return } diff --git a/micro/example_test.go b/micro/example_test.go index 5672bfad1..9018ef18a 100644 --- a/micro/example_test.go +++ b/micro/example_test.go @@ -255,7 +255,7 @@ func ExampleRequest_Error() { handler := func(req *Request) { // respond with an error // Error sets Nats-Service-Error and Nats-Service-Error-Code headers in the response - if err := req.Error("400", "bad request"); err != nil { + if err := req.Error("400", "bad request", []byte(`{"error": "value should be a number"}`)); err != nil { log.Fatal(err) } } diff --git a/micro/request.go b/micro/request.go index be6e72c6d..837a51508 100644 --- a/micro/request.go +++ b/micro/request.go @@ -56,7 +56,8 @@ func (r *Request) RespondJSON(response interface{}) error { // Error prepares and publishes error response from a handler. // A response error should be set containing an error code and description. -func (r *Request) Error(code, description string) error { +// Optionally, data can be set as response payload. +func (r *Request) Error(code, description string, data []byte) error { if code == "" { return fmt.Errorf("%w: error code", ErrArgRequired) } @@ -69,6 +70,7 @@ func (r *Request) Error(code, description string) error { ErrorCodeHeader: []string{code}, }, } + response.Data = data r.errResponse = true return r.RespondMsg(response) } diff --git a/micro/service.go b/micro/service.go index 8bc434607..dbc670f69 100644 --- a/micro/service.go +++ b/micro/service.go @@ -266,7 +266,7 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { infoHandler := func(req *Request) { response, _ := json.Marshal(svc.Info()) if err := req.Respond(response); err != nil { - if err := req.Error("500", fmt.Sprintf("Error handling INFO request: %s", err)); err != nil && config.ErrorHandler != nil { + if err := req.Error("500", fmt.Sprintf("Error handling INFO request: %s", err), nil); err != nil && config.ErrorHandler != nil { svc.asyncDispatcher.push(func() { config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) }) } } @@ -275,7 +275,7 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { pingHandler := func(req *Request) { response, _ := json.Marshal(ping) if err := req.Respond(response); err != nil { - if err := req.Error("500", fmt.Sprintf("Error handling PING request: %s", err)); err != nil && config.ErrorHandler != nil { + if err := req.Error("500", fmt.Sprintf("Error handling PING request: %s", err), nil); err != nil && config.ErrorHandler != nil { svc.asyncDispatcher.push(func() { config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) }) } } @@ -284,7 +284,7 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { statsHandler := func(req *Request) { response, _ := json.Marshal(svc.Stats()) if err := req.Respond(response); err != nil { - if err := req.Error("500", fmt.Sprintf("Error handling STATS request: %s", err)); err != nil && config.ErrorHandler != nil { + if err := req.Error("500", fmt.Sprintf("Error handling STATS request: %s", err), nil); err != nil && config.ErrorHandler != nil { svc.asyncDispatcher.push(func() { config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) }) } } @@ -293,7 +293,7 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { schemaHandler := func(req *Request) { response, _ := json.Marshal(svc.Schema) if err := req.Respond(response); err != nil { - if err := req.Error("500", fmt.Sprintf("Error handling SCHEMA request: %s", err)); err != nil && config.ErrorHandler != nil { + if err := req.Error("500", fmt.Sprintf("Error handling SCHEMA request: %s", err), nil); err != nil && config.ErrorHandler != nil { svc.asyncDispatcher.push(func() { config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) }) } } diff --git a/micro/service_test.go b/micro/service_test.go index 27c647bb8..91d2cff27 100644 --- a/micro/service_test.go +++ b/micro/service_test.go @@ -41,7 +41,7 @@ func TestServiceBasics(t *testing.T) { // Stub service. doAdd := func(req *Request) { if rand.Intn(10) == 0 { - if err := req.Error("500", "Unexpected error!"); err != nil { + if err := req.Error("500", "Unexpected error!", nil); err != nil { t.Fatalf("Unexpected error when sending error response: %v", err) } return @@ -50,7 +50,7 @@ func TestServiceBasics(t *testing.T) { // Random delay between 5-10ms time.Sleep(5*time.Millisecond + time.Duration(rand.Intn(5))*time.Millisecond) if err := req.Respond([]byte("42")); err != nil { - if err := req.Error("500", "Unexpected error!"); err != nil { + if err := req.Error("500", "Unexpected error!", nil); err != nil { t.Fatalf("Unexpected error when sending error response: %v", err) } return @@ -705,7 +705,7 @@ func TestMonitoringHandlers(t *testing.T) { func TestServiceStats(t *testing.T) { handler := func(r *Request) { if bytes.Equal(r.Data, []byte("err")) { - r.Error("400", "bad request") + r.Error("400", "bad request", nil) } r.Respond([]byte("ok")) } @@ -850,6 +850,7 @@ func TestRequestRespond(t *testing.T) { respondData interface{} errDescription string errCode string + errData []byte expectedMessage string expectedCode string expectedResponse []byte @@ -879,6 +880,14 @@ func TestRequestRespond(t *testing.T) { name: "generic error", errDescription: "oops", errCode: "500", + errData: []byte("error!"), + expectedMessage: "oops", + expectedCode: "500", + }, + { + name: "error without response payload", + errDescription: "oops", + errCode: "500", expectedMessage: "oops", expectedCode: "500", }, @@ -909,6 +918,7 @@ func TestRequestRespond(t *testing.T) { respError := test.withRespondError errCode := test.errCode errDesc := test.errDescription + errData := test.errData // Stub service. handler := func(req *Request) { if errors.Is(test.withRespondError, ErrRespond) { @@ -941,7 +951,7 @@ func TestRequestRespond(t *testing.T) { return } - err := req.Error(errCode, errDesc) + err := req.Error(errCode, errDesc, errData) if respError != nil { if !errors.Is(err, respError) { t.Fatalf("Expected error: %v; got: %v", respError, err) @@ -984,6 +994,9 @@ func TestRequestRespond(t *testing.T) { if code != test.expectedCode { t.Fatalf("Invalid response code; want: %q; got: %q", test.expectedCode, code) } + if !bytes.Equal(resp.Data, test.errData) { + t.Fatalf("Invalid response payload; want: %q; got: %q", string(test.errData), resp.Data) + } return } From 1f3fcdc50a8bde7df8eefe6ec5d0cbd5d7ead9f9 Mon Sep 17 00:00:00 2001 From: Piotr Piotrowski Date: Thu, 15 Dec 2022 18:04:27 +0100 Subject: [PATCH 11/14] Return error from handler to be used in stats --- micro/example_package_test.go | 5 ++-- micro/example_test.go | 20 +++++++++------- micro/request.go | 9 ++++--- micro/service.go | 26 ++++++++++++-------- micro/service_test.go | 45 +++++++++++++++++++++++------------ 5 files changed, 66 insertions(+), 39 deletions(-) diff --git a/micro/example_package_test.go b/micro/example_package_test.go index 4c9cfd689..113e8825d 100644 --- a/micro/example_package_test.go +++ b/micro/example_package_test.go @@ -34,15 +34,16 @@ func Example() { // Service handler is a function which takes Service.Request as argument. // req.Respond or req.Error should be used to respond to the request. - incrementHandler := func(req *Request) { + incrementHandler := func(req *Request) error { val, err := strconv.Atoi(string(req.Data)) if err != nil { req.Error("400", "request data should be a number", nil) - return + return nil } responseData := val + 1 req.Respond([]byte(strconv.Itoa(responseData))) + return nil } config := Config{ diff --git a/micro/example_test.go b/micro/example_test.go index 9018ef18a..b8ca1b0b9 100644 --- a/micro/example_test.go +++ b/micro/example_test.go @@ -27,8 +27,9 @@ func ExampleAddService() { } defer nc.Close() - echoHandler := func(req *Request) { + echoHandler := func(req *Request) error { req.Respond(req.Data) + return nil } config := Config{ @@ -71,7 +72,7 @@ func ExampleService_Info() { Name: "EchoService", Endpoint: Endpoint{ Subject: "echo", - Handler: func(*Request) {}, + Handler: func(*Request) error { return nil }, }, } @@ -99,7 +100,7 @@ func ExampleService_Stats() { Version: "0.1.0", Endpoint: Endpoint{ Subject: "echo", - Handler: func(*Request) {}, + Handler: func(*Request) error { return nil }, }, } @@ -109,7 +110,7 @@ func ExampleService_Stats() { stats := srv.Stats() fmt.Println(stats.AverageProcessingTime) - fmt.Println(stats.TotalProcessingTime) + fmt.Println(stats.ProcessingTime) } @@ -125,7 +126,7 @@ func ExampleService_Stop() { Version: "0.1.0", Endpoint: Endpoint{ Subject: "echo", - Handler: func(*Request) {}, + Handler: func(*Request) error { return nil }, }, } @@ -156,7 +157,7 @@ func ExampleService_Stopped() { Version: "0.1.0", Endpoint: Endpoint{ Subject: "echo", - Handler: func(*Request) {}, + Handler: func(*Request) error { return nil }, }, } @@ -185,7 +186,7 @@ func ExampleService_Reset() { Version: "0.1.0", Endpoint: Endpoint{ Subject: "echo", - Handler: func(*Request) {}, + Handler: func(*Request) error { return nil }, }, } @@ -252,12 +253,13 @@ func ExampleRequest_RespondJSON() { } func ExampleRequest_Error() { - handler := func(req *Request) { + handler := func(req *Request) error { // respond with an error // Error sets Nats-Service-Error and Nats-Service-Error-Code headers in the response if err := req.Error("400", "bad request", []byte(`{"error": "value should be a number"}`)); err != nil { - log.Fatal(err) + return err } + return nil } fmt.Printf("%T", handler) diff --git a/micro/request.go b/micro/request.go index 837a51508..f7cc4a580 100644 --- a/micro/request.go +++ b/micro/request.go @@ -24,11 +24,15 @@ import ( type ( Request struct { *nats.Msg - errResponse bool } // RequestHandler is a function used as a Handler for a service. - RequestHandler func(*Request) + // It takes a request, which contains the data (payload and headers) of the request, + // as well as exposes methods to respond to the request. + // + // RequestHandler returns an error - if returned, the request will be accounted form in stats (in num_requests), + // and last_error will be set with the value. + RequestHandler func(*Request) error ) var ( @@ -71,6 +75,5 @@ func (r *Request) Error(code, description string, data []byte) error { }, } response.Data = data - r.errResponse = true return r.RespondMsg(response) } diff --git a/micro/service.go b/micro/service.go index dbc670f69..a6dabb1f4 100644 --- a/micro/service.go +++ b/micro/service.go @@ -71,7 +71,8 @@ type ( ServiceIdentity NumRequests int `json:"num_requests"` NumErrors int `json:"num_errors"` - TotalProcessingTime time.Duration `json:"total_processing_time"` + LastError error `json:"last_error"` + ProcessingTime time.Duration `json:"processing_time"` AverageProcessingTime time.Duration `json:"average_processing_time"` Started string `json:"started"` Data interface{} `json:"data,omitempty"` @@ -263,40 +264,44 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { Version: config.Version, } - infoHandler := func(req *Request) { + infoHandler := func(req *Request) error { response, _ := json.Marshal(svc.Info()) if err := req.Respond(response); err != nil { if err := req.Error("500", fmt.Sprintf("Error handling INFO request: %s", err), nil); err != nil && config.ErrorHandler != nil { svc.asyncDispatcher.push(func() { config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) }) } } + return nil } - pingHandler := func(req *Request) { + pingHandler := func(req *Request) error { response, _ := json.Marshal(ping) if err := req.Respond(response); err != nil { if err := req.Error("500", fmt.Sprintf("Error handling PING request: %s", err), nil); err != nil && config.ErrorHandler != nil { svc.asyncDispatcher.push(func() { config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) }) } } + return nil } - statsHandler := func(req *Request) { + statsHandler := func(req *Request) error { response, _ := json.Marshal(svc.Stats()) if err := req.Respond(response); err != nil { if err := req.Error("500", fmt.Sprintf("Error handling STATS request: %s", err), nil); err != nil && config.ErrorHandler != nil { svc.asyncDispatcher.push(func() { config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) }) } } + return nil } - schemaHandler := func(req *Request) { + schemaHandler := func(req *Request) error { response, _ := json.Marshal(svc.Schema) if err := req.Respond(response); err != nil { if err := req.Error("500", fmt.Sprintf("Error handling SCHEMA request: %s", err), nil); err != nil && config.ErrorHandler != nil { svc.asyncDispatcher.push(func() { config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) }) } } + return nil } if err := svc.verbHandlers(nc, InfoVerb, infoHandler); err != nil { @@ -456,15 +461,16 @@ func (s *service) addInternalHandler(nc *nats.Conn, verb Verb, kind, id, name st // reqHandler itself func (s *service) reqHandler(req *Request) { start := time.Now() - s.Endpoint.Handler(req) + err := s.Endpoint.Handler(req) s.m.Lock() s.stats.NumRequests++ - s.stats.TotalProcessingTime += time.Since(start) - avgProcessingTime := s.stats.TotalProcessingTime.Nanoseconds() / int64(s.stats.NumRequests) + s.stats.ProcessingTime += time.Since(start) + avgProcessingTime := s.stats.ProcessingTime.Nanoseconds() / int64(s.stats.NumRequests) s.stats.AverageProcessingTime = time.Duration(avgProcessingTime) - if req.errResponse { + if err != nil { s.stats.NumErrors++ + s.stats.LastError = err } s.m.Unlock() } @@ -535,7 +541,7 @@ func (s *service) Stats() Stats { }, NumRequests: s.stats.NumRequests, NumErrors: s.stats.NumErrors, - TotalProcessingTime: s.stats.TotalProcessingTime, + ProcessingTime: s.stats.ProcessingTime, AverageProcessingTime: s.stats.AverageProcessingTime, Started: s.stats.Started, Data: s.stats.Data, diff --git a/micro/service_test.go b/micro/service_test.go index 91d2cff27..91c81d727 100644 --- a/micro/service_test.go +++ b/micro/service_test.go @@ -39,12 +39,14 @@ func TestServiceBasics(t *testing.T) { defer nc.Close() // Stub service. - doAdd := func(req *Request) { + doAdd := func(req *Request) error { if rand.Intn(10) == 0 { - if err := req.Error("500", "Unexpected error!", nil); err != nil { + if err := req.Error("400", "client error!", nil); err != nil { t.Fatalf("Unexpected error when sending error response: %v", err) } - return + + // for client-side errors, return nil to avoid tracking the errors in stats + return nil } // Happy Path. // Random delay between 5-10ms @@ -53,8 +55,9 @@ func TestServiceBasics(t *testing.T) { if err := req.Error("500", "Unexpected error!", nil); err != nil { t.Fatalf("Unexpected error when sending error response: %v", err) } - return + return err } + return nil } var svcs []Service @@ -191,7 +194,7 @@ func TestServiceBasics(t *testing.T) { } func TestAddService(t *testing.T) { - testHandler := func(*Request) {} + testHandler := func(*Request) error { return nil } errNats := make(chan struct{}) errService := make(chan struct{}) closedNats := make(chan struct{}) @@ -530,7 +533,7 @@ func TestMonitoringHandlers(t *testing.T) { Version: "0.1.0", Endpoint: Endpoint{ Subject: "test.sub", - Handler: func(*Request) {}, + Handler: func(*Request) error { return nil }, }, Schema: Schema{ Request: "some_schema", @@ -703,11 +706,19 @@ func TestMonitoringHandlers(t *testing.T) { } func TestServiceStats(t *testing.T) { - handler := func(r *Request) { + handler := func(r *Request) error { if bytes.Equal(r.Data, []byte("err")) { + r.Error("500", "oops", nil) + return fmt.Errorf("oops") + } + + // client errors (validation etc.) should not be accounted for in stats + if bytes.Equal(r.Data, []byte("client_err")) { r.Error("400", "bad request", nil) + return nil } r.Respond([]byte("ok")) + return nil } tests := []struct { name string @@ -803,6 +814,9 @@ func TestServiceStats(t *testing.T) { t.Fatalf("Unexpected error: %v", err) } } + if _, err := nc.Request(srv.Info().Subject, []byte("client_err"), time.Second); err != nil { + t.Fatalf("Unexpected error: %v", err) + } if _, err := nc.Request(srv.Info().Subject, []byte("err"), time.Second); err != nil { t.Fatalf("Unexpected error: %v", err) } @@ -824,11 +838,11 @@ func TestServiceStats(t *testing.T) { if stats.ID != info.ID { t.Errorf("Unexpected service name; want: %s; got: %s", info.ID, stats.ID) } - if stats.NumRequests != 11 { - t.Errorf("Unexpected num_requests; want: 11; got: %d", stats.NumRequests) + if stats.NumRequests != 12 { + t.Errorf("Unexpected num_requests; want: 12; got: %d", stats.NumRequests) } if stats.NumErrors != 1 { - t.Errorf("Unexpected num_requests; want: 11; got: %d", stats.NumErrors) + t.Errorf("Unexpected num_requests; want: 1; got: %d", stats.NumErrors) } if test.expectedStats != nil { if val, ok := stats.Data.(map[string]interface{}); !ok || !reflect.DeepEqual(val, test.expectedStats) { @@ -920,7 +934,7 @@ func TestRequestRespond(t *testing.T) { errDesc := test.errDescription errData := test.errData // Stub service. - handler := func(req *Request) { + handler := func(req *Request) error { if errors.Is(test.withRespondError, ErrRespond) { nc.Close() } @@ -931,7 +945,7 @@ func TestRequestRespond(t *testing.T) { if !errors.Is(err, respError) { t.Fatalf("Expected error: %v; got: %v", respError, err) } - return + return nil } if err != nil { t.Fatalf("Unexpected error when sending response: %v", err) @@ -942,13 +956,13 @@ func TestRequestRespond(t *testing.T) { if !errors.Is(err, respError) { t.Fatalf("Expected error: %v; got: %v", respError, err) } - return + return nil } if err != nil { t.Fatalf("Unexpected error when sending response: %v", err) } } - return + return nil } err := req.Error(errCode, errDesc, errData) @@ -956,11 +970,12 @@ func TestRequestRespond(t *testing.T) { if !errors.Is(err, respError) { t.Fatalf("Expected error: %v; got: %v", respError, err) } - return + return nil } if err != nil { t.Fatalf("Unexpected error when sending response: %v", err) } + return nil } svc, err := AddService(nc, Config{ From ade30c47067bc892f723d920f13df157c4e94dd9 Mon Sep 17 00:00:00 2001 From: Piotr Piotrowski Date: Fri, 16 Dec 2022 16:58:42 +0100 Subject: [PATCH 12/14] Fix schema handler --- micro/service.go | 31 +++++++++++++++---------------- micro/service_test.go | 33 +++++++++++++++++++++++++++------ 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/micro/service.go b/micro/service.go index a6dabb1f4..6c52bade1 100644 --- a/micro/service.go +++ b/micro/service.go @@ -234,13 +234,14 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { cbQueue: make(chan func(), 100), }, } + svcIdentity := ServiceIdentity{ + Name: config.Name, + ID: id, + Version: config.Version, + } svc.verbSubs = make(map[string]*nats.Subscription) svc.stats = &Stats{ - ServiceIdentity: ServiceIdentity{ - Name: config.Name, - ID: id, - Version: config.Version, - }, + ServiceIdentity: svcIdentity, } svc.setupAsyncCallbacks() @@ -258,11 +259,7 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { return nil, err } - ping := &Ping{ - Name: config.Name, - ID: id, - Version: config.Version, - } + ping := Ping(svcIdentity) infoHandler := func(req *Request) error { response, _ := json.Marshal(svc.Info()) @@ -294,8 +291,12 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { return nil } + schema := SchemaResp{ + ServiceIdentity: svcIdentity, + Schema: config.Schema, + } schemaHandler := func(req *Request) error { - response, _ := json.Marshal(svc.Schema) + response, _ := json.Marshal(schema) if err := req.Respond(response); err != nil { if err := req.Error("500", fmt.Sprintf("Error handling SCHEMA request: %s", err), nil); err != nil && config.ErrorHandler != nil { svc.asyncDispatcher.push(func() { config.ErrorHandler(svc, &NATSError{req.Subject, err.Error()}) }) @@ -317,11 +318,9 @@ func AddService(nc *nats.Conn, config Config) (Service, error) { return nil, err } - if svc.Schema.Request != "" || svc.Schema.Response != "" { - if err := svc.verbHandlers(nc, SchemaVerb, schemaHandler); err != nil { - svc.asyncDispatcher.close() - return nil, err - } + if err := svc.verbHandlers(nc, SchemaVerb, schemaHandler); err != nil { + svc.asyncDispatcher.close() + return nil, err } svc.stats.Started = time.Now().Format(time.RFC3339) diff --git a/micro/service_test.go b/micro/service_test.go index 91c81d727..1ae6b7350 100644 --- a/micro/service_test.go +++ b/micro/service_test.go @@ -625,22 +625,43 @@ func TestMonitoringHandlers(t *testing.T) { { name: "SCHEMA all", subject: "$SRV.SCHEMA", - expectedResponse: Schema{ - Request: "some_schema", + expectedResponse: SchemaResp{ + ServiceIdentity: ServiceIdentity{ + Name: "test_service", + Version: "0.1.0", + ID: info.ID, + }, + Schema: Schema{ + Request: "some_schema", + }, }, }, { name: "SCHEMA name", subject: "$SRV.SCHEMA.TEST_SERVICE", - expectedResponse: Schema{ - Request: "some_schema", + expectedResponse: SchemaResp{ + ServiceIdentity: ServiceIdentity{ + Name: "test_service", + Version: "0.1.0", + ID: info.ID, + }, + Schema: Schema{ + Request: "some_schema", + }, }, }, { name: "SCHEMA ID", subject: fmt.Sprintf("$SRV.SCHEMA.TEST_SERVICE.%s", info.ID), - expectedResponse: Schema{ - Request: "some_schema", + expectedResponse: SchemaResp{ + ServiceIdentity: ServiceIdentity{ + Name: "test_service", + Version: "0.1.0", + ID: info.ID, + }, + Schema: Schema{ + Request: "some_schema", + }, }, }, { From d1e8c980675620d8926603ffd536d17474250555 Mon Sep 17 00:00:00 2001 From: Piotr Piotrowski Date: Tue, 20 Dec 2022 09:27:11 +0100 Subject: [PATCH 13/14] Fix comment on AddService Co-authored-by: Tomasz Pietrek --- micro/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/micro/service.go b/micro/service.go index 6c52bade1..75a63b550 100644 --- a/micro/service.go +++ b/micro/service.go @@ -217,7 +217,7 @@ func (s Verb) String() string { // AddService adds a microservice. // It will enable internal common services (PING, STATS, INFO and SCHEMA) as well as // the actual service handler on the subject provided in config.Endpoint -// A service name and Endpoint configuration are required to add a service. +// A service name, version and Endpoint configuration are required to add a service. // AddService returns a [Service] interface, allowing service menagement. // Each service is assigned a unique ID. func AddService(nc *nats.Conn, config Config) (Service, error) { From 63932a2b2b1081ecf24bb846f1c7cf8975b04f6f Mon Sep 17 00:00:00 2001 From: Piotr Piotrowski Date: Tue, 20 Dec 2022 12:25:23 +0100 Subject: [PATCH 14/14] Switch to using json.RawMessage for stats data --- micro/example_test.go | 3 ++- micro/service.go | 20 ++++++++++---------- micro/service_test.go | 10 +++++++--- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/micro/example_test.go b/micro/example_test.go index b8ca1b0b9..3e50497ac 100644 --- a/micro/example_test.go +++ b/micro/example_test.go @@ -16,6 +16,7 @@ package micro import ( "fmt" "log" + "reflect" "github.com/nats-io/nats.go" ) @@ -198,7 +199,7 @@ func ExampleService_Reset() { empty := Stats{ ServiceIdentity: srv.Info().ServiceIdentity, } - if srv.Stats() != empty { + if !reflect.DeepEqual(srv.Stats(), empty) { log.Fatal("Expected endpoint stats to be empty") } } diff --git a/micro/service.go b/micro/service.go index 75a63b550..6943f170b 100644 --- a/micro/service.go +++ b/micro/service.go @@ -69,13 +69,13 @@ type ( // It contains stats for a specific endpoint (either request handler or monitoring enpoints). Stats struct { ServiceIdentity - NumRequests int `json:"num_requests"` - NumErrors int `json:"num_errors"` - LastError error `json:"last_error"` - ProcessingTime time.Duration `json:"processing_time"` - AverageProcessingTime time.Duration `json:"average_processing_time"` - Started string `json:"started"` - Data interface{} `json:"data,omitempty"` + NumRequests int `json:"num_requests"` + NumErrors int `json:"num_errors"` + LastError string `json:"last_error"` + ProcessingTime time.Duration `json:"processing_time"` + AverageProcessingTime time.Duration `json:"average_processing_time"` + Started string `json:"started"` + Data json.RawMessage `json:"data,omitempty"` } // Ping is the response type for PING monitoring endpoint. @@ -91,7 +91,7 @@ type ( // SchemaResp is the response value for SCHEMA requests. SchemaResp struct { ServiceIdentity - Schema + Schema Schema `json:"schema"` } // Schema can be used to configure a schema for a service. @@ -469,7 +469,7 @@ func (s *service) reqHandler(req *Request) { if err != nil { s.stats.NumErrors++ - s.stats.LastError = err + s.stats.LastError = err.Error() } s.m.Unlock() } @@ -529,7 +529,7 @@ func (s *service) Stats() Stats { s.m.Lock() defer s.m.Unlock() if s.StatsHandler != nil { - s.stats.Data = s.StatsHandler(s.Endpoint) + s.stats.Data, _ = json.Marshal(s.StatsHandler(s.Endpoint)) } info := s.Info() return Stats{ diff --git a/micro/service_test.go b/micro/service_test.go index 1ae6b7350..4cf4ccd0c 100644 --- a/micro/service_test.go +++ b/micro/service_test.go @@ -187,7 +187,7 @@ func TestServiceBasics(t *testing.T) { ServiceIdentity: svcs[0].Info().ServiceIdentity, } - if svcs[0].Stats() != emptyStats { + if !reflect.DeepEqual(svcs[0].Stats(), emptyStats) { t.Fatalf("Expected empty stats after reset; got: %+v", svcs[0].Stats()) } @@ -866,8 +866,12 @@ func TestServiceStats(t *testing.T) { t.Errorf("Unexpected num_requests; want: 1; got: %d", stats.NumErrors) } if test.expectedStats != nil { - if val, ok := stats.Data.(map[string]interface{}); !ok || !reflect.DeepEqual(val, test.expectedStats) { - t.Fatalf("Invalid data from stats handler; want: %v; got: %v", test.expectedStats, val) + var data map[string]interface{} + if err := json.Unmarshal(stats.Data, &data); err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if !reflect.DeepEqual(data, test.expectedStats) { + t.Fatalf("Invalid data from stats handler; want: %v; got: %v", test.expectedStats, data) } } })