els0r / goProbe

High-performance IP packet metadata aggregation and efficient storage and querying of flows
GNU General Public License v2.0
12 stars 4 forks source link

Antipattern in some (custom) error type assertions #255

Closed fako1024 closed 8 months ago

fako1024 commented 12 months ago

The implementation from #250 contains a few type assertions which are not safe, e.g.

if err != nil {
    // if there's an args error, print it in a user-friendly way
    prettyErr, ok := err.(types.Prettier)
...

or

switch t := err.(type) {
case *query.ArgsError:
    vr.ArgsError = t
default:
    LogAndAbort(ctx, c, http.StatusInternalServerError, err)
    return
}

The issue with these assertions is that they won't work on wrapped errors (which may or may not be present, but might of course at any point in time). Best practices suggest to use errors.As() instead.

fako1024 commented 12 months ago

@els0r FYI.

els0r commented 11 months ago

Thanks for reporting and the suggestion.

Let's make sure to clean this before 4.1.