Comment by tedsuo | Hacker News Reader

Comment by tedsuo | Hacker News Reader

Parent Full thread

Oh god, there is some terrible advice in that article.

1. Don’t wrap errors, log.

Errors have two purposes - control flow for your application, and information for the developer and operator. If you are already logging the flow of your program, there is no need to wrap an error and create a call stack - you already have the call stack. If you cannot follow the control flow from your logs, you need to improve your logging, because you will also need to debug production problems that do not produce a literal error object.

And if you want more causality than you can get out of your logging, consider upgrading from logging to tracing. I contribute to http://opentracing.io so naturally that’s the tracing API I would suggest you look at.

2. Do not do anything with panics except recover from them or exit.

If you have a panic that you cannot recover from (and in general, that should be all of them), then it means your application is in a unknowable state. While Go is not as unsafe as C, any and all state in your program could be bad, and it is completely unclear what is still working and what is not. Nothing you do at this point is safe. For the love of god, do not hang your application and prevent it from exiting by trying to send a Slack message. Your goal should be to restart your program from a fresh clean state as quickly as possible, not to tie it up on the way out. Let the program exit and have an external monitoring program, whose state is NOT corrupted because its in a separate process, do all of the triage and reporting.

(1) I see this all the time.

An error is logged and propogated, then logged and propogated, then logged and propogated, then pretty soon your logs are 50x copies of themselves and the flood of logging sweeps you away.

You either (a) know what you to do about an error and log it or (b) you don't know what to do and propogate it.

An error should be logged eventually, but it shouldn't be logged 20 times.

I totally agree, there's no need to log that you bubbled an error. My recommendation is to have the function that generates the error also log it internally. That way, you don't need to log any returned errors at all, unless you are calling a 3rd party library that can't log it's own error.

Error handling, which includes logging the error, should almost always be the responsibility of the caller. Only the caller knows why the call was made, and only the caller knows the impact of the error on what the caller is trying to do.

Exactly.

You run a process that exits with a non-zero code. Is that an error that should be logged? Or just informational? (E.g. `diff`.)

You request an HTTP resource that doesn't exist. Loggable error? Or normal result?

Only the caller knows.

I recommend separating collection from instrumentation. Always instrument at the source, so that you can instrument the flow of your entire application. If the caller decides there is something about the code flow that is worth recording, it can trigger collection of the entire trace.

I see that way too often. Bonus points if the exceptions are being logged in different ways, in different parts of the library.

(1) is entirely debatable. imho your prescription is bad advice. it is very helpful to have []uint call stack pointers (i.e. not a ton of data) associated with errors for the 1 in 10 chance you'll need that information, especially when dealing with libraries you don't control that generate errors (which for most programs is the majority, things like lib/pq etc.)

In Go, errors are just values, and `error` is just an interface. If some data (e.g. stack traces) is helpful to you then make it part of your error implementation.

I'm not entirely opposed to Dave Cheney's errors package but in general I think it's trying to be too clever and novel. If anybody wants to unwrap the errors you return, they have to basically buy into that package and its ecosystem. It's strongly coupled to itself, and that's a bad design choice IMO.

We've been fine-tuning error logging and handling for decades. We ought not reinvent the wheel solely because new language constructs are available. My opinion is that structured data and not "errors with behaviors" wins every time, especially when you're building distributed systems and you are going to need to slice/dice your logs when you investigate problems.

It's not strongly coupled beyond requiring that the wrapper implement a `Cause() error` method. I don't really think that could ever be considered strong coupling

they don't have to buy into anything, you just do a `fmt.Printf("%+v", err)` when you want to show stack traces

For 2, I stopped reading and went straight for the comments to see if my logic was flawed.

The process is "panicking", but it is still reliable enough to post a message to Slack? Even writing a log entry might be too much, depending on what exactly the failure was.

> The process is "panicking", but it is still reliable enough to post a message to Slack? Even writing a log entry

Well.. most likely yes. If I invoke panic, all state is exactly as it was just prior to that statement. And even if not, it's not gonna "launch the missiles" if that slack post or log-write fails now, is it?

> Even writing a log entry might be too much

"Too much", how? Either it succeeds in which case it'll help you, or it won't which has the same result as not attempting it in the first place: no log entry..

The most common problem is that code executing at that point can hang in unexpected ways, preventing shutdown. It's a real bummer when that happens. I've even seen it happen with logging written the wrong way, where the code attempts to flush the logs to ensure they are written... and hangs.

