How we centralized and structured error handling in Golang

https://olivernguyen.io/w/namespace.error/

by thunderbong • 2 years ago

171 257 2 years ago

olivernguyen.io

omeid2• 2 years ago

This is a cry for sum types.

nikolayasdf123• 2 years 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 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• 2 years ago

Trying 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• 2 years ago

ah, yes, completely separate.

HTTP code: 200 ok

Body: {"error":"internal server error"}

bb883• 2 years ago

My 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!"

saghm5• 2 years ago

Honestly, 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.

maccard• 2 years ago

Think 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• 2 years ago

If 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?

rollulus4• 2 years ago

The 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.

bvrmn6• 2 years ago

404 is quite an ominous thing. 404 because route is not found or entity not found. God bless your monitoring.

Groxx• 2 years ago

100% 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• 2 years ago

The 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• 2 years ago

Go’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.

solumunus3• 2 years ago

I would be all over Go with a better type system or exceptions.

gf00016• 2 years ago

C#, OCaml, Java, Scala, Kotlin all fulfill these requirements, while targeting the same niche.

thiht3• 2 years ago

If Go ever adds exceptions[1] as an error handling mechanism, I'm out. Value errors are far superior to exceptions, even in their current state in Go.

[1]: assuming panics are not an error handling mechanism but a recovery mechanism

KingOfCoders6• 2 years ago

I thought so too, after years with Scala and Rust. Now I think (X, error) is fine, indeed I think it is great for it's simplicity. I might want to have a safe assignment

   // x() (X,error)
   x != x() 
   // x is X
   // return on error

But the urge is not very high.

dontlaugh• 2 years ago

The problem is indeed composition. How do I chain 3 calls that short-circuit on the first error? In Go that's verbose in the extreme. With exceptions it's easy to miss an error. Sum type errors have neither problem.

KingOfCoders6• 2 years ago

The problem here is that if your style is not to return the error, but wrap it, you need a way to wrap it.

You could use special syntax

   x != x() ~ "x failed"

that would wrap the error and return it,

   x, err := x() 
   if err != nil {
      return fmt.Errorf("x failed: %w", err)
   }

is more verbose but less so compared to the "just return error" case.

If you want to get rid of that, you need to have some auto conversion magic. This the way Rust does it, but it adds magic, you can't see the error handling, Rust needs (slow compiling) macros for this etc.

winstonp• 2 years ago

I 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.

wruza3• 2 years ago

And 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• 2 years ago

Feels 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?

KingOfCoders6• 2 years ago

I 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-ha...

RVuRnvbM2e2• 2 years ago

This was exactly my train of thought. I even went looking for Dave's blog post about it before I saw your comment. :D

gf00016• 2 years 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?

oefrha• 2 years ago

No, TFA is mostly about making errors consistent in a large application, while exception (vs error as standard return value) is largely about easier bubbling, which is one thing TFA hardly talked about (maybe I missed it, I only skimmed the article). In fact it spends a lot of energy on wrapping which is the opposite of automatic bubbling provided by exceptions by default. Throwing random, inconsistent module/package/whatever-specific exceptions from everywhere causes most of the same problems described in TFA.

I feel like all the canned comments saying TFA is about implementing exceptions / ADT result type are from people who didn’t read the article and just want to repeat all the cliche on the topic (for easy karma? No idea what’s the point).

zeroxfe• 2 years ago

That's not how I read it. It's more about having a consistent approach to managing error types in large code bases. This is a common problem with exception-based languages too.

ori_b• 2 years ago

How so? This is about how errors are defined, not how they're propagated through the application. Feels like you didn't actually read what was being done by the OP.

retrodaredevil• 2 years ago

Exceptions have a hierarchical nature to them in most languages, or at least have some sort of identity to them. Your correct that the author doesn't try to change the way errors are propagated, but you can see similarities between what the author is creating themselves, and what already exists in languages with exceptions.

revskill• 2 years ago

Too much writing and lack of diagramming is a sign of digging through the rabitt hole.

rollulus4• 2 years ago

Ah, a God error package that has all seeing knowledge of the domain around it. What a monstrosity.

mrkeen• 2 years ago

It's not the worst idea for an organisation to centralise stuff that needs to be centralised.

Like defining protobuf schemas, it's no good if each team does its own thing.

joesb3• 2 years ago

Either the package does or the developers do. And only one of them has compilation checks.

