Merge ee1aad4 into 9adbe65

composition/composition_handler.go

@@ -51,7 +51,7 @@ func (agg *CompositionHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 
 		} else if res.Def.Required {
 			log.WithField("fetchResult", res).Errorf("error loading content from: %v", res.Def.URL)
-			res.Def.ErrHandler.Handle(res.Err, w, r)
+			res.Def.ErrHandler.Handle(res.Err, res.HttpStatus, w, r)
 			return
 		} else {
 			log.WithField("fetchResult", res).Warnf("optional content not loaded: %v", res.Def.URL)

composition/composition_handler_test.go

@@ -173,9 +173,10 @@ func Test_CompositionHandler_ErrorInFetching(t *testing.T) {
 	contentFetcherFactory := func(r *http.Request) FetchResultSupplier {
 		return MockFetchResultSupplier{
 			&FetchResult{
-				Def:     NewFetchDefinition("/foo"),
-				Content: nil,
-				Err:     errors.New(errorString),
+				Def:        NewFetchDefinition("/foo"),
+				Content:    nil,
+				Err:        errors.New(errorString),
+				HttpStatus: 502,
 			},
 		}
 	}
@@ -185,7 +186,7 @@ func Test_CompositionHandler_ErrorInFetching(t *testing.T) {
 	r, _ := http.NewRequest("GET", "http://example.com", nil)
 	aggregator.ServeHTTP(resp, r)
 
-	a.Equal("Bad Gateway: "+errorString+"\n", string(resp.Body.Bytes()))
+	a.Equal("Error: "+errorString+"\n", string(resp.Body.Bytes()))
 	a.Equal(502, resp.Code)
 }
 

composition/content_fetcher.go

@@ -5,6 +5,7 @@ import (
 	"sync"
 
 	"github.com/tarent/lib-compose/logging"
+	"net/http"
 	"strings"
 )
 
@@ -15,10 +16,11 @@ func (def *FetchDefinition) IsFetchable() bool {
 }
 
 type FetchResult struct {
-	Def     *FetchDefinition
-	Err     error
-	Content Content
-	Hash    string // the hash of the FetchDefinition
+	Def        *FetchDefinition
+	Err        error
+	Content    Content
+	HttpStatus int
+	Hash       string // the hash of the FetchDefinition
 }
 
 // ContentFetcher is a type, which can fetch a set of Content pages in parallel.
@@ -75,7 +77,7 @@ func (fetcher *ContentFetcher) AddFetchJob(d *FetchDefinition) {
 
 	fetcher.activeJobs.Add(1)
 
-	fetchResult := &FetchResult{Def: d, Hash: hash, Err: errors.New("not fetched")}
+	fetchResult := &FetchResult{Def: d, Hash: hash, Err: errors.New("not fetched"), HttpStatus: http.StatusBadGateway}
 	fetcher.r.results = append(fetcher.r.results, fetchResult)
 
 	go func() {
@@ -94,7 +96,7 @@ func (fetcher *ContentFetcher) AddFetchJob(d *FetchDefinition) {
 		// want to override the original URL with expanded values
 		definitionCopy := *d
 		definitionCopy.URL = url
-		fetchResult.Content, fetchResult.Err = fetcher.fetch(&definitionCopy)
+		fetchResult.Content, fetchResult.Err, fetchResult.HttpStatus = fetcher.fetch(&definitionCopy)
 
 		if fetchResult.Err == nil {
 			fetcher.addMeta(fetchResult.Content.Meta())
@@ -112,7 +114,7 @@ func (fetcher *ContentFetcher) AddFetchJob(d *FetchDefinition) {
 	}()
 }
 
-func (fetcher *ContentFetcher) fetch(fd *FetchDefinition) (Content, error) {
+func (fetcher *ContentFetcher) fetch(fd *FetchDefinition) (Content, error, int) {
 	if strings.HasPrefix(fd.URL, FileURLPrefix) {
 		return fetcher.fileContentLoader.Load(fd)
 	}

composition/content_fetcher_test.go

@@ -68,9 +68,9 @@ func Test_ContentFetcher_FetchingWithDependency(t *testing.T) {
 	a := assert.New(t)
 
 	loader := NewMockContentLoader(ctrl)
-	barFd := getFetchDefinitionMock(ctrl, loader, "/bar", nil, time.Millisecond*2, map[string]interface{}{"foo": "bar"})
-	fooFd := getFetchDefinitionMock(ctrl, loader, "/foo", []*FetchDefinition{barFd}, time.Millisecond*2, map[string]interface{}{"bli": "bla"})
-	bazzFd := getFetchDefinitionMock(ctrl, loader, "/bazz", []*FetchDefinition{barFd}, time.Millisecond, map[string]interface{}{})
+	barFd := getFetchDefinitionMock(ctrl, loader, "/bar", nil, http.StatusOK, time.Millisecond*2, map[string]interface{}{"foo": "bar"})
+	fooFd := getFetchDefinitionMock(ctrl, loader, "/foo", []*FetchDefinition{barFd}, http.StatusOK, time.Millisecond*2, map[string]interface{}{"bli": "bla"})
+	bazzFd := getFetchDefinitionMock(ctrl, loader, "/bazz", []*FetchDefinition{barFd}, http.StatusOK, time.Millisecond, map[string]interface{}{})
 
 	fetcher := NewContentFetcher(nil)
 	fetcher.httpContentLoader = loader
@@ -91,7 +91,7 @@ func Test_ContentFetcher_FetchingWithDependency(t *testing.T) {
 	a.Equal("bla", meta["bli"])
 }
 
-func getFetchDefinitionMock(ctrl *gomock.Controller, loaderMock *MockContentLoader, url string, requiredContent []*FetchDefinition, loaderBlocking time.Duration, metaJSON map[string]interface{}) *FetchDefinition {
+func getFetchDefinitionMock(ctrl *gomock.Controller, loaderMock *MockContentLoader, url string, requiredContent []*FetchDefinition, requiredStatusCode int, loaderBlocking time.Duration, metaJSON map[string]interface{}) *FetchDefinition {
 	fd := NewFetchDefinition(url)
 	fd.Timeout = time.Second * 42
 
@@ -110,7 +110,7 @@ func getFetchDefinitionMock(ctrl *gomock.Controller, loaderMock *MockContentLoad
 			func(fetchDefinition *FetchDefinition) {
 				time.Sleep(loaderBlocking)
 			}).
-		Return(content, nil)
+		Return(content, nil, requiredStatusCode)
 
 	return fd
 }

composition/fetch_definition.go

@@ -167,6 +167,6 @@ func NewDefaultErrorHandler() *DefaultErrorHandler {
 	return deh
 }
 
-func (der *DefaultErrorHandler) Handle(err error, w http.ResponseWriter, r *http.Request) {
-	http.Error(w, "Bad Gateway: "+err.Error(), 502)
+func (der *DefaultErrorHandler) Handle(err error, status int, w http.ResponseWriter, r *http.Request) {
+	http.Error(w, "Error: "+err.Error(), status)
 }

composition/fetch_definition_test.go

@@ -41,5 +41,5 @@ func Test_FetchDefinition_use_DefaultErrorHandler_if_not_set(t *testing.T) {
 	a := assert.New(t)
 
 	fd := NewFetchDefinitionWithErrorHandler("http://upstream:8080/", nil)
-        a.Equal(NewDefaultErrorHandler(), fd.ErrHandler)
+	a.Equal(NewDefaultErrorHandler(), fd.ErrHandler)
 }

composition/file_content_loader.go

@@ -21,14 +21,14 @@ func NewFileContentLoader() *FileContentLoader {
 	}
 }
 
-func (loader *FileContentLoader) Load(fd *FetchDefinition) (Content, error) {
+func (loader *FileContentLoader) Load(fd *FetchDefinition) (Content, error, int) {
 	if fd.RespProc != nil {
-		return nil, ResponseProcessorsNotApplicable
+		return nil, ResponseProcessorsNotApplicable, 502
 	}
 	filename := strings.TrimPrefix(fd.URL, FileURLPrefix)
 	f, err := os.Open(filename)
 	if err != nil {
-		return nil, fmt.Errorf("error opening file %v: %v", fd.URL, err)
+		return nil, fmt.Errorf("error opening file %v: %v", fd.URL, err), 502
 	}
 
 	c := NewMemoryContent()
@@ -43,9 +43,9 @@ func (loader *FileContentLoader) Load(fd *FetchDefinition) (Content, error) {
 			WithField("duration", time.Since(parsingStart)).
 			Debug("content parsing")
 		f.Close()
-		return c, err
+		return c, err, c.httpStatusCode
 	}
 
 	c.reader = f
-	return c, nil
+	return c, nil, c.httpStatusCode
 }

composition/file_content_loader_test.go

@@ -24,7 +24,7 @@ func Test_FileContentLoader_LoadHTML(t *testing.T) {
 	a.NoError(err)
 
 	loader := NewFileContentLoader()
-	c, err := loader.Load(NewFetchDefinition(FileURLPrefix + fileName))
+	c, err, _ := loader.Load(NewFetchDefinition(FileURLPrefix + fileName))
 	a.NoError(err)
 	a.NotNil(c)
 	a.Nil(c.Reader())
@@ -42,7 +42,7 @@ func Test_FileContentLoader_LoadStream(t *testing.T) {
 	a.NoError(err)
 
 	loader := NewFileContentLoader()
-	c, err := loader.Load(NewFetchDefinition(FileURLPrefix + fileName))
+	c, err, _ := loader.Load(NewFetchDefinition(FileURLPrefix + fileName))
 	a.NoError(err)
 	a.NotNil(c)
 	body, err := ioutil.ReadAll(c.Reader())
@@ -54,7 +54,7 @@ func Test_FileContentLoader_LoadError(t *testing.T) {
 	a := assert.New(t)
 
 	loader := NewFileContentLoader()
-	_, err := loader.Load(NewFetchDefinition("/tmp/some/non/existing/path"))
+	_, err, _ := loader.Load(NewFetchDefinition("/tmp/some/non/existing/path"))
 	a.Error(err)
 }
 
@@ -66,7 +66,7 @@ func Test_FileContentLoader_RequestProcessor(t *testing.T) {
 	fd := NewFetchDefinition("/tmp/some/non/existing/path")
 	fd.RespProc = NewMockResponseProcessor(ctrl)
 
-	_, err := NewFileContentLoader().Load(fd)
+	_, err, _ := NewFileContentLoader().Load(fd)
 	a.Equal(ResponseProcessorsNotApplicable, err)
 }
 

composition/http_content_loader.go

@@ -23,13 +23,14 @@ func NewHttpContentLoader() *HttpContentLoader {
 }
 
 // TODO: Should we filter the headers, which we forward here, or is it correct to copy all of them?
-func (loader *HttpContentLoader) Load(fd *FetchDefinition) (Content, error) {
+func (loader *HttpContentLoader) Load(fd *FetchDefinition) (Content, error, int) {
 	client := &http.Client{Timeout: fd.Timeout}
+	statusCode := 502
 
 	var err error
 	request, err := http.NewRequest(fd.Method, fd.URL, fd.Body)
 	if err != nil {
-		return nil, err
+		return nil, err, 0
 	}
 	request.Header = fd.Header
 	if request.Header == nil {
@@ -41,26 +42,30 @@ func (loader *HttpContentLoader) Load(fd *FetchDefinition) (Content, error) {
 
 	resp, err := client.Do(request)
 
+	if resp != nil {
+		statusCode = resp.StatusCode
+	}
+
 	logging.Call(request, resp, start, err)
 	if err != nil {
-		return nil, err
+		return nil, err, statusCode
 	}
 
 	if fd.RespProc != nil {
 		err = fd.RespProc.Process(resp, fd.URL)
 	}
 	if err != nil {
-		return nil, err
+		return nil, err, statusCode
 	}
 
-	if resp.StatusCode < 200 || resp.StatusCode > 399 {
-		return nil, fmt.Errorf("(http %v) on loading url %q", resp.StatusCode, fd.URL)
+	if statusCode < 200 || statusCode > 399 {
+		return nil, fmt.Errorf("(http %v) on loading url %q", statusCode, fd.URL), statusCode
 	}
 
 	c := NewMemoryContent()
 	c.url = fd.URL
 	c.httpHeader = resp.Header
-	c.httpStatusCode = resp.StatusCode
+	c.httpStatusCode = statusCode
 
 	// take the first parser for the content type
 	// direct access to the map does not work, because the
@@ -81,11 +86,11 @@ func (loader *HttpContentLoader) Load(fd *FetchDefinition) (Content, error) {
 					WithField("full_url", c.URL()).
 					WithField("duration", time.Since(parsingStart)).
 					Debug("content parsing")
-				return c, err
+				return c, err, statusCode
 			}
 		}
 	}
 
 	c.reader = resp.Body
-	return c, nil
+	return c, nil, statusCode
 }

composition/http_content_loader_test.go

@@ -32,7 +32,7 @@ func Test_HttpContentLoader_Load(t *testing.T) {
 
 	loader.parser["text/html"] = mockParser
 
-	c, err := loader.Load(NewFetchDefinition(server.URL))
+	c, err, _ := loader.Load(NewFetchDefinition(server.URL))
 	a.NoError(err)
 	a.NotNil(c)
 	a.Nil(c.Reader())
@@ -64,7 +64,7 @@ func Test_HttpContentLoader_Load_ResponseProcessor(t *testing.T) {
 
 	mockResponseProcessor := NewMockResponseProcessor(ctrl)
 	mockResponseProcessor.EXPECT().Process(gomock.Any(), gomock.Any())
-	c, err := loader.Load(NewFetchDefinitionWithResponseProcessor(server.URL, mockResponseProcessor))
+	c, err, _ := loader.Load(NewFetchDefinitionWithResponseProcessor(server.URL, mockResponseProcessor))
 	a.NoError(err)
 	a.NotNil(c)
 	a.Nil(c.Reader())
@@ -105,7 +105,7 @@ func Test_HttpContentLoader_Load_POST(t *testing.T) {
 	fd.Method = "POST"
 	fd.Body = strings.NewReader("post content")
 
-	c, err := loader.Load(fd)
+	c, err, _ := loader.Load(fd)
 	a.NoError(err)
 	a.NotNil(c)
 	a.Nil(c.Reader())
@@ -124,7 +124,7 @@ func Test_HttpContentLoader_LoadStream(t *testing.T) {
 	defer server.Close()
 
 	loader := &HttpContentLoader{}
-	c, err := loader.Load(NewFetchDefinition(server.URL))
+	c, err, _ := loader.Load(NewFetchDefinition(server.URL))
 	a.NoError(err)
 	a.NotNil(c.Reader())
 	body, err := ioutil.ReadAll(c.Reader())
@@ -143,14 +143,31 @@ func Test_HttpContentLoader_LoadStream_No_Composition_Header(t *testing.T) {
 	defer server.Close()
 
 	loader := &HttpContentLoader{}
-	c, err := loader.Load(NewFetchDefinition(server.URL))
+	c, err, _ := loader.Load(NewFetchDefinition(server.URL))
 	a.NoError(err)
 	a.NotNil(c.Reader())
 	body, err := ioutil.ReadAll(c.Reader())
 	a.NoError(err)
 	a.Equal("{}", string(body))
 }
 
+func Test_HttpContentLoader_Pass_404(t *testing.T) {
+	a := assert.New(t)
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(404)
+		w.Write([]byte("{}"))
+	}))
+
+	defer server.Close()
+
+	loader := &HttpContentLoader{}
+	c, err, status := loader.Load(NewFetchDefinition(server.URL))
+	a.Error(err)
+	a.Nil(c)
+	a.Equal(404, status)
+}
+
 func Test_HttpContentLoader_LoadError500(t *testing.T) {
 	a := assert.New(t)
 
@@ -160,17 +177,18 @@ func Test_HttpContentLoader_LoadError500(t *testing.T) {
 	defer server.Close()
 
 	loader := &HttpContentLoader{}
-	c, err := loader.Load(NewFetchDefinition(server.URL))
+	c, err, statusCode := loader.Load(NewFetchDefinition(server.URL))
 	a.Error(err)
 	a.Nil(c)
 	a.Contains(err.Error(), "http 500")
+	assert.True(t, statusCode == 500)
 }
 
 func Test_HttpContentLoader_LoadErrorNetwork(t *testing.T) {
 	a := assert.New(t)
 
 	loader := &HttpContentLoader{}
-	c, err := loader.Load(NewFetchDefinition("..."))
+	c, err, _ := loader.Load(NewFetchDefinition("..."))
 	a.Error(err)
 	a.Nil(c)
 	a.Contains(err.Error(), "unsupported protocol scheme")

composition/interface_mocks_test.go

@@ -5,7 +5,7 @@ package composition
 
 import (
 	gomock "github.com/golang/mock/gomock"
-	
+
 	io "io"
 	http "net/http"
 )
@@ -62,11 +62,12 @@ func (_m *MockContentLoader) EXPECT() *_MockContentLoaderRecorder {
 	return _m.recorder
 }
 
-func (_m *MockContentLoader) Load(_param0 *FetchDefinition) (Content, error) {
+func (_m *MockContentLoader) Load(_param0 *FetchDefinition) (Content, error, int) {
 	ret := _m.ctrl.Call(_m, "Load", _param0)
 	ret0, _ := ret[0].(Content)
 	ret1, _ := ret[1].(error)
-	return ret0, ret1
+	ret2, _ := ret[2].(int)
+	return ret0, ret1, ret2
 }
 
 func (_mr *_MockContentLoaderRecorder) Load(arg0 interface{}) *gomock.Call {