Fix/4xx logs

[Umang01-hash]

Pull Request Template

Description:

• Closes issue Avoid Logging 4XX Status Codes as Errors #1279

Checklist:

• I have formatted my code using goimport and golangci-lint.
• All new code is covered by unit tests.
• This PR does not decrease the overall code coverage.
• I have reviewed the code comments and documentation for clarity.

Thank you for your contribution!

[ccoVeille]

If you have to do this, it means you could do better with the struct definition

Suggested change

	func isHTTPError(err error) bool {
	httpErrors := []error{
	&gofrHTTP.ErrorEntityAlreadyExist{},
	&gofrHTTP.ErrorEntityNotFound{},
	&gofrHTTP.ErrorInvalidParam{},
	&gofrHTTP.ErrorMissingParam{},
	}
	for _, knownErr := range httpErrors {
	if errors.As(err, &knownErr) {
	return true
	}
	}
	return false
	}
	func isHTTPError(err error) bool {
	return errors.Is(err, gofrHTTP.Error) {
	}

Now let's move to gofr http package

Here is the current code for ErrorEntityNotFound

// ErrorEntityNotFound represents an error for when an entity is not found in the system.
type ErrorEntityNotFound struct {
        Name  string
        Value string
}

func (e ErrorEntityNotFound) Error() string {
        // For ex: "No entity found with id: 2"
        return fmt.Sprintf("No entity found with %s: %s", e.Name, e.Value)
}

func (ErrorEntityNotFound) StatusCode() int {
        return http.StatusNotFound
}

Let's define a error, then add a method to ErrorEntityNotFound

var ErrCommon = errors.New("HTTP error")

func (ErrorEntityNotFound) UnWrap() error {
        return ErrCommon
}

[ccoVeille]

You changed the implementation

Please consider what I suggested anyway

[ccoVeille]

You can even define errCommon as unexported

And add this to http package

func IsHTTPError(err error) bool {
    return errors.Is(err, errCommon)
}

[Umang01-hash]

@ccoVeille the current code of the errors in Http package represents different structs for various scenarios like entity not found, entity already exist, missing param, invalid param etc.

Each of these structs are implementing a Error() string as well as StatusCode() int methods that help us to set the statuscode from framework if any of them has been used.

I don't think we can entirely change that.

But I would really like if you can review the new implementation for logError and tell how can we improve on it.

[Umang01-hash]

@ccoVeille we can't use errors.Is as it will match the exact value (then the populated fields in the error struct will be treated as different error)

So based on your comment can we do something like this:

import (
    "errors"
    gofrHTTP "path/to/gofrHTTP"
)

// Log the error (if any) with traceID and errorMessage.
func (h handler) logError(traceID string, err error) {
    if err != nil {
        errorLog := &ErrorLogEntry{TraceID: traceID, Error: err.Error()}

        if isKnownError(err) {
            h.container.Logger.Info(errorLog)
        } else {
            h.container.Logger.Error(errorLog)
        }
    }
}

func isKnownError(err error) bool {
    switch {
    case isEntityAlreadyExistError(err):
        return true
    case isEntityNotFoundError(err):
        return true
    case isInvalidParamError(err):
        return true
    case isMissingParamError(err):
        return true
    default:
        return false
    }
}

func isEntityAlreadyExistError(err error) bool {
    var target *gofrHTTP.ErrorEntityAlreadyExist
    return errors.As(err, &target)
}

func isEntityNotFoundError(err error) bool {
    var target *gofrHTTP.ErrorEntityNotFound
    return errors.As(err, &target)
}

func isInvalidParamError(err error) bool {
    var target *gofrHTTP.ErrorInvalidParam
    return errors.As(err, &target)
}

func isMissingParamError(err error) bool {
    var target *gofrHTTP.ErrorMissingParam
    return errors.As(err, &target)
}

[ccoVeille]

I disagree, mainly because that's the purpose of adding Unwrap

I could work on a better code, but I don't have time yet

[Umang01-hash]

@ccoVeille will wait for your suggestion on a better code.

[ccoVeille]

I opened #1298

[Umang01-hash]

Closing this PR as the work of this PR has been addressed in PR #1298.