bilbo-b-baggins2• 2 years ago

Bro got dragged so hard in the comments he took his site down. Oof.

jatins• 2 years ago

I 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• 2 years ago

more like hug of death from HN users. Since the site is back up and working again

tzahifadida• 2 years ago

I 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.

RVuRnvbM2e2• 2 years ago

It is telling that you come from Java with this opinion. OP's approach is certainly not idiomatic Go.

wruza3• 2 years ago

Idiomatic here means no idiom suggested really. So yeah, non-idiomatic.

Animats• 2 years ago

Is 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.

gf00016• 2 years 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.

margalabargala2• 2 years ago

Not 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.

arp242• 2 years ago

One of the issues in Go is that if all you ever do if "if err != nil { return err }", you will quickly run in to problems because you will have errors like "open foo: no such file or directory" or "sql: no rows in result set" without a clue where that error came from. Sometimes that's obvious, often it's not.

I'm not sure how Rust handles that? But it's more than just "propagate errors", but more like "propagate errors with the appropriate context for this specific error".

bbatha2• 2 years ago

Rust uses the `?` operator to convert between error types which allows for users and libraries to hook in to the error before its returned.

There are a number of helper libraries that provide an extended type erased error type to attach a real stack trace to the error, such as `anyhow`. These helper libraries also provide ways to attach extra metadata to the error so you can do things like `returns_a_result().context("couldn't do it")?` so you can quickly annotate the error. The standard library is support for this through a `context.Value` like api on the Error trait. The std lib `Error` trait also has functions for find the cause of the error and traverse a collected chain of errors, very similar to go's `errors.Cause` api.

Rust also has a number of libraries for making specific error types like `thiserror` which can help generate error enums with the implementations required to carry backtraces, context and causes.

wruza3• 2 years ago

When 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• 2 years ago

The 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.

rollulus4• 2 years ago

This 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-ha...

franticgecko3• 2 years ago

I'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• 2 years ago

From 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.

mariusor4• 2 years ago

Do you maybe have a constructive advice for people that need to return errors that demand different behaviour from the calling code?

I gave an example higher in the thread: if searching for the entity that owns the creds.json files fails, we want to return a 404 HTTP error, but if creds.json itself is missing, we want a 401 HTTP error. What would be the idiomatic way of achieving this in your opinion?

mattgreenrocks• 2 years ago

New Go users: most of the time returning an error without checking its value or adding extra context is the right thing to do

Thank you.

Feels like Go is having its Java moment: lots of people started using it, so questions of practice arise despite the language aiming at simplicity, leading to the proliferation of questionable advice by people who can't recognize it as such. The next phase of this is the belief that the std library is somehow inadequate even for tiny prototypes because people have it beaten over their heads that "everybody" uses SuperUltraLogger now, so it becomes orthodox to pull that dependency in without questioning it.

After a bunch of iterations of this cycle, you're now far away from simplicity the language was meant to create. And the users created this situation.

vl• 2 years ago

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.

It’s a terrible error. It’s not structured, so you can’t aggregate it effectively in logs, on top of that it leaks potential secret, so you can’t return it from RPC handler.

rollulus4• 2 years ago

The string representation is obviously not structured, because it's a string representation and strings are scalars. The typed representation is structured, which you can put into your structured logs as you'd like, omitting sensitive information where needed.

mariusor4• 2 years ago

As someone that ended up implementing something very similar to TFA, I'd like to ask in which way can you pass errors from 3 layers deep in your stack to the top layer and maintain context?

Ie, when I can't find cred.json I want to return a 401 error, but when I can't find the entity cred.json is supposed to be owned by I want to return 404. How can one "not incompetent" Go developer solve this and distinguish between the two errors?

tetha3• 2 years ago

No surprise your errors get clunky if you make them clunky.

From a user perspective, good errors in go make me think or Perls croak/carp. Croak and carp gave you a stacktrace of your error, but it cut out all the module-internal calls and left you with the function calls across module boundaries. Very useful - enough so that Java discovered it again later on.

Personally, I wouldn't wrap the errors in loadCredentials at all. I'd just wrap the result of this method into an fmt.Errorf("failed to load credentials: %w"). This way the user knows the context the error happened in, and then we have to cross our fingers the error returned by this is good enough.

But something like "application startup failed: failed to load credentials: open cred.json: no such file or directory" is a very nice error message from an application. Just enough context to know what's going on, but no 1200 line stacktrace to sift through.

