[FSSDK-9155] fix(utils): logging correct error message in HTTP requests (#374)

* Fixed error being logged in HTTP requests

* Added test for logged errors

* Improve test error checking + comments

* Fix unnecessary re-cast in Error

* Update copyright dates in headers

* add review changes

* Update bad url example

---------

Co-authored-by: Sean Pfeifer <[email protected]>

Author: pulak-opti

pkg/utils/requester.go

@@ -1,5 +1,5 @@
 /****************************************************************************
- * Copyright 2019,2022 Optimizely, Inc. and contributors                    *
+ * Copyright 2019,2022-2023 Optimizely, Inc. and contributors               *
  *                                                                          *
  * Licensed under the Apache License, Version 2.0 (the "License");          *
  * you may not use this file except in compliance with the License.         *
@@ -143,7 +143,7 @@ func (r HTTPRequester) Do(url, method string, body io.Reader, headers []Header)
 	single := func(request *http.Request) (response []byte, responseHeaders http.Header, code int, e error) {
 		resp, doErr := r.client.Do(request)
 		if doErr != nil {
-			r.logger.Error(fmt.Sprintf("failed to send request %v", request), e)
+			r.logger.Error(fmt.Sprintf("failed to send request %v", request), doErr)
 			return nil, http.Header{}, 0, doErr
 		}
 		defer func() {

pkg/utils/requester_test.go

@@ -1,5 +1,5 @@
 /****************************************************************************
- * Copyright 2019,2021-2022 Optimizely, Inc. and contributors               *
+ * Copyright 2019,2021-2023 Optimizely, Inc. and contributors               *
  *                                                                          *
  * Licensed under the Apache License, Version 2.0 (the "License");          *
  * you may not use this file except in compliance with the License.         *
@@ -190,12 +190,41 @@ func TestPostObj(t *testing.T) {
 	assert.NotNil(t, err)
 }
 
+type mockLogger struct {
+	Errors []error
+}
+
+func (m *mockLogger) Debug(message string)   {}
+func (m *mockLogger) Info(message string)    {}
+func (m *mockLogger) Warning(message string) {}
+func (m *mockLogger) Error(message string, err interface{}) {
+	if err, ok := err.(error); ok {
+		m.Errors = append(m.Errors, err)
+	}
+}
+
 func TestGetBad(t *testing.T) {
+	// Using a mockLogger to ensure we're logging the expected error message
+	mLogger := &mockLogger{}
+	httpreq := NewHTTPRequester(mLogger)
 
-	httpreq := NewHTTPRequester(logging.GetLogger("", ""))
-	_, _, _, err := httpreq.Get("blah12345/good")
-	_, ok := err.(*url.Error)
+	badURL := "http://ww.bad-url.fake/blah12345"
+	_, _, _, err := httpreq.Get(badURL)
+	returnedErr, ok := err.(*url.Error)
 	assert.True(t, ok, "url error")
+
+	// Check to make sure we have some log for bad url
+	assert.NotNil(t, mLogger.Errors)
+	// If we didn't get the expected error, we need to stop before we do the rest
+	// of the checks that depend on that error.
+	if !assert.Len(t, mLogger.Errors, 1, "logged error") {
+		t.FailNow()
+	}
+	// Check to make sure the error that was logged is the same as what was returned
+	loggedErr, ok := mLogger.Errors[0].(*url.Error)
+	assert.True(t, ok, "is URL error")
+	assert.Equal(t, returnedErr, loggedErr, "expected same error")
+	assert.Equal(t, badURL, loggedErr.URL, "expected the URL we requested")
 }
 
 func TestGetBadWithResponse(t *testing.T) {