djdv / go-filesystem-utils

ISC License
10 stars 2 forks source link

error values need to be modernized #22

Open djdv opened 2 years ago

djdv commented 2 years ago

Short version: This codebase predates Go 1.13. All the features added to Go's standard errors since; should be used instead of the various things we do right now.

Long version: Throughout the project there are a few ways errors are handled. The bulk of this code was written in 2017-2018 which predates Go 1.13. (This is true too for some of our dependencies as well) As such, things like error wrapping, comparison, joining, etc. were less standardized and relied on per-project conventions instead.


The most common pattern in the current codebase is almost a direct copy of how errors are handled within upspin, with minor deviations and extensions. (ref: https://commandcenter.blogspot.com/2017/12/error-handling-in-upspin.html) Although other patterns exist at various layers too.

These afforded many benefits, most important of which was the ability to add additional information to errors without losing comparison, as well as extending them to contain data which could be interpreted for multiple systems. E.g. our fserrors.IO can be returned from common code, and at edges, be translated into things like FUSE's errorno, 9P error strings, 9P2000.L's errorno, native syscall errors, etc. The types and values of which all differ, despite being the same kind of error, derived from the same cause/fault.

While these were great then, you wouldn't do this today. The standard errors pkg has evolved significantly and errors.{Is|As|Join} along with fmt.Errorf's %w (and in 1.20 the ability to use multiple %w tokens), satisfy the same requirements we have, but in a standard way.


We should try to migrate away from this older style somehow. Either gradually, or all in one shot. And favour the standard library style instead. Utilizing constant errors formatted with fmt.Errorf, and errors.Is to compare and translate Go errors to API specific errors where needed. If possible we should extend this effort into our dependencies so that things are consistent the whole way through.

djdv commented 1 year ago

Reminder to update any tests when we tackle this: https://github.com/djdv/go-filesystem-utils/pull/25/commits/63d7b5650c6190a3e8e2b637fc2efe21929dfec6#r1037622423