ThePhysicist• 2 years ago

I 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.

Yoric4• 2 years ago

Well, 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• 2 years ago

Yea, 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.

sethammons4• 2 years ago

The 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

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• 2 years ago

Posts 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.

binary1324• 2 years ago

I 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.

jdbxhdd2• 2 years ago

You 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

binary1324• 2 years ago

your 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 :)

mukunda_johnson2• 2 years ago

Adding 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.

kbolino2• 2 years ago

I 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_johnson2• 2 years ago

I 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.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.

samiv8• 2 years ago

The 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• 2 years 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.

Otherwise, some situations simply don't warrant a full shutdown, and it might be okay to run the application in degraded mode.

samiv8• 2 years 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.

ramses0• 2 years ago

"MIT v. Berkeley - Worse is Better" => https://blog.codinghorror.com/worse-is-better/

"Fail Fast / Let it Crash" => https://erlang.org/pipermail/erlang-questions/2003-March/007...

...you're in good company. :-)

jayd164• 2 years ago

Like everything in life, it depends.

If this is some inconsequential part of the codebase it might be better to limp on then to completely stop anyone, user or fellow dev, from running the app at all.

Said another way, graceful degradation is a thing.

samiv8• 2 years ago

How do you gracefully degrade when your program is in a buggy state and you no longer know what data is valid, what is garbage and what conditions hold ?

If I told you to write a function that takes a chunk of customer JSON data but I told you that the data was produced / processed by some code that is buggy and it might have corrupted the data and your job is to write a function that works on that data how would you do it?

Now your answer is likely to be "just check sum it", but what if i told you that the functions that compute the check sums sometimes go off rails in buggy branches and produce incorrect checksums.

Then what?

In a sane world your software is always well defined state. This means buggy conditions cannot be let to execute. If you don't honor this you have no chance of correct program.

gf00016• 2 years ago

I think this is precisely why exceptions model particularly well - well - exceptional situations.

They let you install barriers, and you can safely fail up until that point, disallowing the program from entering cursed states, all the while a user can be returned a readable error message.

In fact, I would be interested in more research into transactions/transactional memory.

texuf• 2 years ago

If you're validating parameters that originate from your program (messages, user input, events, etc), ASSERT and ASSERT often. If you're handling parameters that originate from somewhere else (response from server, request from client, loading a file, etc) - you model every possible version of the data and handle all valid and invalid states.

Why? When you or your coworkers are adding code, the stricter you make your code, the fewer permutations you have to test, the fewer bugs you will have. But, you can't enforce an invariant on a data source that you don't control.

samiv8• 2 years ago

Yes of course the key here is to understand the difference between BUGS and logical (error) conditions.

If I write an image processing application failing to process an image .png when:

  - user doesn't permission to the file
  - file is actually not a file
  - file is actually not an image
  - file contains a corrupt image
  etc.

are all logical conditions that the application needs to be able to handle.

The difference is that from the software correctness perspective none of these are errors. In the software they're just logical conditions and they are only errors to the USER.

BUGS are errors in the software.

