Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A module like this should never log an error, never log anything to stdout, and should always return an error if one exists #101

Open
bmayfi3ld opened this issue Jul 2, 2020 · 0 comments
Assignees

Comments

@bmayfi3ld
Copy link

A module should never log an error. It should always be returned to the user to decide what to do with the error (ignore or move on). When it is logged to stdout randomly by underlying packages the caller of the api can't really control or even know what is going on. This is so established, I couldn't even find an article that talks about doing it, they all just assume you are already doing it because it is so basic to the language.

https://golang.org/doc/effective_go.html#errors
https://medium.com/@hussachai/error-handling-in-go-a-quick-opinionated-guide-9199dd7c7f76
https://blog.golang.org/error-handling-and-go
https://medium.com/rungo/error-handling-in-go-f0125de052f0
https://blog.logrocket.com/error-handling-in-golang/

All of these are talking about different techniques around error handling, but all assume any non-main function is returning error for the higher level functions to manage.

I see in this codebase has a large number of areas where the log package is used to print errors, sometimes without the underlying package even returning the error.

In NewTransportConfig
if err != nil { log.Printf("Cannot load certificate file '%s'", sslVerify) return } if !caPool.AppendCertsFromPEM(cert) { err = fmt.Errorf("Cannot append certificate from file '%s'", sslVerify) return }

Error is just printed in the first block and the second block creates an error but does not return it?

In getHTTPResponseError
msg := fmt.Sprintf("WAPI request error: %d('%s')\nContents:\n%s\n", resp.StatusCode, resp.Status, content) log.Printf(msg) return errors.New(msg)

This at least returns the error but it still should not print to stdout.

if err != nil { log.Fatal(err) }
Here we kill the program because of the error. What if I wanted there to be an error here? what if I was checking for something to not work?

if err != nil { log.Printf("Http Reponse ioutil.ReadAll() Error: '%s'", err) return }

Here we at least return the error but still it should not be printed out.

There are many more examples but I think this is enough for the post.

@bmayfi3ld bmayfi3ld changed the title A module like this should never log an error, nor log anything to stdout A module like this should never log an error, nor log anything to stdout, should always return an error if one exists Jul 2, 2020
@bmayfi3ld bmayfi3ld changed the title A module like this should never log an error, nor log anything to stdout, should always return an error if one exists A module like this should never log an error, never log anything to stdout, and should always return an error if one exists Jul 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants