honeybadger-io / honeybadger-go

Send Go (golang) panics and errors to Honeybadger.
https://www.honeybadger.io/
MIT License
34 stars 15 forks source link

Error Class #9

Closed matthewdu closed 8 years ago

matthewdu commented 8 years ago

Many of the errors in our applications are errors.errorString struct. However, when the errors are sent to Honeybadger, they are all treated as the same error because they are of the same class.

Can Error.Class be updated to be the message if the type is errorString struct? It would make find errors easier, and not force us to create new error types.

joshuap commented 8 years ago

@matthewdu I'm a little hesitant to change the behavior specifically for that class by default. What if we were to add a callback which would be called before the notice was sent to Honeybadger which would allow you to change the class depending on its value? You would have access to all the data in the Notice struct: https://github.com/honeybadger-io/honeybadger-go/blob/master/notice.go#L19

alfredr commented 8 years ago

Yeah this is absolutely crazy to me. We have a background job system it works like this:

func backgroundScheduleTask(scheduleSpec string, f backgroundTask) {
    backgroundCron.AddFunc(scheduleSpec, func() {
        if err := f(); err != nil {
            honeybadger.Notify(err)
        }
    })
}

That is idiomatic golang -- the error could be any number of things, why in the world would we want to treat them the same because the error is an errorString? Am I using this wrong?

joshuap commented 8 years ago

@alfredr can you give me an example of some of the various errorString values you might encounter? My main concern is that error class in HB is meant for a specific class of error, like a category. It is not meant to be a long string/sentence. Doesn't golang allow you to create custom types for errors?

alfredr commented 8 years ago

@joshuap it can be literally anything.

Error handling in go works like this: errors are just values. There is a built in interface called error. Anything which supports an Error() function which returns a string can be an error.

Library vendors do not typically create custom error structures to encode the type of error. Instead they may do something like declare several types of error using the errors package (e.g. the most common postgres driver lib/pq) which creates (you guessed it) an errorString.

If the user is concerned about the nature of the error they may choose to compare the return value with one of the values (e.g. if err == pq.ErrNotSupported)

If they are not concerned, they often times abort simply returning the value to the caller.

Thus we often have a situation where we have an error interface of unknown origin. That's okay -- we can still test if its something we think we can handle. If not, then we would probably pass it up the stack. Eventually, we'd like to report it :). It's not feasible to edit the postgres driver code to force them to use different types. Further, It's both non-idiomatic and silly to wrap all of the error responses with some kind of "postgres error" object, because then these would be lumped together and we would have to make as many types as situations...

tldr; the "class" of an error in golang does not establish its identity.

joshuap commented 8 years ago

@matthewdu I've made it possible to override the error's class name for manual error notifications like this: honeybadger.Notify(err, honeybadger.ErrorClass{"CustomClassName"}) (see 63adad3172929d5d227c4f7c5d980c570c0a4eb6). So if you are doing manual notifications, you could make the error class anything you want (including the message). I still don't want to add an option to always change the error class to the error message, but we're planning on adding a new option for go projects which will include the message in our grouping strategy. I don't have an ETA on that yet, but I'll update this issue when it's available.

joshuap commented 8 years ago

Update: you can now globablly group errors.errorString by message instead of class: https://github.com/honeybadger-io/honeybadger-go#grouping