(People often get confused because the term "error" without more context doesn't adequately distinguish between an error condition experienced by the user when using the software and errors in the program itself.)

keybored• 2 years ago

But, you can't enforce an invariant on a data source that you don't control.

This is obvious.

kstrauser4• 2 years ago

I largely agree. If it came to pass that the precondition fails, there's a bug somewhere and this code just hides it. At the very least, that should go to an error log that someone actually sees.

I'm writing a Rust project right now where I deliberately put almost no error handling in the core of the code apart from the bits accepting user input. In Rust speak, I use .unwrap() all over the place when fetching a mandatory row from the DB or loading config files or opening a network connection to listen on or writing to stdout. If any of those things fail, there's not a thing I can plausibly do to recover from it in this context. I suppose I could write code like

  if let Ok(cfg) = load_config() {
    println!("Loaded the config without failing!");
    Ok(cfg);
  }
  else {
    eprintln!("Oh no! Couldn't load the config file!";
    Err("Couldn't load the config file");
  }

and make the program exit if it returns an error, but that's just adding noise around:

  return load_config().unwrap();

The only advantage is that the error message is more gentle, at the expense of adding a bunch of code and potentially hiding the underlying error message from the user so that they could fix it.

I think Python also gets that right, where it's common to raise exceptions when exceptional things happen, and only ever handle the exceptions you can actually do something about. In 99.999% of projects, what are you actually going to do at the application level to properly deal with an OOM or disk full error? Nothing. It's almost always better to just crash and let the OS / daemon manager / top level event loop log that something bad happened and schedule a retry.

samiv8• 2 years ago

The whole story is 3-fold. We have

  - errors in the software itself, aka BUGS
  - logical conditions that are expected part of the program execution flow and expected state. some of these might be error conditions but only for the *user*. In other words they're not errors in the software itself.
  - unexpected failures where none of the above applies. typically only when some OS resource allocation fails, failed to allocate memory, socket, mutex etc and the reason is not because the programmer called the API wrong.

In the first category we're dealing with BUGS and when I advocate asserting and terminating the process that only really applies to BUG conditions. If you let an application to continue in a buggy state then you cannot logically reason about it anymore.

The logical conditions are the typical cases for example "file not found" or whatever. User tries to use the software but there's a problem. The application needs to deal with these but from the software correctness perspective there's no error. The error is only what the user perceives. When your browser prints "404" or "no internet connection" the software works correctly. The error is only from the user perspective.

Finally the last category are those unexpected situations where something that should not fails. It is quite tricky to get these right. Is straight up exiting the right choice? Maybe the system will have more sources later if you just back off and try again later. Personally in C++ projects my strategy is to employ exceptions and let the callstack unwind to the UI level, inform the user and and then just return to the event loop. Of course the real trick is to keep the program state such that it's in some well defined state and not in a BUGGY state ;-)

veidelis• 2 years ago

Makes sense. Better to unwrap via .expect("msg"), though.

dehrmann• 2 years ago

This is often called offensive programming.

samiv8• 2 years ago

Hot damn, I never heard of this term before but yeah that's exactly what it is.

TIL, thanks.

crabbone• 2 years ago

I don't agree with any of this.

First of all, this results in unintelligible errors. Linux is famous for abysmal error reporting, where no matter what the problem really is, you get something like ENOENT, no context, no explanation. Errors need to propagate upwards and allow the handling code to reinterpret them in the context of the work it was doing. Otherwise, for the user they are either meaningless or dangerous.

Secondly, any particular function that encounters an unexpected condition doesn't have a "moral right" to terminate the entire program (who knows how many layers there are on top of what this particular function does?) Perhaps the fact that a function cannot handle a particular condition is entirely expected, and the level above this function is completely prepared to deal with the problem: insufficient permissions to access the file -- ask user to elevate permission; configuration file is missing in ~/.config? -- perhaps it's in /etc/? cannot navigate to URL? -- perhaps the user needs to connect to Wi-Fi network? And so on.

What I do see in practice, is that programmers are usually incapable of describing errors in a useful way, and are very reluctant to write code that automates error recovery, even if it's entirely within reach. I think, the reason for this is that the acceptance criteria for code usually emphasizes the "good path", and because usually multiple bad things can happen down the "bad path", it becomes cumbersome and tiresome to describe and respond to the bad things, and then it's seldom done.

dvektor• 2 years ago

yup. we have definitely all gotten an ENOENT or EIO before with no context.

rikthevik• 2 years ago

The code is allowed to continue after it has logically bugged out.

I'm a big fan of asserting preconditions and making it clear that we are getting into a bad place. I would rather dig through Sentry for an AssertionError than propagate a bad state and having to fix mangled data after the fact. If the AssertionError means that we mishandled valid user input, no problem, we'll go fix it.

A few times in my career I've had to ask, "okay, how long has this bug been quietly mangling user data?" and it's not a fun place to be.

Side note: I've never understood the convention of removing asserts in production builds. It seems like removing the seatbelts from the car before the race just to save a few pounds.

zarzavat• 2 years ago

Side note: I've never understood the convention of removing asserts in production builds. It seems like removing the seatbelts from the car before the race just to save a few pounds.

Once an upon a time computers were slow and every cycle mattered. Assertions were compiled out of the build by necessity. Better a crash once in a while than the program hardly running because it was so slow.

monlockandkey• 2 years ago

Asserts are only available in debug compile mode.

adonovan• 2 years ago

I’m a big fan of assertion and rigorous preconditions but there are times when a failure of some invariant in a minor subsystem should not be allowed to crash the entire process, especially if the context makes it easy to return an error.

In our project (the language server for Go) we have gotten tremendous value from telemetry: return an error, but report home the 1-bit fact that the assertion has failed. Often that fact is enough to figure out why; other times it is necessary to refine the assertion into two or more (in a later release) to get another bit or two of information about the nature of the failure.

j1elo2• 2 years ago

Please, no!

I worked with a 3rd party library that had this mentality. "A bug is a bug so the assert fails and thus the code is now in an unknown state thus The right thing to do would be to ABORT the process execution immediately". Oh my.

Just do "if (pointer)" and when that fails, error out from the smallest context possible that applies to that pointer, and nothing more than that. I.e. the real BEST thing to do is to abort the current connection. To skip the current file with an error. To fail writing that piece of memory. Whatever. But never abort (unless maybe in debug builds).

The end result of this library was that we had a WebRTC server handling 100s of simultaneous video calls, and then when a single new user tripped up during connection and went through a bogus code path, the library would decide "oh something is not as I expected so I'll abort, of course!" and the whole production server was brought down with it.

That kind of behavior does not help achieving high production quality and providing robust and reliable services.

We ended up removing the library's runtime assertions, which meant that connections that would bug the library code would just end up failing with an error somewhere else, that could be used to just discard the attempt and try again. All in all, numbers showed it was a huge positive in stability for the service.

mcdeltat• 2 years ago

I think the original comment was more directed to scenarios where the precondition fail is let slide, NOTHING happens, and that's not the desired behaviour, so you have a bug. Such code exists too often and it's just poor quality.

If there's anything I've learnt about error handling, it's that it must be approached with very careful consideration of what you want the app to do (conceptually) when the error occurs. Sometimes that's crash the program, sometimes it's throw an error, sometimes it's log and move on. The issue comes when devs don't want to think about this, whereby the simplest solution is absorb any error and forget about it.

tsss• 2 years ago

Any sane person will throw an exception, which, when unhandled, will crash the program.

samiv8• 2 years ago

So the library was full of bugs.

(IMHO) The right thing to do would be to

  - Fix the actual bugs
  - Provide super visor  and isolation as a protection mechanism and resiliciency.

What you've now done is to essentially just hammer it quiet while sweeping all the issues under the rug and pretend everything is great when the library is actually in ill defined state. How's that possibly any better? You're probably experiencing hard to detect bugs, occasional runtime corruptions and all the fun stuff now that you'll likely never be able to fix :)

"That kind of behavior does not help achieving high production quality and providing robust and reliable services."

It absolutely does.

When bugs are obvious and caught early they're easier to fix and this leads to higher quality more robust and reliable service.

Pretending that everything is good is never the solution.

Mawr• 2 years ago

Oh my indeed.

That kind of behavior does not help achieving high production quality and providing robust and reliable services.

Right, so since it's the messenger (assertion) that brought news of defeat in battle, you kill the messenger instead of trying to win the next battle.

Your problem is that the entire business seems to hinge on a single point of failure - if the server binary dies, you lose customers. This has nothing to do with what some library does or does not do, it's just a horribly designed system.

That crash should have made you all stop and think real hard about what you're doing because clearly it's not working:

1. Don't write bugs in the first place. No I'm not kidding. Your tests clearly suck so fix them, make sure you do proper PR reviews, make sure to leverage your type system to the fullest, use all available tooling, etc.

2. Design your system be resilient to the server binary dying because I assure you, there's nothing you can do to prevent it from happening for one reason or another.

jminter• 2 years ago

Absolutely. An intermediate path in Go is to recover any panics on your goroutines: in this case a nil dereference panic may cause the death of the goroutine but not the whole application.

An example where this can be useful is in HTTP request handling: a single request might fail but the others can keep going -- but there are plenty of other use cases too.

The panic recovery code can log for further investigation, as well as in the HTTP case for example probably returning a 500 to the caller if wanted.

There are of course plenty of valid reasons not to take an approach like this too, but in some circumstances it can be useful.

gregwebs• 2 years ago

There'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/ (errors with stack traces and metadata)

https://github.com/gregwebs/errcode (adding codes to errors- working on improving docs and writing about this now).

AceCream• 2 years ago

type xError struct { msg message, stack: callers(), }

is this legit in go?

ptman• 2 years ago

I'm still just growing organically. Constant errors are useful for sentinel values https://dave.cheney.net/2016/04/07/constant-errors . 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.

How we centralized and structured error handling in Golang

Related Stories