diff --git a/composition/composition_handler.go b/composition/composition_handler.go index afefb14..822ed7b 100644 --- a/composition/composition_handler.go +++ b/composition/composition_handler.go @@ -64,7 +64,10 @@ func (agg *CompositionHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) mergeContext.AddContent(res) } else if res.Def.Required { - logging.Application(r.Header).WithField("fetchResult", res).Errorf("error loading content from: %v", res.Def.URL) + // 404 Error already become logged in logger.go + if res.Content.HttpStatusCode() != 404 { + logging.Application(r.Header).WithField("fetchResult", res).Errorf("error loading content from: %v", res.Def.URL) + } res.Def.ErrHandler.Handle(res.Err, res.Content.HttpStatusCode(), w, r) return } else { diff --git a/composition/content_fetcher.go b/composition/content_fetcher.go index 025ecf0..49dd9e5 100644 --- a/composition/content_fetcher.go +++ b/composition/content_fetcher.go @@ -100,10 +100,14 @@ func (fetcher *ContentFetcher) AddFetchJob(d *FetchDefinition) { } } } else { - logging.Logger.WithError(fetchResult.Err). - WithField("fetchDefinition", d). - WithField("correlation_id", logging.GetCorrelationId(definitionCopy.Header)). - Errorf("failed fetching %v", d.URL) + // 404 Error already become logged in logger.go + if fetchResult.Content.HttpStatusCode() != 404 { + logging.Logger.WithError(fetchResult.Err). + WithField("fetchDefinition", d). + WithField("correlation_id", logging.GetCorrelationId(definitionCopy.Header)). + Errorf("failed fetching %v", d.URL) + } + } }() } diff --git a/composition/content_merge.go b/composition/content_merge.go index 0680778..2942c6d 100644 --- a/composition/content_merge.go +++ b/composition/content_merge.go @@ -5,6 +5,7 @@ import ( "errors" "io" "strings" + // "fmt" ) const ( @@ -42,7 +43,8 @@ func (cntx *ContentMerge) GetHtml() ([]byte, error) { executeFragment = func(fragmentName string) error { f, exist := cntx.Body[fragmentName] if !exist { - return errors.New("Fragment does not exist: " + fragmentName) + missingFragmentString := generateMissingFragmentString(cntx.Body, fragmentName) + return errors.New(missingFragmentString) } return f.Execute(w, cntx.MetaJSON, executeFragment) } @@ -120,3 +122,21 @@ func urlToFragmentName(url string) string { url = strings.Replace(url, `]§`, `\]\§`, -1) return url } + +// Generates String for the missing Fragment error message. It adds all existing fragments from the body +func generateMissingFragmentString(body map[string]Fragment, fragmentName string) string { + text := "Fragment does not exist: " + fragmentName + ". Existing fragments: " + index := 0 + for k, _ := range body { + + if k != "" { + if index == 0 { + text += k + } else { + text += ", " + k + } + index++ + } + } + return text +} diff --git a/composition/content_merge_test.go b/composition/content_merge_test.go index a5ed545..34da188 100644 --- a/composition/content_merge_test.go +++ b/composition/content_merge_test.go @@ -62,6 +62,22 @@ func Test_ContentMerge_PositiveCase(t *testing.T) { type MockPage1BodyFragment struct { } +func Test_GenerateMissingFragmentString(t *testing.T) { + body := map[string]Fragment{ + "footer": nil, + "header": nil, + "": nil, + } + fragmentName := "body" + fragmentString := generateMissingFragmentString(body, fragmentName) + + a := assert.New(t) + a.Contains(fragmentString, "Fragment does not exist: body.") + a.Contains(fragmentString, "footer") + a.Contains(fragmentString, "header") + +} + func (f MockPage1BodyFragment) Execute(w io.Writer, data map[string]interface{}, executeNestedFragment func(nestedFragmentName string) error) error { w.Write([]byte("\n")) if err := executeNestedFragment("page2-a"); err != nil { @@ -86,7 +102,7 @@ func Test_ContentMerge_MainFragmentDoesNotExist(t *testing.T) { cm := NewContentMerge(nil) _, err := cm.GetHtml() a.Error(err) - a.Equal("Fragment does not exist: ", err.Error()) + a.Equal("Fragment does not exist: . Existing fragments: ", err.Error()) } type closedWriterMock struct { diff --git a/composition/http_content_loader.go b/composition/http_content_loader.go index 2fddb9c..347514c 100644 --- a/composition/http_content_loader.go +++ b/composition/http_content_loader.go @@ -1,12 +1,12 @@ package composition import ( - "fmt" "io/ioutil" "net/http" "strings" "time" "errors" + "fmt" "github.com/tarent/lib-compose/logging" "github.com/tarent/lib-servicediscovery/servicediscovery" "net/url" diff --git a/logging/logger.go b/logging/logger.go index 98e4fcf..619304f 100644 --- a/logging/logger.go +++ b/logging/logger.go @@ -80,6 +80,7 @@ func access(r *http.Request, start time.Time, statusCode int, err error) *logrus "method": r.Method, "proto": r.Proto, "duration": time.Since(start).Nanoseconds() / 1000000, + "User_Agent": r.Header.Get("User-Agent"), } if statusCode != 0 { diff --git a/logging/logger_test.go b/logging/logger_test.go index e208407..5087938 100644 --- a/logging/logger_test.go +++ b/logging/logger_test.go @@ -28,6 +28,7 @@ type logReccord struct { Error string `json:"error"` Message string `json:"message"` Level string `json:"level"` + UserAgent string `json:"User_Agent"` } func Test_Logger_Set(t *testing.T) { @@ -122,11 +123,14 @@ func Test_Logger_Access(t *testing.T) { AccessLogCookiesBlacklist = []string{"ignore", "user_id"} UserCorrelationCookie = "user_id" + // Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.84 Safari/537.36 + // and a request r, _ := http.NewRequest("GET", "http://www.example.org/foo?q=bar", nil) r.Header = http.Header{ CorrelationIdHeader: {"correlation-123"}, "Cookie": {"user_id=user-id-xyz; ignore=me; foo=bar;"}, + "User-Agent": {"Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.84 Safari/537.36"}, } r.RemoteAddr = "127.0.0.1" @@ -153,6 +157,7 @@ func Test_Logger_Access(t *testing.T) { a.Equal("access", data.Type) a.Equal("/foo?q=bar", data.URL) a.Equal("user-id-xyz", data.UserCorrelationId) + a.Equal("Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.84 Safari/537.36", data.UserAgent) } func Test_Logger_Access_ErrorCases(t *testing.T) {