4 ms·
On the surface this sounds logical. But going back to the case I dealt with recently, all errors during authentication lead back to an HTTP 403 workflow. Errors
by jniedrauer 7y ago
On the surface this sounds logical. But going back to the case I dealt with recently, all errors during authentication lead back to an HTTP 403 workflow. Errors themselves can occur a long distance from any sane central place to handle them. Unless you pass the http.ResponseWriter all the way up the stack, the only "in place" handling you could do is to log every error along with a unique string.
Is this really the best we can do?
if err := frobulate(); err != nil {
log.Printf("Error on line 123: %+v", err)
return err
}
- q3k 7y agoYou example is code smell. When dealing with error handling in Go (and many other languages), you should do one _and only one_ of the following: - if you have enough context to handle the error (eg. retry, or degrade gracefully, or perform whatever other application logic to recover, possibly returning a more friendly error to your caller), do that, possibly logging the source error and do not return it further up the stack directly - you have now healed the error condition, and the application code can continue. - otherwise, return the error, decorating it with extra context if possible. Doing both, ie. logging (which is a case of handling the error) and returning it up the stack leads to extremely chatty and difficult to debug code. Errors are a fact of life, and your path of handling them should be as well engineered as the happy life. That's one of the most important things in writing reliable software. That's also one of the reasons there's no generic stack unwinding/exceptions on Go: it forces you to _think_ about error cases and at least reminds you to handle them gracefully. You might not like this approach of treating programmers this way, but that's one of the cornerstones of Go.
- jniedrauer 7y agoWell, sure, I agree with you. But then we're back to wrapping every single error in a unique string before returning it. Developers simply don't do that, so you end up with the un-debuggable mess that I had.
- klodolph 7y agoYou don’t wrap every error in a unique string, only in places where there is additional context. Developers I work with do this.
- wst_ 7y agoAnd hundreds of others don't. "It works in my place" is never a good argument. Thing is that people are taking shortcuts. We have that in our DNA. If something isn't imposed on us, will be abused. I saw a code base of almost 2M lines, of which 1.5M is written in Go by seasoned developers. You'd be surprised how messy it gets over time and how many times people forget to handle errors or just decide not to, out of convenience. Not to mention other design issues, simply, because Go doesn't protect itself against almost anything. If Go authors counted on self-discipline of the devs then, sorry, not sure what planet are they living on.
- xienze 7y ago> - if you have enough context to handle the error (eg. retry, or degrade gracefully, or perform whatever other application logic to recover, possibly returning a more friendly error to your caller), do that, possibly logging the source error and do not return it further up the stack directly - you have now healed the error condition, and the application code can continue. > - otherwise, return the error, decorating it with extra context if possible. This is precisely how checked exception handling works in Java, and yet there's no shortage of people complaining about it and praising how Go "makes you deal with it the way you're supposed to." > it forces you to _think_ about error cases and at least reminds you to handle them gracefully. So do checked exceptions. And 99% of the time, the solution to an error popping up in Java is to blindly re-throw it because hey, I'm just a component buried a couple levels deep in a REST call, what could I possibly do to fix the problem?
- badsectoracula 7y agoInterestingly you bring checked exceptions, i used to dislike them at the past but in recent years i came to appreciate having a visible flow of execution (and as such i prefer the C approach of explicit returns - note that i haven't used Go at all, i'm just curious about it) which made me think that Java forcing you to acknowledge any exception that may pass through and actively make a decision about it (handle it or explicitly pass it through) is a good thing. Though it also made me dislike its decision for runtime exceptions since they essentially bypass the checking (on the other hand they'd somehow had to handle things like NullPointerException, otherwise everything would need to declare that it would throw it - though perhaps that is a side effect of Java having nullable pointers and a different language without nullable pointers wouldn't need that... but that'd only fix the NullPointerException case).
- klodolph 7y agoError handling is hard. Checked exceptions and Go’s error returns are both good attempts to solve it. They both have plenty of drawbacks but they’re really different approaches here. > This is precisely how checked exception handling works in Java, and yet there's no shortage of people complaining about it and praising how Go "makes you deal with it the way you're supposed to." Just because people complain about Java’s checked exceptions does’t mean that their complaints are correct or that they somehow apply to Go’s errors. The main complaints I hear about checked exceptions are that they force you to expose the types of all possible exceptions in the method signature. This creates unexpected API churn and design difficulties, because it’s difficult to predict which exception types you would want to expose from a particular interface. You also have to choose which errors are programming errors and which are not when you declare the error type. This turned out to cause problems. NumberFormatException is an example. > So do checked exceptions. And 99% of the time, the solution to an error popping up in Java is to blindly re-throw it because hey, I'm just a component buried a couple levels deep in a REST call, what could I possibly do to fix the problem? In Go, blindly returning an error is done much less than 99% of the time, at least in the code bases I’ve worked on or read.
- RHSeeger 7y agoWhat about the situation where you need to clean up a little but you can't "handle" the error yourself? A simple example being 1. Open a file for reading 2. Try to open a file for writing, but error 3. Close the file that was opened for reading 4. Propagate the error upwards (because the reason this function was called failed)
- klodolph 7y agoThis is just returning the error upwards, as far as I can tell. This is the pattern I usually see in actual Go codebases: infp, err := os.Open(infile) if err != nil { return err } defer infp.Close() outfp, err := os.Create(outfile) if err != nil { return err } defer outfp.Close() // ... do some stuff ... return outfp.Close() Note that double-close is explicitly OK, this pattern just avoids losing the error.
- wst_ 7y agoCould you, please, expand on double closing? I can't find any good reason to do it.
- klodolph 7y agoOn a file open for writing, close(2) may return EIO. If you ignore the error, you have silent data loss. So you must propagate the error. The double-close pattern is just the easy way to accomplish this in go. You defer fp.Close(), and then return fp.Close(). The return fp.Close() closes the file and propagates the error. Afterwards, the deferred fp.Close() executes, but this simply returns an error (which is ignored) because the file is already closed. The behavior of fp.Close(), the fact that it is safe to call multiple times, is documented in the Go documentation. There are alternative ways to achieve the same effect but they require more code. So I use the double-close pattern everywhere, and then put a comment in every time I use it.
- flippinburgers 7y agoI don't find it unreasonable to log and return. You know what happened where exactly, but the returned error might be part of a larger process that needs cleanup of other kinds. I default to immediately logging and returning with all the code I write.
- omegabravo 7y agoit is very noisy and hard to see what is happening if you log and return an error at every layer of the stack. Like everything there are exceptions to the rule and at times it makes sense to do both, but it shouldn't be the norm.
- JulianMorrison 7y agoI feel like the rule should be 1. If you can handle it, you probably should also log it since you know what it means and what's being done about it. 2. Otherwise, if you can add pertinent information, like for example the fact that the file you couldn't find was supposed to be the SQLite database, but can't handle it here, wrap and return. Do not log. 3. Otherwise, sticky fingers off, pass it up unaltered. Do not log.