preslavrachev / preslav.me-comments

0 stars 0 forks source link

2023/04/14/golang-error-handling-is-a-form-of-storytelling/ #22

Open utterances-bot opened 1 year ago

utterances-bot commented 1 year ago

Go's Error Handling Is a Form of Storytelling · Preslav Rachev

I am a software engineer with a decade-long experience developing software using Java, Go, and Elixir.

https://preslav.me/2023/04/14/golang-error-handling-is-a-form-of-storytelling/

ghost commented 1 year ago

I like beginning a func that returns an error with a defer that wraps the error. It lets me follow the flow from func to func.

func DoIt() (err error) {
    defer func() {
        if err != nil {
            err = fmt.Errorf("DoIt: %w", err)
        }
    }
    ...
    return
}
thomas708 commented 1 year ago

@josephbudd If there are multiple places inside DoIt() where an error could be returned it would be better to have additional information on which place the error comes from. Your approach loses that info. Except for very small functions (i.e. utility functions) most larger functions that contain business logic would have multiple places inside the function where an error could be returned.

batazor commented 1 year ago

I think the best way to describe the errors is to print the logs as near as possible to where the error occurred and also to use the runtime package to print the context information (file name, line of code). This is much better than any wrapper over it.

Also, specifying request-id will help in debugging cross-service communication. We end up using something like Grafana-Loki or Kibana to search among thousands of logs, and this is best done with the JSON format and the addition of meta information needed for error research or auditing.

adam-debkowski commented 1 year ago

I do not see how repeating the name of the function that just failed is any useful. If your functions do not read well (i.e. you do not know what a function does and need additional comment for this) why not rename the function itself? Or refactor it so you can have smaller pieces of logic that would be easier to name. Just adding a comment seems like a fix for the wrong issue.

I would argue that adding such comments is actually bad for your code. Not only do you repeat yourself every time (since an error message will basically be the same as function name, but with more human readable format), when you change the name of the function you also need to update the comment! Especially when doing some refactoring, you rename the function in multiple places at once - easy to miss those comments. And you might already see another issue with this approach - if a single function is being called multiple times from different places in the code, you will have the same error comment repeated for every function invocation.

So, what to do instead? If the problem you are trying to solve is locating the origin of an error, then use the stack trace. It has all the information you would need, and it is all generated automatically. No need to double your codebase in size just to have pretty messages, no problems when refactoring, and very precise information about the error location.

There might be other advantages of using error messages, but code readability and locating error origin I would argue are not the ones.

ghost commented 1 year ago

@adam-debkowski @batazor

Thanks. I'll have to look into that.

jmhungdev commented 1 year ago

I enjoyed this read, thanks!

KZiemian commented 11 months ago

Good article.

healsdata commented 7 months ago

Is there a linter that finds places where you just pass through the underlying error? I want to get better at wrapping errors but often find myself just return the error to get a test passing. Basically this:

if err != nil {
    return nil, err
}
preslavrachev commented 7 months ago

@healsdata Yes, if you are using the GolangCI Lint runner, there is a neat linter, called wrapcheck. It will report when errors remain unwrapped. I think it is exactly what you need. See the example below.

image