Meanwhile, the crazy code that panicked is still running it's other goroutines - remember, you called recover! - so maybe now the webserver still has an open port and is allowing your users to access whatever strange state is left inside... gonzo things really can happen if you let a corrupted program stay on rather than shutting it down immediately. Even if nothing bad is happening, you still are out of commission for that entire period.

The point is, murphy's law always comes into play. So if we're talking about production best practices, consider that "most likely fine" means "definitely not fine at scale over time". Just make sure that whatever you're doing during shutdown can't block.

Would you suggest the same course of action for any language which uses exceptions? Because there is nothing special about the stack unwinding you get with panic/recover.

I'm seriously wondering if you ran into any trouble with recovering panics in production, because that would imply all Java, C#, Python, JS and Ruby server code in production which is happily catching and logging exceptions in the main request handler is constantly running into corrupted state.

That's not an accurate comparison. The issue is not the stack unwinding, but the root cause of the panic. In all of those other languages, exceptions are used for normal error handling in addition to "fatal" events. In Go, panics, if used at all, are generally used for fatal runtime errors (memory, segfault, etc), or in place of assert, and indicates a fatal, unrecoverable flaw in your program.

At every job I've worked at it was generally discouraged to handle MemoryError exceptions in python and OutOfMemoryError exceptions in Java. It's safer to assume there is nothing you can do on such occasions, because sometimes (usually the worst possible time) there isn't.

Another relevant Java example might be VirtualMachineError (superclass of OutOfMemoryError) and the wonderfully named subclass UnknownError(!).

If you invoke panic(), sure. If the runtime invokes panic(), it might be in a pretty awful state (e.g. unable to allocate memory.)

There are many ways to "fail to do X", most of them much more complex (and possibly damaging) than "won't do X", especially in a program whose state is now corrupted. So limit that surface area as much as possible.

^ EDIT: Since this is the top comment, I should clarify that I don’t think the author is dumb or anything; 1. in particular is fairly common advice. The issue is not the error wrapping itself, but the smell that your logging solution is probably not recording causality.

And as for 2., I recall a hilarious incident with an email alert turning a deployment into a spambot, so... :)

Exceptions as flow control is an anti-pattern, unless you're ducktyping like in Ruby or Python

I'm trying to figure out what you could possibly mean by that—exceptions are explicitly a tool for altering control flow, that's basically the only thing they do, and Go also doesn't have exceptions.

They do much more than just alter control flow - they add to the stack, and are used to debug issue or bonk out of your program entirely. In duck-typed like Ruby/Python, you don't necessarily know whether a piece of code is going to fail, so you assume it does. That's why you have try/catch blocks all over the place.

In statically-typed languages like Java, you should be able to infer, mostly, what your program is doing at runtime, and if it's doing something you're expected, you raise an exception/error so that you stop processing whatever you're processing.

> In duck-typed like Ruby/Python, you don't necessarily know whether a piece of code is going to fail, so you assume it does. That's why you have try/catch blocks all over the place.

That's an unreasonable viewpoint. Whether you understand what a piece of code does at runtime is down to the particulars of the code far more than it depends on the language you choose, and it's nothing but code smell to put try/catch everywhere because the code isn't doing what it's supposed to be doing. At best we might agree that it's difficult to maintain a high level of code quality in a large Python project, but try/catch everywhere won't improve things.

There are generally two good places to put try/catch. The first is to put try/catch around a small piece of code that is expected to throw an exception, like parseInt(), and handle the exception right there. The second is to put try/catch around a piece of work which is large enough to make a decision about even when you don't know why it failed—things like top-level HTTP request handlers in web servers, or individual work items in batch processors where you can decide to skip the item and move on to the next. In either of these cases, you're using exceptions to alter control flow.

That's the same way it works in Python, Java, C++, C#, and so many others. And Go uses the same principle, too, even though Go uses error returns. Which is why I am still confused about your original comment—since Go does not have exceptions. Go literally just has panic() and return.

> In duck-typed like Ruby/Python, you don't necessarily know whether a piece of code is going to fail, so you assume it does

You should always assume there may be runtime errors regardless of whether you have some sort of static type safety or not.

There are some languages that can guarantee no runtime errors, but they make significant sacrifices in terms of features [1].

[1] http://www.adacore.com/sparkpro/

> In statically-typed languages like Java, you should be able to infer, mostly, what your program is doing at runtime

Perhaps if your exceptions are type-checked. AFAIK Java is the only language that does that.

I find that "Use exceptions, but don't use them for flow control" always comes from Java folks.

Everyone else either doesn't use exceptions, or embraces them.

In Go or in general?

Please please write a blog post or tiny sample app illustrating this stuff

Reply on news.ycombinator.com