How to tidy up repeated if err := x(); err != nil { return err }?

Is there a better way to write this, something similar to <a href="https://www.postgresql.org/docs/current/static/functions-conditional.html#FUNCTIONS-COALESCE-NVL-IFNULL">PostgreSQL COALESCE</a>? <pre><code>if err := a(); err != nil { return err } if err := b(); err != nil { return err } return c() </code></pre> <hr/>**评论：** JackOhBlades: <pre>I think there's some psychology behind why this comes up a lot. As a Go newb you return errors without touching them and you start to wonder, why do I need to repetitively check and return? Couldn't this be like, implicit, or something? It's just really non-DRY code! I'm just doing the same thing over again and Go is forcing me to code this way! And then after digesting some Dave Chaney posts and other Go materials you start to realise that maybe you shouldn't be just returning the errors. Maybe the problem isn't with Go, it's with the fact that you're choosing, over and over again, to do nothing with the error. Then you start wrapping errors with context and you realise there's actually nothing to DRY up, you were simply choosing not to handle the error correctly before. Finally you either accept or disagree with one thing: Go forces you to handle the error. Even if that means simply returning it. It puts that choice in your face, making it hard for programmers to ignore inconvenient errors. This is important for scaling code bases, especially when on-boarding new programmers who might be tempted to not handle errors. I don't know where I was going with this. </pre>shovelpost: <pre><blockquote> Go forces you to handle the error. </blockquote> I agree with everything you said except that technically Go doesn't force you to handle errors (though I understand what you mean). For example this is the signature of <a href="https://golang.org/pkg/fmt/#Println">fmt.Println</a>: <pre><code>func Println(a ...interface{}) (n int, err error) </code></pre> I don't see much error checking on that! :P</pre>YEPHENAS: <pre><blockquote> I agree with everything you said except that technically Go doesn't force you to handle errors (though I understand what you mean). </blockquote> psychological force, not technical force</pre>shovelpost: <pre><a href="https://www.youtube.com/watch?v=hou0lU8WMgo" rel="nofollow">You're technically correct.</a></pre>JackOhBlades: <pre>I foresaw this response! ;) I'd argue that my comments generally hold true in practice, errors that should be handled are hard to not handle. I think a strong counter point would be an anecdote describing how it was Go semantics, not the programmer, that was mostly responsible for why a serious error was not handled correctly, or rather, was hard to handle correctly. </pre>FierceDeity_: <pre>Basically I'd say the "psychological forcing" is in that you have to purposefully ignore go errors by typing comma, underscore every time. In a language like C# ignoring them is much easier than in go. In go you have to explicitly not take that error</pre>SeerUD: <pre>I still have a love/hate relationship with this part of Go, even a couple of years in now. I like how simple errors are, but they're not easy. Go is the first language that I've used where I've had to add the context myself manually, and I think that's where the problem lies in it for me. I don't know why people seem to hate stack traces so much. Sure, they can go out of date, but provided your logging is sufficient (e.g. log app version too?) then you don't have a problem there either. They can be long, but at the end of it all, they tell you exactly where to look for a problem. This is part of what <code>pkg/errors</code> provides (if you manually add it on each error), but I feel like it could be part of the language really.</pre>JackOhBlades: <pre>Stack traces will tell you the nitty gritty details you didn't ask for, this can at least, slow you down a lot, and at most, completely bury the actual problem. Stack traces can't capture the semantics of the error. Good error handling encapsulates domain specific details, the semantics of the error (in my opinion). I don't think they are mutually exclusive: you can have good errors and a stack trace included for good measure. If you want some interesting material on error handling and Go I really enjoyed these articles: <ul> <li><a href="https://commandcenter.blogspot.com.au/2017/12/error-handling-in-upspin.html" rel="nofollow">error handling in upspin</a></li> <li><a href="https://hackmd.io/s/B1NrW6_dM" rel="nofollow">the two sides of an error</a></li> </ul></pre>SeerUD: <pre>I agree; context is very important. Like you've said though, they're not mutually exclusive. If stack traces were a built-in piece of functionality for errors then it'd just make the process easier.</pre>JackOhBlades: <pre>I think one must also consider that stack traces aren't free. There's a performance penalty associated with them, and so making them a default would be questionable given Go's philosophy of making costly operations explicit. I'm not 100% sure what you mean by 'built-in', though you can add this functionality quite easily with <code>github.com/pkg/errors.Wrap</code> which adds a stack trace to any given error (thanks <a href="/u/qu33ksilver" rel="nofollow">/u/qu33ksilver</a>!). I do empathise with your concerns. At the end of the day, I think I agree with Go on this; the ergonomic cost is preferable to the performance cost... But this is purely my opinion.</pre>SeerUD: <pre>By built-in, I made automatic really. Or at least, maybe you should be able to ask for the stack for an error at a given point? Sort of like how panics are able to give you a stack trace too, at the time of the panic.</pre>skidooer: <pre>Interestingly, the core Go team found that <a href="https://commandcenter.blogspot.com/2017/12/error-handling-in-upspin.html" rel="nofollow">stack traces weren't all that useful in the presence of meaningful errors</a>. <blockquote> For those cases where stack traces would be helpful, we allow the errors package to be built with the "debug" tag, which enables them. This works fine, but it's worth noting that we have almost never used this feature. </blockquote></pre>therealfakemoot: <pre><blockquote> Then you start wrapping errors with context and you realise there's actually nothing to DRY up, you were simply choosing not to handle the error correctly before. </blockquote> Could you elaborate on that point a little more? I'd say I'm somewhere between novice and intermediate Go development skill, and at an abstract level I understand the "add context to errors" bit, but I"m not 100% sure how that looks in actual Go code. I've seen loads of libraries/toolkits that let you wrap errors or have fancy error value chains ( for building custom stacktraces, effectively, I guess ) but I've yet to fully realize a meaningful implementation of this behavior/pattern.</pre>skidooer: <pre>There is a <a href="https://commandcenter.blogspot.com/2017/12/error-handling-in-upspin.html" rel="nofollow">good article</a> from Rob Pike and Andrew Gerrand about error handling and wrapping context that you might find useful.</pre>therealfakemoot: <pre>Nice, looks like a good read. Thanks for sharing.</pre>JackOhBlades: <pre>Along with the upspin article as <a href="/u/skidooer" rel="nofollow">/u/skidooer</a> suggested, I'd recommend <a href="https://hackmd.io/s/B1NrW6_dM" rel="nofollow">the two sides of an error</a>. Here's some real code that I wrote to demonstrate adding context to errors: <a href="https://play.golang.org/p/84sdS4KopCG" rel="nofollow">https://play.golang.org/p/84sdS4KopCG</a> The program downloads any URL links you feed to it (concurrently, in true Go fashion). This is not advanced or sophisticated error handling, but when an error occurs you'll be able to instantly pinpoint what the error is and where it occurs instantly. Furthermore, there's no instance in this code that can be made DRYer. The errors are just handled. </pre>anaerobic_lifeform: <pre><blockquote> Then you start wrapping errors with context and you realise there's actually nothing to DRY up, you were simply choosing not to handle the error correctly before. </blockquote> The claim that in Go you really handle errors as opposed to other languages, is a fallacy that must die. A kind of No True Scotsman: "errors are not properly handled if you don't write all steps yourself". I understand you may prefer writing all the error handling plumbing by hand, and in some cases I can agree that the result might be preferable to exceptions in terms of readability or performance. But I see no reason to believe the claim that Go code has necessarily better error handling by default, or scales better. Note that with exceptions, all errors are checked, propagated and handled automatically (even errors you did not think about beforehand). The default handler either enters the debugger or aborts the program with a stacktrace, because it has no way of doing better; but at least all errors are guaranteed to be "in your face", sooner or later. In some way, it is true that Go forces to handle all errors, to the extent tat the compiler complains if you don't use intermediate results (except for single-return functions). However, in practice, this does not change much. My experience with checked exceptions or "monadic" error handling is that attempts to make compilation fails until all errors are checked tend to make developers take shortcuts and ignore errors (underscores or empty try/catch blocks) just to get rid of compilation errors during first drafts. And in that case, I tend to side with those developers: better flesh out the logic first; once you commit to a specific solution, then the error path is worth being finalized. <blockquote> I don't know where I was going with this. </blockquote> Same here :-)</pre>JackOhBlades: <pre><blockquote> The claim that in Go you really handle errors as opposed to other languages, is a fallacy that must die. </blockquote> I never claimed this. I don't think anyone has ever claimed that you don't or can't handle errors in other languages. My argument is about how the error value check is not actually simple language boilerplate, and why that is a good thing. The main hypothesis is that when people complain about <code>if err != nil { return err }</code> boilerplate it's often because they're not handling the errors, not because it is actually boilerplate. Following that, when people do handle the errors "properly" in Go they tend to (and this is anecdote) not consider the error handling conventions as boilerplate any longer. <blockquote> I understand you may prefer writing all the error handling plumbing by hand </blockquote> This is where we disagree. It's the belief that the error check is simply "plumbing" that I believe causes one to say Go error handling is "verbose boilerplate", to quote many a person. I don't see how checking an error is, in principle, anymore "plumbing" than checking for an exception. You should treat errors as values and handle them like any other value. Do you consider your business logic to be "plumbing"? If so, then I concede. Nevertheless, there are many reasons why the exception model is poor, and I'd rather not repeat those arguments here. I'm not here to claim that Go does errors better than other languages. I'm here to claim that Go's idioms around error handling are not considered verbose when you "do it properly", with respect to Go's semantics. </pre>MonkeeSage: <pre><blockquote> you were simply choosing not to handle the error correctly before </blockquote> Kind of, but kind of not, but mostly. In a lot of other languages with error/exception types there's a syntactic way of handling them with special keywords like try/catch, which allows them to be handled at specific points in the control flow. Think about a method for opening and reading a file into a buffer, which might raise an error if the file doesn't exist, if there are not sufficient permissions to open it, during the reading of the file into the buffer, or when closing the file. But you can bundle all of those up in a try/catch block when you call the method rather than checking at each step inside the method. This seems good when you just want to catch a generic error from a group of linked operations that should succeed or fail as a unit, and log the failure message from the exception (whichever one it might be). But once you start having to care about different kinds of exceptions and handle them differently, you have to then write a bunch of different exception handlers for the different cases. And now you are handling all of them anyway, they are just far away from where the code that threw them is. And even worse, if code below yours doesn't properly wrap code they are calling in try/catch, an exception might bubble up to your code! And this isn't just theoretical, I've seen this happen in real world code. The openstack network orchestration layer is neutron and is written in python, and I remember a bug from a few versions ago where some low level code, calling out to the OS to bind a port to a bridge, threw an exception, but wasn't caught when the method that threw it was called, so it bubbled all the way up to an exception handler in the quota code (i.e., "your tenant/user can only have 10 ports but you asked for 11"). Took quite a a bit of reading through code and debugging to figure out the actual issue was several layers down, and the over quota log message was just masking the real issue. Java of course has checked exceptions but we're trying to be less verbose and tedious so moving on. Go also does have a <code>recover</code> keyword that can unwind a <code>panic</code> (and can only be used inside a <code>defer</code> block), but panics are highly discouraged from being used as flow control and treated as errors, making a distinction between error conditions and exceptional (panic) conditions. Course there's nothing stopping you from assigning all errors in go to <code>_</code> and ignoring them either. But the convention of handling all errors immediately and explicitly in go does seem to be a really good idea overall, and does force you to actually think about what the errors mean at the time rather than just passing the buck downstream.</pre>JackOhBlades: <pre>I lol'd at the Java comment. <blockquote> But once you start having to care about different kinds of exceptions and handle them differently, you have to then write a bunch of different exception handlers for the different cases. And now you are handling all of them anyway, they are just far away from where the code that threw them is. </blockquote> Exactly. I suppose there's two working hypothesis at play here. <ul> <li>If errors can be handled generally in most cases, then handling them individually is going to be "verbose".</li> <li>If errors can't be handled generally in most cases, then handling them individually is "necessary". </li> </ul> Given that both of these cases occur in software, it's really a question of which case is more common, or harder to deal with. In practice I've found that in most cases you actually want to handle each error uniquely. This is probably what biases me towards Go vs exception based languages. One might question why you can't have both a generic mechanism like a try-catch and and the classic value check in the same language. I'd argue that while there is no technical reason why you can't, in practice many programmers would just treat all errors generally when they shouldn't, giving you all the issues associated with try-catch. Humans are lazy. We do what we know, we stick the familiar. No one would embrace the value check if try-catch was available in the language. In many ways it's really about competing philosophies, not competing syntax. </pre>shovelpost: <pre><blockquote> Is there a better way to write this </blockquote> Yes there is: <pre><code>if err := a(); err != nil { return fmt.Errorf("doing a stuff: %v", err) } if err := b(); err != nil { return fmt.Errorf("doing b stuff: %v", err) } if err := c(); err != nil { return fmt.Errorf("doing c stuff: %v", err) } return nil </code></pre> Also <a href="https://blog.golang.org/errors-are-values">errors are values</a>. You can program with them to make the API more concise: <pre><code>type abc struct { err error } func (t *abc) a() { if t.err != nil { return } t.err = a() } func (t *abc) b() { if t.err != nil { return } t.err = b() } func (t *abc) c() { if t.err != nil { return } t.err = c() } func main() { t := abc{} t.a() t.b() t.c() if t.err != nil { log.Fatal(t.err) } } </code></pre> Check out <a href="https://www.youtube.com/watch?v=1B71SL6Y0kA&feature=youtu.be&t=15s">this talk</a>.</pre>qu33ksilver: <pre>Even better - <pre><code>if err := a(); err != nil { return errors.WithMessage(err, "doing a stuff") } if err := b(); err != nil { return errors.WithMessage(err, "doing b stuff") } if err := c(); err != nil { return errors.WithMessage(err, "doing c stuff") } return nil </code></pre> With the indispensable <code>github.com/pkg/errors</code> package</pre>theephie: <pre>Link for the lazy: <a href="https://github.com/pkg/errors" rel="nofollow">https://github.com/pkg/errors</a> Do you mean <code>errors.Wrap</code>?</pre>qu33ksilver: <pre>Nah, I meant <code>WithMessage</code>. <code>Wrap</code> captures the stack trace too, which is a costly operation. <code>WithMessage</code> is much lighter. Since the OP used <code>fmt.Errorf</code>, I used its closest alternative.</pre>JackOhBlades: <pre>I had no idea about this, thanks for the tip!</pre>shovelpost: <pre>I don't understand why you have to use a dependency of 1.2k+ lines of code just to send a simple message up to the user. If you really care about the cause of the error then instead of a simple message you can send a custom type that includes the cause and which is trivial to do.</pre>SeerUD: <pre>Mainly because a lot of the time there's a <code>fmt.Errorf</code> in the middle somewhere. <code>pkg/errors</code> exists to remove the boilerplate of adding the custom types every time you want to wrap an error, and provide utilities to work with the wrapped errors too. I don't think <code>pkg/errors</code> is perfect though, there are a couple of bits it's missing IMO.</pre>shovelpost: <pre><blockquote> pkg/errors exists to remove the boilerplate of adding the custom types every time you want to wrap an error </blockquote> The way I see it, pkg/errors exist to gather the stack trace. That's the only useful feature. Everything else can be done with 5-6 lines of code vs importing 1.2k+ lines. But who cares because it's just a few extra lines in Gopkg.toml right? <blockquote> and provide utilities to work with the wrapped errors too. </blockquote> That forces the client to use the dependency as well. To be fair, I think pkg/errors is good when it's used in large, complex applications but small applications have no need for it. The absolute worst case is when it's used by libraries. That's simply horrifying.</pre>titpetric: <pre>Actually, depends on how you can handle an error. Generally if you can't handle an error you just return it down the execution stack, until it hits the outer most code which does something like logging. Since by default errors don't have stack traces, it makes is really hard, really impossibly hard to figure out where some error is coming from, especially if you didn't wrap it along it's path, for which you'd probably convert it to string and lose some underlying context which is returned by some packages (encoding/json SyntaxError, database/sql driver dependant error objects,...). At least pkg/errors allows you to Wrap an error without losing it's underlying cause (<code>Cause() error</code>), which you can still type cast or print with <code>%#v</code> with all it's details. The most obvious issue I have with pkg/errors is that <code>errors.New</code> is used to define errors that may be returned in the package, and it does this by using a var. See the <code>os</code> package for <a href="https://godoc.org/os#pkg-variables" rel="nofollow">the ultimate example</a> in the stdlib. The stack trace for such declarations of errors is useless, as it doesn't give you information about where the error was returned from, but only where the instance was made. While I concede that it's overkill having stack traces everywhere, it feels somehow limiting that you can't have them with compiling with some debug flags or something. If we're into this kind of territory, then it's better to end it here. As with all things, if you know how it works, you can use it to your advantage.</pre>shovelpost: <pre>I don't disagree on the usefulness of pkg/errors. But in my opinion it is way overused without strong reason. You do not need stack traces for good error handling.</pre>SeerUD: <pre>I'm not sure I agree really. I think it's functionality could be useful for almost anything unless you only have one layer in your app that returns errors (i.e. an error from a library is always only one error away from being handled for the last time), because you might want to treat different errors differently. Aside from stack traces, how would you provide the rest of the functionality of <code>pkg/errors</code> in 5-6 lines? edit: I actually made an error wrapping library myself for internal use, it clocks in at 1308 lines in 12 files. It handles the error wrapping, stack traces, and a bunch of utilities for extracting errors that happened or checking if a certain type of error is in the stack, and others for printing out the information is different ways (e.g. stack as array, stack as string, etc.). It's not a small library, but it's drastically improved our ability to do things like appropriate logging around errors, and has allowed us to provide much more useful and meaningful context as a result.</pre>shovelpost: <pre><blockquote> Aside from stack traces, how would you provide the rest of the functionality of pkg/errors in 5-6 lines? </blockquote> I was thinking about the aforementioned example. To create a custom error that holds the cause, you need around 5-6 lines. I am not sure what else you need. If we are to also count a function that checks the error maybe we go up to 10 lines or something.</pre>chownplus: <pre>The vast majority of that "1.2k+ lines" of code in the dependency are tests. Once you disregard that, the dependency is a very manageable 416 lines of which over 20% amount to comments. </pre>shovelpost: <pre>Even if we omit the loc, in my opinion it is yet another dependency that the majority is blindingly importing in their projects without good reason. The worst offenders are libraries.</pre>weberc2: <pre>Why does the package loc matter? Only the lines that are used will be compiled into the final binary. Anyway, it's often better than "error doing foo stuff: error doing bar stuff: error doing baz stuff: ...". This is especially true if you log as JSON and use a tool that understands JSON to query your logs. I get the desire to omit the dependency, but it's truly useful and the std lib offers no analog.</pre>shovelpost: <pre><blockquote> but it's truly useful and the std lib offers no analog. </blockquote> If we are talking about the stack traces feature then yes I agree. But without that the standard library easily delivers. I am don't arguing about the usefulness of pkg/errors.</pre>theephie: <pre>Interesting take on solving the issue. I don't like the readability of this though, it's not obvious there are conditions inside: <blockquote> <pre><code>t.a() t.b() t.c() </code></pre> </blockquote></pre>anaerobic_lifeform: <pre><code>errOR</code> is an OR operation on errors. The name should be changed, probably. <pre><code>func errOR(errors ...error) (error) { for _, err := range errors { if err != nil { return err } } return nil } </code></pre> Call site: <pre><code>return errOR(a(), b(), c()); </code></pre></pre>theephie: <pre>I find this hilarious yet readable!</pre>mcouturier: <pre>The problem is that a, b and c often returns values along with the error</pre>SeerUD: <pre>You could wrap the functions and store the results in other variables defined outside of the call... something like: <pre><code>var ar, br, cr int return errOR( func() { ar, err := a() return err }, func() { br, err := b() return err }, func() { cr, err := c() return err }, ) </code></pre> ...please don't do this.</pre>gargamelus: <pre>b and c may also have side effects that are not wanted if a errs out.</pre>weberc2: <pre>This could be solved by making the function accept func() error and then calling it with errOR(a, b, c). Of course, supporting functions with arguments or return values means jumping through closure hoops, which defeats the purpose.</pre>mcouturier: <pre>Stop trying to workaround, this is really bad from the start!</pre>weberc2: <pre>Calm down, I agree. I was musing for amusement. :)</pre>asian_driver_wee: <pre>Unpopular opinion, but I find just panicing whenever I don't plan to handle an error makes the app code much cleaner and easier to deal with. I don't do this when I write libs though.</pre>titpetric: <pre>I wrote about some useful patterns before <a href="https://scene-si.org/2017/11/13/error-handling-in-go/" rel="nofollow">in this article</a>. You could ctrl+f "flow" in there. Alternatively, you could just look at <a href="https://play.golang.org/p/AStZiZ_-Ml" rel="nofollow">this playground sample</a>, or perhaps the <a href="https://godoc.org/golang.org/x/sync/errgroup" rel="nofollow">x/sync/errgroup package</a>.</pre>skidooer: <pre><pre><code>for _, fn := range []func() error{a, b, c} { if err := fn(); err != nil { return err } } return nil </code></pre></pre>

用户登录

今日阅读排行

一周阅读排行

最新主题