12 ms·
How we centralized and structured error handling in Golang
- omeid2 2y agoThis is a cry for sum types.
- nikolayasdf123 2y ago> centralized system [... for errors] dont think this will scale. errors are part of API. (especially Go mantra errors are values https://go.dev/blog/errors-are-values https://go.dev/blog/errors-are-values it is ever more prominent). and each API is responsibility of a service so unless you deal with infrastructure or standards/protocols layer (say you define what HTTP 500 means or common pattern for URL paths in your API), then better not couple all services. those standards are very minimal and primitive that works for everything, which is opposite what you doing here aggregating all the specifics into single place
- vrosas 2y agoTrying to shoehorn code errors into HTTP errors is a prime example of conflating two very different things because sometimes they look similar. Let different things be different, I like to say. You either let your HTTP handlers do their own error-to-http-code management or you end up with a massive switch statement trying to map them all, or whatever monstrosity this approach is. Also the entire problem of the OP would go away if they just implemented opentelemetry tracing to their logs.
- pluto_modadic 2y agoah, yes, completely separate. HTTP code: 200 ok Body: {"error":"internal server error"}
- deleted 2y ago[deleted]
- bb88 2y agoMy favorite example of this was renaming a 500 error due to an unhandled exception to a 400 error to make it look like it was the error of the caller. Management was also possibly tracking 500 errors too, so the 400 could also have been gaming the system. In some mental models, though, it did make sense. Particularly the one that went, "Well, we never would have errored, if you never called us!"
- xmprt 2y agoIt's somewhat fair though. If there's a case that would cause errors for the system and it's a case that you're not supposed to handle, then a 400 error sounds perfect for that case. For example, if you have a service and it panics/returns 500 when you pass in an empty user id, then you could instead return a 400 before you hit the panic and all is good.
- bb88 2y agoNormally you should attempt to find all the corner cases and present the errors to the user -- before processing the request. If you can't do this, it's time to rethink how your api works. A good api is simple to use and simple to write. It also simplifies your business logic in that all the possible user defined idiocies are caught before your business logic actually processes the request. Some frameworks do this better than others. And rather than documentation, I tend to prefer comprehensive error messages.
- fmbb 2y ago> Normally you should attempt to find all the corner cases and present the errors to the user -- before processing the request. That is what they are suggesting. You check the request and return 400 if it’s bad.
- bb88 2y agoOne example of a 500 error is a null pointer error. Was it a bad request or a logic error? One is your problem the other is not. Just returning a 400 hides that issue. Validating the payload before processing it simplifies the issue for everyone involved. A 500 error should be your problem with a stack trace in the log. A 400 error should provide enough description to tell the user it's theirs and how to fix it. Just marking recoding a 500 to a 400 because of a null pointer error would get noticed on a code review and marked up on a code review.
- saghm 2y agoHonestly, my controversial take is that for APIs, it would be cleaner to not use any HTTP status codes other than 200 and have all of the semantics in the body of the response. I'm sure someone smarter than me will jump in and explain why this wouldn't work in practice, but it just feels like application semantics are leaking from a much more natural location in the body of the response. I feel similarly about HTTP request methods other than POST in APIs; between the endpoint route and the body, there should be more than enough room to express the difference between POST, PATCH, and DELETE without needing them to be encoded as separate HTTP methods.
- evnix 2y agoThis is the way to go, pretty much solves, 404 resource not found or route not found. But you will get laughed at by so called architectural dogmatists. Remember we aren't really doing REST, it's just RPC and let's call it that. Shoehorning http protocols error codes as application error codes, drinking the cool aid and calling it best practice is beyond bizzare.
- marcus_holmes 2y agoAgree. "200 - successfully failed to do the thing" is valid and useful. 500 is "failed to do anything at all"
- tayo42 2y agoYour kind of describing things like thrift and other rpc servers?
- saghm 2y agoPossibly. I'm not sure why it should require switching to an entirely different protocol though; my point is that making an API that only uses POST and always returns 200 is something that already works in HTTP though, and I have trouble understanding why that isn't enough for pretty much everything.
- 2y ago
- maccard 2y agoThink about what the client code looks like to handle this and the alternative, particularly if you’re implementing an sdk and the api is an implementation detail. I’m not saying I would choose this path, but it certainly reduces the amount of code on both sides that you have to write.
- ranger207 2y agoIf HTTP is your API's transport layer, then HTTP errors should be related to problems with the transport layer and not to API itself. Is the internal server error caused by a bad HTTP request or a bad API request?
- rollulus 2y agoThe error to code in the http handler is the true path. It’s the only place where the context and knowledge is about semantics. In one endpoint if something is not found it can be a proper 404, if its existence is truly optional. In another endpoint the absence might very well qualify as a 500.
- bvrmn 2y ago404 is quite an ominous thing. 404 because route is not found or entity not found. God bless your monitoring.
- rounce 2y ago422 is frequently used for this case despite being part of the WebDAV extensions.
- Groxx 2y ago100% this. Deciding on end user error handling in a low level is making assumptions that cannot be known at the low level. The caller decides how something is going to be handled and presented, not the callee, or you inevitably miss them in important places and silently miscategorize stuff. Far better to have that scenario lead to a 500 (unmapped error, unknown problem) so it can be fixed.
- bedobi 2y agoThe sane thing to do is to let lower layer functions return Either<Error, Foo> then, in the HTTP layer, your endpoint code just looks like return fooService .getFoo( fooId ) //Either<Error, Foo> .fold({ //left (error) case when (it) { is GetFooErrors.NotAuthenticated -> { Response.status(401).build() } is GetFooErrors.InalidFooId -> { Response.status(400).build() } is GetFooErrors.Other -> { Response.status(500).build() } } }, { //right case Response.ok(it) }) the benefits of this are hard to overstate. * Errors are clearly enumerated in a single place. * Errors are clearly separated from but connected to HTTP, in the appropriate layer. (=the HTTP layer) Developers can tell from a glance at the resource method what the endpoint will return for each outcome, in context. * Errors are guaranteed to be exhaustively mapped because Kotlin enforces that sealed classes are exhaustively mapped at compile time. So a 500 resulting from forgetting to catch a ReusedPasswordException is impossible, and if new errors are added without being mapped to HTTP responses, the compiler will let us know. It beats exceptions, it beats Go's shitty error handling.
- rednafi 2y agoGo’s error handling is still cumbersome and lacking. I love writing Go but I don’t want to ever adopt anything like. It’s bending over backwards to achieve something sum types provide and this pattern is a mess.
- solumunus 2y agoI would be all over Go with a better type system or exceptions.
- deleted 2y ago[deleted]
- gf000 2y agoC#, OCaml, Java, Scala, Kotlin all fulfill these requirements, while targeting the same niche.
- solumunus 2y agoYes there are indeed lots of languages in existence.
- LinXitoW 2y agoGo has insanely good tooling and very fast single binary compiling. While all these languages (afaik) can reach similar levels of functionality (GraalVM e.g.), it's more work. As much as I hate the language Go, I can't deny how braindead simple it is to just make a tool with it. I don't need to choose a build tool, or a runtime version, there's a library for everything and most developers with more than a room temp IQ can immediately start working on it. The only other language that currently comes close is Rust. If only they had stuck to using a GC, I'd be in heaven.
- Capricorn2481 2y agoIs this that different from languages like Dotnet? I think one line in your config can make it Aot compiled.
- winstonp 2y agoI agree Go error handling is unoptimal, but this is simply not the right approach. This essentially turns error handling into a whole other language, almost like how Ginkgo is a separate language for handling tests.
- wruza 2y agoAnd most languages are lacking this useful error language. You can’t speak if you have no language, so having it must be a good thing. The only questionable thing here is that this framework is not a part of the main language still, which means near zero adoption. But that train has sailed.
- jaza 2y agoFeels like OP is basically implementing exceptions and exception handling at the application level. If this is what you want, then why not just switch to one of the many other languages that has exceptions built in at the language level?
- KingOfCoders 2y agoI think they use too many sentinel errors [0] I have been doing Java for two decades, and I thought you need to handle individual errors by type. Using Go, I've learned from the code I write, 90%+ of errors I don't need to handle individually, or I can't do anything except bubble an error up. There is the rare case (10%) when a file does not exist, and I try to read an alternative one and I don't bubble up an error. For customer support I also found it much easier, instead of an error number, print a UUID that customers can give to support, and that UUID (Request ID) then can be found in the logs to find out what happened by developers. [0]:https://dave.cheney.net/2016/04/27/dont-just-check-errors-handle-them-gracefully https://dave.cheney.net/2016/04/27/dont-just-check-errors-ha...
- RVuRnvbM2e 2y agoThis was exactly my train of thought. I even went looking for Dave's blog post about it before I saw your comment. :D
- deleted 2y ago[deleted]
- gf000 2y ago> Using Go, I've learned from the code I write, 90%+ of errors I don't need to handle individually, or I can't do anything except bubble an error up So... exceptions are better, because they would do the correct thing by default in the majority of cases?
- lordofgibbons 2y agoUnless you forget to catch the right type of exception. Then all hell breaks loose.
- revskill 2y agoToo much writing and lack of diagramming is a sign of digging through the rabitt hole.
- rollulus 2y agoAh, a God error package that has all seeing knowledge of the domain around it. What a monstrosity.
- bilbo-b-baggins 2y agoBro got dragged so hard in the comments he took his site down. Oof.
- jatins 2y agoI mean their intentions are good but if I worked at a place that made me use that error package I'd not have a good time In general with golang, if something is not idiomatic Go then don't try too hard to fit constructs from other languages into it. Even the use of lodash like packages feels awkward in Go
- asabla 2y agomore like hug of death from HN users. Since the site is back up and working again
- tzahifadida 2y agoI arrived to a similar conclusion. I come from Java and in Java you have exceptions with TryCatch clauses and declaring them in function signatures. It works fairly well but very difficult and not idiomatic to Golang. Therefor, I created a simple rule. If you do not know what this error means to the user yet then let it stay a fmt.errorf("xx:%w",err). If you do, wrap it in your own custom ServerError struct and return that type from now on. Do not change the meaning of ServerError even if you wrap the Error with another ServerError.
- RVuRnvbM2e 2y agoIt is telling that you come from Java with this opinion. OP's approach is certainly not idiomatic Go.
- wruza 2y agoIdiomatic here means no idiom suggested really. So yeah, non-idiomatic.
- Animats 2y agoIs this just someone's proposal, or a formal addition to Go, or what? "All errors must implement the Error interface." That's a step forward. Rust really has the same error handling as Go - return an error status. But the syntax is cleaner. Rust thrashed around with errors at first. Then things sort of settled down. At this point, everybody uses Result<UsefulValue, Error>, but "Error" is just a trait that doesn't require much information. And "?" for propagating errors upwards is a huge convenience. It's probably too late to retrofit "Result" and "?" into Go libraries, although they'd fit the language.
- gf000 2y ago> Rust really has the same error handling as Go Not at all. Rust has proper sum types, that it can return just like anything else in the language, while Go has a special cased error return slot (one may be tempted to call it an ugly hack), and it can return a value on both, which it does in some standard library calls.
- margalabargala 2y agoNot at all. Go has an error type, and Go functions have the ability to return zero, one, two, or more items, ordered however the developer likes. An error may be among those, as desired, and populated as desired. Some software also writes to both STDOUT and STDERR.
- gf000 2y agoI know, special cased may have been better worded as "just a convention". My point is, this is not much different than using a thread-local variable, like errno, and adds useless confusion - your return values represent n*m values, while there is only n+m case with proper error semantics. Re STDERR: but shells don't decide whether a program execution failed on having written to STDERR, but by the returned singular error code.
- margalabargala 2y ago
- wruza 2y agoWhen I thought about errors/exceptions, I basically came to the same conclusion. To reiterate or add to tfa: standard formulations, expected vs. happened, reasonable context visible in logs, error trees, automatic http/etc codes, tidy client messages in prod, reasonable distinction between: unexpected, semi-normal, programming error, likely fatal. Not sure why most (all?) programming languages have such poor support for errors. Coding may feel like 2024, but error handling like 1980. Anyone with 2-5 years of any programming experience (in where errors do happen and they choose to handle them) will come to similar ideas. Also the fact that try {} and catch/finally {} are always three different scopes is just idiotic. It should be try {catch{} finally{}}, what in the cargo cult that {}{}{} is? Everyone copies it blindly from grammar to grammar.
- ikiris 2y agoThe fact that this code also has gorm in it in one of the examples is neither supportive of the proposal’s fit for the language, nor really surprising.
- rollulus 2y agoThis approach is so bad, I don't even know where to start. But it's all symptoms of their, sorry, incompetence. Take the loadCredentials example on top. If os.ReadFile cannot find the file, it returns an error with string representation: "open cred.json: no such file or directory". This comes straight from the std lib as it is, a great error. What does the errors.Is(err, os.ErrNotExist) do: prepend "file not found" to it, rendering: "file not found: open cred.json: no such file or directory". So this adds exactly nothing. The next if will prepend "failed to read file" to it, again, adding nothing as well. The two errors checks should be replaced by one if statement, optionally wrapping it with a context string but I cannot think of any use. Then the next step, error handling of verifyCredentials. I can only guess what it does, but assume that it returns an "username 'foo bar' cannot contain spaces" error. Does prepending "invalid credentials" help anything? Nope, so the whole if can be removed as well. No surprise your errors get clunky if you make them clunky. I have more pressing things to do than dissect this article line by line, but let me suffice that I feel sorry for newcomers to the language that an article like this is so high on HN. Back in the days there was just Dave Cheney's material to read [1], and it was excellent. It's unfortunately outdated in certain regards (e.g. with new Is/As functionality in the errors package for inspection and the %w formatting directive in fmt.Errorf) but it's still an excellent article. [1]: https://dave.cheney.net/2016/04/27/dont-just-check-errors-handle-them-gracefully https://dave.cheney.net/2016/04/27/dont-just-check-errors-ha...
- franticgecko3 2y agoI'm worried readers of this article will be horrified and believe this kind of DIY error handling is necessary in Go. The author has attempted to fix their unidiomatic error handling with an even more unidiomatic error framework. New Go users: most of the time returning an error without checking its value or adding extra context is the right thing to do
- vultour 2y agoFrom my experience this is not the case. If you error out 7 functions deep and only return the original error there's no chance you're figuring out where it happened. Adding context on several levels is basically a simplified stack trace which lets you quickly find the source of the error.
- ThePhysicist 2y agoI think that's overkill, most of the time I just bubble errors up and I have very few cases where the error handling depends on the type of error. I guess it's because I don't use errors for things that are recoverable and try to fix them instead inside the given function. An example given here in the thread is reading from a file and if it doesn't work try a backup. Rather than having a function that reads from a file and returns a bunch of different errors I'd just make one with a list argument and then handle the I/O errors inside, and return an "unrecoverable" error otherwise. For adding context, %w is good enough I find, though as I said I only very sparingly use errors.Is(...). Go isn't a language that's designed around rich error or exception types, and I don't think you should use it like that.
- Yoric 2y agoWell, yes, if you're just using errors as error messages, you only need strings and %w. That's usually good enough if you're writing an application. However, if you're writing a library, chances are that your users want to catch the errors, find out whether the call failed because, say, the remote API is down or because the password is wrong. Or if you're writing an API, you probably want to return different error codes. If your errors are bubbling, you'll need to somehow `errors.Is`/`errors.As` somewhere.
- eptcyka 2y agoYea, but like, when making an HTTP request, a timeout is significantly different from a failure to open a socket from a failure to resolve the hostname from a 429 error. And often it is up to the caller to decide how to handle those situations.
- sethammons 2y agoThe concepts aren't wrong (structured logs from structured errors), but I find this code to be very un-go-like and there are obvious signs of trying to write java in go (iFace, structs with one property "because everything needs to be contained in an object", and others). Return "error" and not a custom type "mypkg.Error" - you run into more nil interface pointer problems and you are breaking an idiom. Let me provide a counter example for helping create structured logs from structured errors that I wrote up that is much more idiomatic if not more narrowly focused: https://github.com/sethgrid/kverr https://github.com/sethgrid/kverr As in the article, if you want to attach "username: foo", this package lets you return kverr.New(err, "username", foo, ...), and then extract a slice or map later for logging like logger.WithArgs(YoinkArgs(err)...).
- Cloudef 2y agoPosts like these remind me how go really has nothing going for it apart from goroutines and channels. It's awkward mix of low level and high level with C like influence, which is weird considering it's a GC language.
- binary132 2y agoI have been seeing this pattern repeated over and over since I started using Go in 2014 where people think they should be “building my favorite missing feature” — whether that’s futures, generics, structural processes, OTP, version managers, package managers, or now apparently exceptions. I always get the sense that the authors think they’ve done something cool and helpful when in the first place if they had simply put more effort into comprehending the simple “Go way” it wouldn’t have been necessary at all, and the needed functionality would have fallen out of the design.
- jdbxhdd 2y agoYou realize that have of the features you are counting are now in Go while missing in the beginning exactly because people were missing them and Go simply did not offer a sane way to work around the missing features? I'm also quite sure that Go will provide a more sane way to handle errors in the not so far future, since it's continuously at the top of people's complaints
- binary132 2y agoyour comment exemplifies the mentality, yes, and unfortunately it has now been adopted by project leadership, so I’m sure you are quite right that more “missing features” will get baked into the language soon :)
- int_19h 2y agoIt's far better to have those features well-designed and baked into the language once, then to have them constantly poorly redesigned and baked into every other Go app.
- binary132 2y agonobody would ever use this argument for the design of C. It’s good for C to stay lean and simple while communities using C (please let’s not with this imaginary monolithic “The Community”) are free to try things and offer competing solutions that others are free to ignore. kitchen sink languages are bad. Justifying them with “well the community is bad, so we need the bad thing to be mainlined” is maybe worse
- mukunda_johnson 2y agoAdding error checks everywhere when you don't care about them is one of the ugliest things about Go. What I do is have a utility package that lets me panic on most errors, so I can recover in a generalized handler. x, err := doathing() Catch(err, "didn't do the thing") The majority of error handling is "the operation failed, so cancel the request." Sure there are places where the error matters and you can divert course, but that is far from the majority of cases.
- kbolino 2y agoI don't agree, but having said that, this feels like an entirely predictable/justifiable perspective to hold, given the terrible design of net/http in the standard library. Of course it feels easier to just panic, it's not like you can return an error from a handler. There is so much compatibility baggage from Go 1.0 in that package, that doing the right thing (contexts, errors, etc.) is so much harder than it should be, and most people end up doing the wrong thing because it's more ergonomic.
- mukunda_johnson 2y agoI usually use Echo which does have an error to return from handlers, but I don't think it's necessarily the wrong thing unless you're writing a library. I used to avoid panics with the same mindset that they aren't supposed to be used like exceptions, but I've found that panics are a clean way to handle a bulk of error cases that are "log and retreat", centralizing the process with some syntactic sugar to not have to check err != nil everywhere. More of my thoughts here if any are curious: https://blog.mukunda.com/cat/2022/dont-be-afraid-to-panic.txt https://blog.mukunda.com/cat/2022/dont-be-afraid-to-panic.tx... I think one thing that could help if the codebase wants to avoid regular panics is more syntactic sugar to help error bubbling, like Rust has.
- samiv 2y agoThe most crucial thing that I've seen over the years is that most developers are simply afraid of bringing the application down on bugs. They conflate error handling with writing code for bugs and this leads to proliferation of issues and second/third/etc degree issues where the code fails because it already encountered a BUG but the execution was left to continue. What do I mean in practice? Practical example: I program mostly in C and C++ and I often see code like this if (some_pointer) { ... } and the context of the code is such that some_pointer being a NULL pointer is in fact not allowed and is thus a BUG. The right thing to do would be to ABORT the process execution immediately but instead the programmer turned this it into a logical condition. (probably because they were taught to check their pointers). This has the side effect that: - The pre-condition that some_pointer may not be null is now lost. Reading the code it looks like this condition IS allowed. - The code is allowed to continue after it has logically bugged out. Your 1+1 = 2 premise no longer holds. This will lead to second order bugs later on when the BUG let program to continue execution in buggy condition. False reporting will likely happen. The better way to write this code is: ASSERT(some_pointer); Where ASSERT is a unconditional check that will always (regardless of your build config) abort your process gracefully and produce a) stack trace b) core dump file. My advice is: If your environment is such that you can immediately abort your process when you hit a BUG you do so. In the long run this will help with post-mortem diagnosis and fixing of bugs and will result in more robust and better quality code base.
- bluepizza 2y agoI think the issue is that bringing the application down might mean cutting short concurrent ongoing requests, especially requests that will result in data mutation of some sort. Otherwise, some situations simply don't warrant a full shutdown, and it might be okay to run the application in degraded mode.
- samiv 2y ago"I think the issue is that bringing the application down might mean cutting short concurrent ongoing requests, especially requests that will result in data mutation of some sort." Yes but what is worse is silently corrupting the data or the state because of running in buggy state.
- gregwebs 2y agoThere's a lot of comments here that seem overly critical. The author came up with solutions to extend Go's errors to meet their needs and shared that with the world- thank you. I have been solving all the same problems and providing libraries that allow for more flexibility so that users can come up with approaches that best meet their needs. I am finally polishing the libraries and starting to write about them: https://blog.gregweber.info/blog/go-errors-library/ https://blog.gregweber.info/blog/go-errors-library/ (errors with stack traces and metadata) https://github.com/gregwebs/errcode https://github.com/gregwebs/errcode (adding codes to errors- working on improving docs and writing about this now).
- AceCream 2y agotype xError struct { msg message, stack: callers(), } is this legit in go?
- ptman 2y agoI'm still just growing organically. Constant errors are useful for sentinel values https://dave.cheney.net/2016/04/07/constant-errors https://dave.cheney.net/2016/04/07/constant-errors . RFC9457 https://datatracker.ietf.org/doc/html/rfc9457 https://datatracker.ietf.org/doc/html/rfc9457 is useful for REST (JSON) APIs. pkg/errors is nice for stack traces. I define error structs with field to pass structured data instead of trying to parse strings.