4 ms·

Indeed, our code base is littered with fmt.Errorf("...: %w", err), but that only works if enough places in the code add context. Currently only about 15% of ret

Indeed, our code base is littered with fmt.Errorf("...: %w", err), but that only works if enough places in the code add context. Currently only about 15% of return sites do this. And I disagree that the cost of carrying around the callstack is something to worry about. Errors are akin to exceptions in C++/Java: no happy path should rely on errors for control flow (except io.EOF, but that won't generate a call stack). They should be rare enough that any cost below about 1ms and 10k is negligible.

morelisp 3y ago
Are you suggesting it's OK if ParseInt failures take 1ms? Or should ParseInt use a different "kind of error" that's not commensurate with the regular error kind? Do you think most errors look more like ParseInt, or more like sql.Open where 1ms might be acceptable? (Do you think a call stack from the insides of sql.Open would be useful? My experience, mostly not...) So the stacks should probably only be for "complex errors", and only for frames that happen in code you (hand waving) "care about". Maybe your programs just have far too complex internal error handling?
- physicles 3y ago
  See my response to a sibling. I wasn't clear; I was implicitly differentiating between these: 1. errors that can be handled locally (such as parsing; in other languages, these situations are often signaled with return values instead of exceptions) 2. errors that can't be handled locally (such as network errors; other languages use exceptions for these) My argument was that worrying too much about error handling performance in #2 is premature optimization. 1ms is extreme, but the actual figure of capturing a call stack in Go -- several microseconds, by my benchmark -- puts it squarely in the "don't worry about it unless your code is performance-critical" category.
  - kiitos 3y ago
    An error is an error. The immediate caller is always responsible for detecting and handling errors in whatever way is appropriate for their calling context.
randomdata 3y ago
> that only works if enough places in the code add context. It would be a bit odd to not add context, wouldn't it? Same goes for any value. This is not exclusive to errors. If you consider a function which returns T, the T value could equally be hard to trace back if you find you need to determine its call site and someone blindly returned it up the stack. There is nothing special about errors. While ideally you are returning more context than Errorf allows, indeed, it is a good last resort. If your codebase is littered with blind returns, the good news is that it shouldn't be too hard to create a static analyzer which finds blind returns of the error type and injects the Errorf pattern.
kiitos 3y ago
Every error should be annotated at the call site. fmt.Errorf("...: %w", err) isn't litter, it should be a basic expectation of any code which passes code review. > Errors are akin to exceptions in C++/Java: no happy path should rely on errors for control flow (except io.EOF, but that won't generate a call stack). They should be rare enough that any cost below about 1ms and 10k is negligible. This may be true in C++ or Java, but in Go, it is absolutely not the case. Errors are essential to, and actually the primary driver of, control flow! Any method or function which is not guaranteed to succeed by the language specification should, generally, return an error. Code which calls such a method or function must always receive and evaluate the returned error. Happy paths always involve the evaluation and processing of errors received from called methods/functions! Errors are normal, not exceptional. (Understanding errors as normal rather than exceptional is one of the major things that distinguish junior vs. senior engineers.)
- physicles 3y ago
  I think there's less daylight between us than it seems. > Errors are normal, not exceptional. The _handling_ of errors is normal. Code that doesn't consider errors is not production code. And granted, in Go, control flow is driven by errors more often than in C++ or Java. Sentinel error values are common. See for example all usage of error.Is, checking for io.EOF, packages that define ErrSituationA and ErrSituationB, etc. But my argument was about errors that can't be dealt with locally, where the origination and ultimate handling are very far apart. A given flow will encounter these errors relatively rarely compared to the happy path (and if it's not rare, you probably need to fix or change something). Having an intuition about this is important for predicting your code's performance. For example: - The SQL call failed because the network connection dropped; client gets 500 or 502, or retry. - A call to an external service failed because the network was bad; it gets retried. - The SQL call succeeded, but the record the client asked for wasn't found, so the client gets a 404. - Writing to a temporary file failed because the disk is full, so some batch job fails with an error. Apart from potential concerns about DoS, worrying too much about the performance of error handling in these relatively rare cases is absolutely premature optimization. DoS isn't even a concern. I just benchmarked capturing a call stack in Go, and it's on the order of a few microseconds. Unless you're in performance critical code (and you're benchmarking, right?), it's fine.

