From 6a9574a5ad6a942fbaaa751aa778510f723fc0cf Mon Sep 17 00:00:00 2001 From: DanielMieg Date: Mon, 3 Jul 2023 16:44:32 +0200 Subject: [PATCH 1/4] Add details for EOF errors --- cmd/abapEnvironmentCloneGitRepo.go | 19 +++++++++---------- cmd/abapEnvironmentPullGitRepo.go | 6 +++--- pkg/abaputils/abaputils.go | 21 ++++++++++++++++++++- pkg/abaputils/manageGitRepositoryUtils.go | 10 +++++----- 4 files changed, 37 insertions(+), 19 deletions(-) diff --git a/cmd/abapEnvironmentCloneGitRepo.go b/cmd/abapEnvironmentCloneGitRepo.go index 6481b3dae7..7a2417e7e7 100644 --- a/cmd/abapEnvironmentCloneGitRepo.go +++ b/cmd/abapEnvironmentCloneGitRepo.go @@ -29,7 +29,6 @@ func abapEnvironmentCloneGitRepo(config abapEnvironmentCloneGitRepoOptions, _ *t } client := piperhttp.Client{} - // error situations should stop execution through log.Entry().Fatal() call which leads to an os.Exit(1) in the end err := runAbapEnvironmentCloneGitRepo(&config, &autils, &client) if err != nil { @@ -76,9 +75,9 @@ func runAbapEnvironmentCloneGitRepo(config *abapEnvironmentCloneGitRepoOptions, logString := repo.GetCloneLogString() errorString := "Clone of " + logString + " failed on the ABAP system" - log.Entry().Info("-------------------------") + abaputils.AddDefaultDashedLine() log.Entry().Info("Start cloning " + logString) - log.Entry().Info("-------------------------") + abaputils.AddDefaultDashedLine() // Triggering the Clone of the repository into the ABAP Environment system uriConnectionDetails, errorTriggerClone, didCheckoutPullInstead := triggerClone(repo, connectionDetails, client) @@ -99,7 +98,7 @@ func runAbapEnvironmentCloneGitRepo(config *abapEnvironmentCloneGitRepoOptions, log.Entry().Info("The " + logString + " was cloned successfully") } } - log.Entry().Info("-------------------------") + abaputils.AddDefaultDashedLine() log.Entry().Info("All repositories were cloned successfully") return nil } @@ -191,12 +190,12 @@ func handleCloneError(resp *http.Response, err error, cloneConnectionDetails aba // As an intermediate workaround, we react to the error message A4C_A2G/257 that gets thrown, if the repository had already been cloned // In this case, a checkout branch and a pull will be performed alreadyCloned = true - log.Entry().Infof("-------------------------") - log.Entry().Infof("-------------------------") + abaputils.AddDefaultDashedLine() + abaputils.AddDefaultDashedLine() log.Entry().Infof("%s", "The repository / software component has already been cloned on the ABAP Environment system ") log.Entry().Infof("%s", "A `checkout branch` and a `pull` will be performed instead") - log.Entry().Infof("-------------------------") - log.Entry().Infof("-------------------------") + abaputils.AddDefaultDashedLine() + abaputils.AddDefaultDashedLine() checkoutOptions := abapEnvironmentCheckoutBranchOptions{ Username: cloneConnectionDetails.User, Password: cloneConnectionDetails.Password, @@ -214,8 +213,8 @@ func handleCloneError(resp *http.Response, err error, cloneConnectionDetails aba if returnedError != nil { return } - log.Entry().Infof("-------------------------") - log.Entry().Infof("-------------------------") + abaputils.AddDefaultDashedLine() + abaputils.AddDefaultDashedLine() pullOptions := abapEnvironmentPullGitRepoOptions{ Username: cloneConnectionDetails.User, Password: cloneConnectionDetails.Password, diff --git a/cmd/abapEnvironmentPullGitRepo.go b/cmd/abapEnvironmentPullGitRepo.go index a4948ae387..659b6f85aa 100644 --- a/cmd/abapEnvironmentPullGitRepo.go +++ b/cmd/abapEnvironmentPullGitRepo.go @@ -95,9 +95,9 @@ func handlePull(repo abaputils.Repository, pullConnectionDetails abaputils.Conne logString := repo.GetPullLogString() errorString := "Pull of the " + logString + " failed on the ABAP system" - log.Entry().Info("-------------------------") + abaputils.AddDefaultDashedLine() log.Entry().Info("Start pulling the " + logString) - log.Entry().Info("-------------------------") + abaputils.AddDefaultDashedLine() uriConnectionDetails, err := triggerPull(repo, pullConnectionDetails, client) if err != nil { @@ -183,7 +183,7 @@ func checkPullRepositoryConfiguration(options abapEnvironmentPullGitRepoOptions) } func finishPullLogs() { - log.Entry().Info("-------------------------") + abaputils.AddDefaultDashedLine() log.Entry().Info("All repositories were pulled successfully") } diff --git a/pkg/abaputils/abaputils.go b/pkg/abaputils/abaputils.go index edb66b2f3b..fbace872fa 100644 --- a/pkg/abaputils/abaputils.go +++ b/pkg/abaputils/abaputils.go @@ -186,11 +186,20 @@ func HandleHTTPError(resp *http.Response, err error, message string, connectionD if resp == nil { // Response is nil in case of a timeout log.Entry().WithError(err).WithField("ABAP Endpoint", connectionDetails.URL).Error("Request failed") + + match, _ := regexp.MatchString(".*EOF$", err.Error()) + if match { + AddDefaultDashedLine() + log.Entry().Infof("%s", "A connection could not be established to the ABAP system. The typical root cause is the network configuration (firewall, IP allowlist, etc.)") + AddDefaultDashedLine() + } + + log.Entry().Infof("Error message: %s,", err.Error()) } else { defer resp.Body.Close() - log.Entry().WithField("StatusCode", resp.Status).Error(message) + log.Entry().WithField("StatusCode", resp.Status).WithField("User", connectionDetails.User).WithField("URL", connectionDetails.URL).Error(message) errorText, errorCode, parsingError := GetErrorDetailsFromResponse(resp) if parsingError != nil { @@ -239,6 +248,16 @@ func ConvertTime(logTimeStamp string) time.Time { return t } +// AddDefaultDashedLine adds 25 dashes +func AddDefaultDashedLine() { + log.Entry().Infof(strings.Repeat("-", 25)) +} + +// AddDefaultDebugLine adds 25 dashes in debug +func AddDebugDashedLine() { + log.Entry().Debugf(strings.Repeat("-", 25)) +} + /******************************* * Structs for specific steps * *******************************/ diff --git a/pkg/abaputils/manageGitRepositoryUtils.go b/pkg/abaputils/manageGitRepositoryUtils.go index 8399b2ef15..954ea78dfe 100644 --- a/pkg/abaputils/manageGitRepositoryUtils.go +++ b/pkg/abaputils/manageGitRepositoryUtils.go @@ -62,7 +62,7 @@ func PrintLogs(repositoryName string, connectionDetails ConnectionDetailsHTTP, c for _, logEntryForDetails := range entity.ToLogOverview.Results { printLog(logEntryForDetails, connectionDetails, client) } - log.Entry().Infof("-------------------------") + AddDefaultDashedLine() return } @@ -151,14 +151,14 @@ func allLogsHaveBeenPrinted(entity LogProtocolResults, page int, err error) bool func printHeader(logEntry LogResultsV2) { if logEntry.Status != `Success` { log.Entry().Infof("\n") - log.Entry().Infof("-------------------------") + AddDefaultDashedLine() log.Entry().Infof("%s (%v)", logEntry.Name, ConvertTime(logEntry.Timestamp)) - log.Entry().Infof("-------------------------") + AddDefaultDashedLine() } else { log.Entry().Debugf("\n") - log.Entry().Debugf("-------------------------") + AddDebugDashedLine() log.Entry().Debugf("%s (%v)", logEntry.Name, ConvertTime(logEntry.Timestamp)) - log.Entry().Debugf("-------------------------") + AddDebugDashedLine() } } From e64d21ca110b9ec06a98af604f6e35a3f324f03e Mon Sep 17 00:00:00 2001 From: DanielMieg Date: Fri, 7 Jul 2023 14:46:36 +0200 Subject: [PATCH 2/4] Add testcase --- pkg/abaputils/abaputils_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pkg/abaputils/abaputils_test.go b/pkg/abaputils/abaputils_test.go index 9ccd2fd4b8..3039358a32 100644 --- a/pkg/abaputils/abaputils_test.go +++ b/pkg/abaputils/abaputils_test.go @@ -350,4 +350,18 @@ func TestHandleHTTPError(t *testing.T) { assert.EqualError(t, err, fmt.Sprintf("%s", receivedErr.Error())) log.Entry().Info(err.Error()) }) + + t.Run("Timeout with EOF", func(t *testing.T) { + + errorValue := "Received Error EOF" + + var resp http.Response + resp = http.Response{} + receivedErr := errors.New(errorValue) + message := "Custom Error Message" + + err := HandleHTTPError(&resp, receivedErr, message, ConnectionDetailsHTTP{}) + assert.EqualError(t, err, fmt.Sprintf("%s", receivedErr.Error())) + log.Entry().Info(err.Error()) + }) } From ec6a97cab9b60dcd1b627de501a80108c7070422 Mon Sep 17 00:00:00 2001 From: DanielMieg Date: Fri, 7 Jul 2023 15:09:12 +0200 Subject: [PATCH 3/4] remove test --- pkg/abaputils/abaputils_test.go | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/pkg/abaputils/abaputils_test.go b/pkg/abaputils/abaputils_test.go index 3039358a32..9ccd2fd4b8 100644 --- a/pkg/abaputils/abaputils_test.go +++ b/pkg/abaputils/abaputils_test.go @@ -350,18 +350,4 @@ func TestHandleHTTPError(t *testing.T) { assert.EqualError(t, err, fmt.Sprintf("%s", receivedErr.Error())) log.Entry().Info(err.Error()) }) - - t.Run("Timeout with EOF", func(t *testing.T) { - - errorValue := "Received Error EOF" - - var resp http.Response - resp = http.Response{} - receivedErr := errors.New(errorValue) - message := "Custom Error Message" - - err := HandleHTTPError(&resp, receivedErr, message, ConnectionDetailsHTTP{}) - assert.EqualError(t, err, fmt.Sprintf("%s", receivedErr.Error())) - log.Entry().Info(err.Error()) - }) } From 85bd4f5746401f534bfaab8182c9567ccfab70b7 Mon Sep 17 00:00:00 2001 From: DanielMieg Date: Fri, 14 Jul 2023 12:03:44 +0200 Subject: [PATCH 4/4] Add unit test --- pkg/abaputils/abaputils_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pkg/abaputils/abaputils_test.go b/pkg/abaputils/abaputils_test.go index 9ccd2fd4b8..82184e7b59 100644 --- a/pkg/abaputils/abaputils_test.go +++ b/pkg/abaputils/abaputils_test.go @@ -13,6 +13,7 @@ import ( "github.com/SAP/jenkins-library/pkg/log" "github.com/SAP/jenkins-library/pkg/mock" "github.com/pkg/errors" + "github.com/sirupsen/logrus/hooks/test" "github.com/stretchr/testify/assert" ) @@ -350,4 +351,21 @@ func TestHandleHTTPError(t *testing.T) { assert.EqualError(t, err, fmt.Sprintf("%s", receivedErr.Error())) log.Entry().Info(err.Error()) }) + + t.Run("EOF Error", func(t *testing.T) { + + message := "Custom Error Message" + errorValue := "Received Error EOF" + receivedErr := errors.New(errorValue) + + _, hook := test.NewNullLogger() + log.RegisterHook(hook) + + err := HandleHTTPError(nil, receivedErr, message, ConnectionDetailsHTTP{}) + + assert.EqualError(t, err, fmt.Sprintf("%s", receivedErr.Error())) + assert.Equal(t, 5, len(hook.Entries), "Expected a different number of entries") + assert.Equal(t, `A connection could not be established to the ABAP system. The typical root cause is the network configuration (firewall, IP allowlist, etc.)`, hook.AllEntries()[2].Message, "Expected a different message") + hook.Reset() + }) }