What's the library's policy on error wrapping vs vanishing?

[Blokyk]

I've spotted a few spots in the library where errors are vanished (%v with fmt.Errorf) instead of wrapped (%w), and it has in fact bitten me a few times on a project I'm currently working on. Is there a hard policy for when errors are vanished vs wrapped? if not, I'd be happy to submit a PR trying to turn a few %v into %w :)

(I'm a complete noob when it comes to golang, so maybe this is just a common convention or w/ever, feel free to shut this down if so)

[ti-mo]

Hi @Blokyk, thanks for the question. Sometimes, this is done on purpose to avoid callers depending on specific sentinels, but it could be that the cases you've spotted were simply mistakes.

We tend to call out specific error returns in the godoc string (e.g. this returns [ErrNotSupported] if ...), as well as having specific tests for them, since they're basically part of the API contract. Please file an issue and/or a PR if you'd like to make some changes.

[Blokyk]

After that reasoning, I think in the case of the function that was causing me trouble (perf_event.go"s attachPerfEventLink), it can make sense not to add the error to the API contract, so I'm fine with it.

[Blokyk]

Though, I do have to wander: why are some errors wrapped things like ErrNotSupported while others are sent as-is from the system (e.g. EEXIST)?

[ti-mo]

Think of wrapping errors with %w as simply adding extra context. For example, it's common to receive an EINVAL from the kernel and wrapping it in ErrNotSupported, which will result in both errors.Is(err, ebpf.ErrNotSupported) as well as errors.Is(err, unix.EINVAL) evaluating to true. Only in rare cases do we throw away the initial errno and replace it with a sentinel, and those cases are usually documented with a reason.

If something returns EEXIST, there's not always something useful to add, since many cases could potentially throw EEXIST.

[Blokyk]

That all makes sense, thanks!