Juju Errors V2 Proposal 🎉
tlm opened this issue · 0 comments
Introduction
Since juju/errors was first concieved a lot of changes have happened in the Go errors space that are starting to be in conflict with the design of this library. We have a chance to cut a new v2 implementation of this library building on top of what is on offer in std errors.
This issue aims to both document my thoughts around what this new library might look like and solicit further design feedback and consensus before proceeding. Being a v2 of this library there will be a number of breaking changes, the migration plan for how we move Juju to this new library if it is adopted is also documented below.
This work should be considered out of band to the current concerns we have going on for Juju and it is my plan to offer up the changes to the library while traveling to get the work done and dusted.
New Design
The new design being proposed below is taking an opinionated design on how to deal with errors that occur during program execution. Some of these opinionated elements may end up being left out of this change based on feed back recieved.
Removal Of Utility Error Types
The first opionated change being proposed is the removal of the const error types defined in v1 of this library such as NotFound & Timeout. While these error types are useful they are far to broad in the problem they are describing and don't clearly indicate to the users of the error which specific part has failed or a reliable way to assert exact problems.
If you recieve a NotFound error what exactly wasn't found and at which level in the error chain is the NotFound error located at? As apposed to package level errors that are communicating specific NotFound instances. For example:
func PackageFunc() error {
if err := operation1(); err != nil {
return err
}
if err := operation2(); err != nil {
return err
}
}If both operation1 and operation2 return errors that satisfy errors.NotFound then how do we know specifically what wasn't found? As opposed to:
func operation1() error { return ErrorThingXNotFound }
func operation2() error { return ErrorThingYNotFound }Now with package specific errors the caller can both assert exactly what wasn't found and even produce better error message for the end user on how to rectify the situation.
What I would like to see is the error types such as NotFound and Timeout moved into a new package in Juju as core/errors and have their respective usages decline over time. Moving them to Juju allows us to both keep using the error types where they make sense and provide better package level errors. This idea is discussed a little more further down.
Removal Of High Level Tracing Funcs
A common pattern we have in Juju is tracing returned errors up a call stack till a caller wishes to process the error and deal with it. Example of how this works:
func One() error {
return errors.Trace(Two())
}
func Two() error {
return errors.Trace(Three())
}
func Three() error {
return errors.New("some error")
}
err := One()
fmt.Println(err)
// some error
fmt.Println(ErrorStack(err))
// github.com/juju/juju/mypackage/test.go:10: some error
// github.com/juju/juju/mypackage/test.go:15: some error
// github.com/juju/juju/mypackage/test.go:20: some errorThe ability to trace errors allows for a debuging perspective later on to take an error and figure out where it has originated from. Adding Trace() at each level annotates the error with the last known place something touched the error.
The problem that I see with this approach is it's very coarse and seems to be hiding two problems that we face when dealing with errors. The first is that errors generated don't contain enough information and context for what has gone wrong and the second is we aren't annotating errors well as they traverse a call stack in Juju.
With this tracing we might know where an error originated from but we don't have enough context for what was the precondition into the error. With traced errors we also have to have the forsight to dump the stack for the error with ErrorStack as they aren't naturally surfaced.
The new design of this library explicitly takes away the high level Trace() function from the user until an operation is performed that first enriches the error with more context. This is a deliberate design decision to promote better error annotating first before tracing. As an example:
func One() error {
return errors.Errorf("executing api request for user %q to update model metadata: %w", "bob", Two()).Trace()
}
func Two() error {
return errors.Errorf("performing model metadata update: %w", Three()).Trace()
}
func Three() error {
return errors.Errorf("model %q constraint failed precondition %q", uuid, precondition).Trace()
}
err := One()
fmt.Println(One())
// executing api request for user "bob" to update model metadata: performing model metadata update: model "123" constraint failed precondition "some-key"
fmt.Println(ErrorStack(err))
// github.com/juju/juju/mypackage/test.go:10: executing api request for user "bob" to update model metadata: performing model metadata update: model "123" constraint failed precondition "some-key"
// github.com/juju/juju/mypackage/test.go:15: performing model metadata update: model "123" constraint failed precondition "some-key"
// github.com/juju/juju/mypackage/test.go:20: model "123" constraint failed precondition "some-key"From the example above you can see that tracing is not performed automatically like it is for most calls to the current errors library. It would be an explicit decision by the user. Forcing the error to first be enriched makes us as developers add more context to the errors first removing one of the main use cases for Trace().
The new ideas demonstrated above are discussed more below. The new approach is not perfect in every aspect and will produce much more verbose error strings.
Core Idea
With this new design I would like to have it focus around two central types.
Const Error
This is the same new type introduced in v1 with no additional changes. The following properties will still remain true about the error.
type ConstError string
const ErrorDoohickey = ConstError("doohickey failure")
const ErrorOther = ConstError("doohickey failure")
const ErrorNoMatch = ConstError("no match for doohickey")
fmt.Println(ErrorDoohickey == ErrOther)
// True
fmt.Println(ErrorNoMatch == ErrOther)
// False
newErr := fmt.Errorf("foo bar %w", ErrorDoohickey)
fmt.Println(newErr)
// foo bar doohickey failure
fmt.Println(errors.Is(newErr, ErrDoohickey))
// True
fmt.Println(errors.Is(newErr, ErrNoMatch))
// FalseError
A new type introduced into this library and not to be confused with any existing Error types that exist in v1. The idea of this type is to introduce a builder pattern where by once you have a variable of this type you can further build on the error.
Errors are to be formed in chains like err1 -> err2 -> err3 where each new error in the chain is offering a new piece of information to help the end user make better decisions.
The following type Error is proposed.
// Error represents a Go error generated by this library that can further be built on. All Errors are immutable.
type Error interface {
// error represents the Go error encapsulated by this type and makes clear that
// Error conforms to the error interface as well.
error
// Add adds the error to this error returning an error chain that satisfies the
// added error. The newly constructed chain will be origError -> addedError. The
// new Error returned by add does not change the original error.Error() message.
// This is useful in cases where the producer of the error would like the error to
// satisfy some existing error type but not have the added errors Error() string
// pollute the message of the original error.
// In v1 of this library this functionality was similar to errors.Hide()
//
// Example:
// const ErrorInvalid = ConstError("invalid operation")
//
// e := errors.Errorf("user id %q is not understood", userId).Add(ErrorInvalid)
//
// fmt.Println(e.Error())
// // user id "bluey" is not understood
//
// fmt.Println(errors.Is(e, ErrorInvalid))
// // True
Add(error) Error
// Trace returns a new error indicating the source code file and line number where
// the Error.Trace() function was called from.
// See ErrorStack for usage.
Trace() Error
// Unwrap implements stderrors.Unwrap().
Unwrap() error
}Deliberate design has been made in designing Error so that it cannot be directly constructed by the end user and can only be obtained by using the global functions of this library. At the moment this new design is pushing the idea that errors should be handled an enriched instead of just annotated with debugging information and passed up the stack blindly.
Static Functions
This section documents the Global static functions on the package and their purpose.
std errors
The following new functions will be added to the library to maintain comptability with std errors.
Join adds the currently missing std errors.Join() function, altering it to now return the Error type offered by this package so the error can be further enriched.
// Join returns an error that wraps the given errors. Any nil error values are
// discarded. Join returns nil if every value in errs is nil. The error formats as
// the concatenation of the strings obtained by calling the Error method of each
// element of errs, with a newline between each string.
//
// A non-nil error returned by Join implements the Unwrap() []error method.
func Join(errs ...error) ErrorThe remaining functions already exist in this package with proposed changes indicated.
// As is a 1:1 implementation of errors.As
func As(error, any) bool
// Is is a 1:1 implementation of errors.Is
func Is(error, error) bool
// New is a 1:1 implementation of errors.New with the difference being that a
// Error is returned as the resulting type so that the error can further be
// enriched.
//
// Changes from v1: New will no longer return either a caused based error or
// a error that has been traced. If the caller wishes to trace the error they
// can perform New("my error").Trace()
func New(string) Error
// Unwrap is a 1:1 implementation of errors.Unwrap
func Unwrap(error) errorstd errors Helpers
In v1 of this library we introduced the following std errors helpers to extend the functionality. This library will retain their functionality.
AsType
// AsType finds the first error in err's chain that is assignable to type T, and if a match is found, returns that error value and true. Otherwise, it returns T's zero value and false.
// AsType is equivalent to errors.As, but uses a type parameter and returns the target, to avoid having to define a variable before the call.
func AsType[T error](err error) (T, bool)HasType
// HasType is a function wrapper around AsType dropping the where return value from AsType().
func HasType[T error](err error) boolfmt errors
The following fmt functions will be included in the set of static functions offered by this package.
// Errorf is a 1:1 implementation of fmt.Errorf with the difference being that
// Error is returned as the resulting type so that the error can further be
// enriched.
//
// Changes from v1: Errorf will no longer return either a caused based error
// or a error that has been traced. If the caller wishes to trace the error they
// can perform Errorf("id %q not found", id).Trace()
func Errorf(string, ...any) ErrorTracing errors
To maintain tracing support in the library we will maintain the following functions.
// ErrorStack returns a string with a new line for each error in the chain. If the error contains tracing information that will also be printed with the error.
func ErrorStack(error) stringRemovals
The following sections list the types and functions I would like to see removed from this package. With each removal a justification has been provided.
Annotation Funcs
The following annotation functions should be removed with justification.
// Wrap* based functions are confusing in their signature and offer no real
// added benefit. They can be directly replaced with calls to Errorf() using
// %w masks for the format string or combined using Join(). By not using Wrap
// we avoid the potential pitfuls of Caused based errors currently introduced
// Wrap.
func Wrap(other, newDescriptive error) error
func Wrapf(other, newDescriptive error, format string, args ...interface{}) error// WithType was introduced as part of the support for the new std errors
// package in go. It offered a way to maintain an already established error
// but now make it also satisfy Is for a ConstError type without introducing
// a tree structure of error chains.
//
// We don't have this limitation anymore and can safely use the new Errors.Add()
// method for adding new error types into the chain.
func WithType(error, ConstError) error// Maskf is used to produce a new error with the format string and args
// hiding the original error from both the error.Error() string and the
// error type. This resultant error needs Cause() called on it to unpack
// the hidden error.
//
// We have 2 uses of this func inside of Juju and they can be replaced with
// the same effect using Errorf("", args...).Add(otherError)
func Maskf(error, string, ...any) error// Hide was introduced as part of the upgrade to std errors. It's purpose
// was to hide printing of an errors Error message in calls to fmt.Errorf
// but still have the error available in the error chain so calls to Is
// and As still worked.
//
// These calls can be replaced with Errors.Add() for the same effect now.
func Hide(error) errorIsError funcs
All of the Is*() error of this type functions. They have been deprecated for a while now and we have now moved off using them in the latest versions of Juju. All of these calls should now be aligned on to the go std errors pkg error.Is() idiom.
func IsAlreadyExists(err error) bool {...}
func IsBadRequest(err error) bool {}
func IsForbidden(err error) bool
func IsMethodNotAllowed(err error) bool
func IsNotAssigned(err error) bool
func IsNotFound(err error) bool
func IsNotImplemented(err error) bool
func IsNotProvisioned(err error) bool
func IsNotSupported(err error) bool
func IsNotValid(err error) bool
func IsNotYetAvailable(err error) bool
func IsQuotaLimitExceeded(err error) bool
func IsTimeout(err error) bool
func IsUnauthorized(err error) bool
func IsUserNotFound(err error) boolError Type Codes
Currently this library has a pre-existing list of common error types that we offer to the user to communicate common error scenarios. For example errors.NotFound, largely these are both based off of common HTTP status codes and Juju's own requirements.
From my point of view I think they offer the following problems:
- They're specific to a single domain, being that of Juju
- They are far too wide in the scope they describe. For example if you returned a NotFound error from a function what specifically wasn't found? That is to say we may want to still check if an error is a type of
NotFounderror but I think we should be actively moving away from describing problems with such high level types and opting for usage of package level errors.
What I would like to propose is the following:
Deprecate The Following Types in V1
This would see the following types and functions deprecated in v1 of this library.
const (
Timeout = ConstError("timeout")
NotFound = ConstError("not found")
UserNotFound = ConstError("user not found")
Unauthorized = ConstError("unauthorized")
NotImplemented = ConstError("not implemented")
AlreadyExists = ConstError("already exists")
NotSupported = ConstError("not supported")
NotValid = ConstError("not valid")
NotProvisioned = ConstError("not provisioned")
NotAssigned = ConstError("not assigned")
BadRequest = ConstError("bad request")
MethodNotAllowed = ConstError("method not allowed")
Forbidden = ConstError("forbidden")
QuotaLimitExceeded = ConstError("quota limit exceeded")
NotYetAvailable = ConstError("not yet available")
)
func AlreadyExistsf(format string, args ...interface{}) error
func BadRequestf(format string, args ...interface{}) error
func Forbiddenf(format string, args ...interface{}) error
func MethodNotAllowedf(format string, args ...interface{}) error
func NewAlreadyExists(err error, msg string) error
func NewBadRequest(err error, msg string) error
func NewForbidden(err error, msg string) error
func NewMethodNotAllowed(err error, msg string) error
func NewNotAssigned(err error, msg string) error
func NewNotFound(err error, msg string) error
func NewNotImplemented(err error, msg string) error
func NewNotProvisioned(err error, msg string) error
func NewNotSupported(err error, msg string) error
func NewNotValid(err error, msg string) error
func NewNotYetAvailable(err error, msg string) error
func NewQuotaLimitExceeded(err error, msg string) error
func NewTimeout(err error, msg string) error
func NewUnauthorized(err error, msg string) error
func NewUserNotFound(err error, msg string) error
func NotAssignedf(format string, args ...interface{}) error
func NotFoundf(format string, args ...interface{}) error
func NotImplementedf(format string, args ...interface{}) error
func NotProvisionedf(format string, args ...interface{}) error
func NotSupportedf(format string, args ...interface{}) error
func NotValidf(format string, args ...interface{}) error
func NotYetAvailablef(format string, args ...interface{}) error
func QuotaLimitExceededf(format string, args ...interface{}) error
func Timeoutf(format string, args ...interface{}) error
func Trace(other error) error
func Unauthorizedf(format string, args ...interface{}) error
func Unwrap(err error) error
func UserNotFoundf(format string, args ...interface{}) errorReimplement The Deprecated Types
With the types above being deprecated we would move them into the Juju code based under core/errors. To aid in the transition these types can be set to their deprecated counter parts from the juju/errors package.
const (
Timeout = errors.Timeout
...
)
var (
AlreadyExistsf = errors.AlreadyExistsf
NewAlreadyExists = errors.NewAlreadyExists
...
)Transition
With breaking changes being introduced we need a way to transition to the v2 errors module should this proposal be adopted. My current plan for doing this is as follows:
- Introduce
errors/v2changes injujumain only. Our older branches for released versions can stay onerrors/v1. We will have to take on some associated merge forward overhead if thejujumain branch ever fully cuts off oferrors/v1. - Have both
github.com/juju/errorsandgithub.com/juju/errors/v2modules imports in ourgo.modfile. All new code written should be written againstv2and when modifying the errors of an existing file the author should take the time to move that file over to the new package where it makes sense. - We would need to transition all of our libraries that rely on
juju/errorsover tov2. The main hassle here based on the proposal is library usage of ConstErrors defined inv1such asNotFoundorTimeout. Ideally we would force our libraries to make public const errors that describe their own errors and move Juju over to using domain specific errors from the respective library. As an example this would be a common case we have today:The first way we can aid this transition is to make our libaries return errors that satisfy both the original error value and the new const errors defined by the library. This would allow us to not have major version bumps in our libraries for the change. Example:// Library code func MyLibraryFuncCheckThingX() error { // ... return errors.NotFound } // Juju Code err := MyLibraryFuncCheckThingX() fmt.Println(errors.Is(err, errors.NotFound)) // True
The second approach is perform a wholesale cut over to library based errors and bump the major version number of each library. The down side to this approach is bug fixes to our libraries will need to be integrated into multiple branches and forward merges will be a bit painful for new uses of any libraries. Example:const ErrorThingNotFound = errors.ConstError("thing not found") func MyLibraryFuncCheckThingX() error { // ... return errors.Errorf("%w some id", ErrorThingNotFound).Add(errorsv1.NotFound) } // Juju Code err := MyLibraryFuncCheckThingX() fmt.Println(errors.Is(err, errors.NotFound)) // True fmt.Println(errors.Is(err, mylib.ErrorThingNotFound)) // True
const ErrorThingNotFound = errors.ConstError("thing not found") func MyLibraryFuncCheckThingX() error { // ... return errors.Errorf("%w some id", ErrorThingNotFound) } // Juju Code err := MyLibraryFuncCheckThingX() fmt.Println(errors.Is(err, errors.NotFound)) // False fmt.Println(errors.Is(err, mylib.ErrorThingNotFound)) // True
Proposed New V2 Interface
The following is the proposed complete interface for the new v2 package:
type ConstError string
func (c ConstError) Error() string {}
type Error interface {
error
Add(error) Error
Trace() Error
Unwrap() error
}
func As(error, any) bool
func AsType[T error](err error) (T, bool) {}
func Errorf(string, ...any) Error {}
func ErrorStack(error) string {}
func HasType[T error](err error) bool {}
func Is(error, error) bool {}
func New(string) Error {}
func Join(errs ...error) Error {}
func Unwrap(error) error {}