morelisp 3y ago

Are you suggesting it's OK if ParseInt failures take 1ms? Or should ParseInt use a different "kind of error" that's not commensurate with the regular error kind? Do you think most errors look more like ParseInt, or more like sql.Open where 1ms might be acceptable? (Do you think a call stack from the insides of sql.Open would be useful? My experience, mostly not...) So the stacks should probably only be for "complex errors", and only for frames that happen in code you (hand waving) "care about". Maybe your programs just have far too complex internal error handling?

physicles 3y ago
See my response to a sibling. I wasn't clear; I was implicitly differentiating between these: 1. errors that can be handled locally (such as parsing; in other languages, these situations are often signaled with return values instead of exceptions) 2. errors that can't be handled locally (such as network errors; other languages use exceptions for these) My argument was that worrying too much about error handling performance in #2 is premature optimization. 1ms is extreme, but the actual figure of capturing a call stack in Go -- several microseconds, by my benchmark -- puts it squarely in the "don't worry about it unless your code is performance-critical" category.
- kiitos 3y ago
  An error is an error. The immediate caller is always responsible for detecting and handling errors in whatever way is appropriate for their calling context.

randomdata 3y ago

> that only works if enough places in the code add context. It would be a bit odd to not add context, wouldn't it? Same goes for any value. This is not exclusive to errors. If you consider a function which returns T, the T value could equally be hard to trace back if you find you need to determine its call site and someone blindly returned it up the stack. There is nothing special about errors. While ideally you are returning more context than Errorf allows, indeed, it is a good last resort. If your codebase is littered with blind returns, the good news is that it shouldn't be too hard to create a static analyzer which finds blind returns of the error type and injects the Errorf pattern.

kiitos 3y ago

Every error should be annotated at the call site. fmt.Errorf("...: %w", err) isn't litter, it should be a basic expectation of any code which passes code review. > Errors are akin to exceptions in C++/Java: no happy path should rely on errors for control flow (except io.EOF, but that won't generate a call stack). They should be rare enough that any cost below about 1ms and 10k is negligible. This may be true in C++ or Java, but in Go, it is absolutely not the case. Errors are essential to, and actually the primary driver of, control flow! Any method or function which is not guaranteed to succeed by the language specification should, generally, return an error. Code which calls such a method or function must always receive and evaluate the returned error. Happy paths always involve the evaluation and processing of errors received from called methods/functions! Errors are normal, not exceptional. (Understanding errors as normal rather than exceptional is one of the major things that distinguish junior vs. senior engineers.)

physicles 3y ago
I think there's less daylight between us than it seems. > Errors are normal, not exceptional. The _handling_ of errors is normal. Code that doesn't consider errors is not production code. And granted, in Go, control flow is driven by errors more often than in C++ or Java. Sentinel error values are common. See for example all usage of error.Is, checking for io.EOF, packages that define ErrSituationA and ErrSituationB, etc. But my argument was about errors that can't be dealt with locally, where the origination and ultimate handling are very far apart. A given flow will encounter these errors relatively rarely compared to the happy path (and if it's not rare, you probably need to fix or change something). Having an intuition about this is important for predicting your code's performance. For example: - The SQL call failed because the network connection dropped; client gets 500 or 502, or retry. - A call to an external service failed because the network was bad; it gets retried. - The SQL call succeeded, but the record the client asked for wasn't found, so the client gets a 404. - Writing to a temporary file failed because the disk is full, so some batch job fails with an error. Apart from potential concerns about DoS, worrying too much about the performance of error handling in these relatively rare cases is absolutely premature optimization. DoS isn't even a concern. I just benchmarked capturing a call stack in Go, and it's on the order of a few microseconds. Unless you're in performance critical code (and you're benchmarking, right?), it's fine.