How we centralized and structured error handling in Golang

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.

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.

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.

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

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.

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.

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.

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.

This is often called offensive programming.

"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. :-)

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");
  }

  return load_config().unwrap();

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.

Asserts are only available in debug compile mode.

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

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

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.

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.

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

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

ah, yes, completely separate.

HTTP code: 200 ok

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

The sane thing to do is to let lower layer functions return

    Either<Error, Foo>

    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)
                })

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

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.

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.

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

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.

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.

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.

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

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

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

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

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

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

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

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?

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.

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

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