You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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:
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:
funcOne() error {
returnerrors.Trace(Two())
}
funcTwo() error {
returnerrors.Trace(Three())
}
funcThree() error {
returnerrors.New("some error")
}
err:=One()
fmt.Println(err)
// some errorfmt.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 error
The 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:
funcOne() error {
returnerrors.Errorf("executing api request for user %q to update model metadata: %w", "bob", Two()).Trace()
}
funcTwo() error {
returnerrors.Errorf("performing model metadata update: %w", Three()).Trace()
}
funcThree() error {
returnerrors.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.
typeConstErrorstringconstErrorDoohickey=ConstError("doohickey failure")
constErrorOther=ConstError("doohickey failure")
constErrorNoMatch=ConstError("no match for doohickey")
fmt.Println(ErrorDoohickey==ErrOther)
// Truefmt.Println(ErrorNoMatch==ErrOther)
// FalsenewErr:=fmt.Errorf("foo bar %w", ErrorDoohickey)
fmt.Println(newErr)
// foo bar doohickey failurefmt.Println(errors.Is(newErr, ErrDoohickey))
// Truefmt.Println(errors.Is(newErr, ErrNoMatch))
// False
Error
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.typeErrorinterface {
// 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))// // TrueAdd(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.funcJoin(errs...error) Error
The remaining functions already exist in this package with proposed changes indicated.
// As is a 1:1 implementation of errors.AsfuncAs(error, any) bool// Is is a 1:1 implementation of errors.IsfuncIs(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()funcNew(string) Error// Unwrap is a 1:1 implementation of errors.UnwrapfuncUnwrap(error) error
std 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.funcAsType[Terror](errerror) (T, bool)
HasType
// HasType is a function wrapper around AsType dropping the where return value from AsType().funcHasType[Terror](errerror) bool
fmt 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()funcErrorf(string, ...any) Error
Tracing 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.funcErrorStack(error) string
Removals
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.funcWrap(other, newDescriptiveerror) errorfuncWrapf(other, newDescriptiveerror, formatstring, 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.funcWithType(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)funcMaskf(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.funcHide(error) error
IsError 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.
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 NotFound error 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.
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.AlreadyExistsfNewAlreadyExists=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/v2 changes in juju main only. Our older branches for released versions can stay on errors/v1. We will have to take on some associated merge forward overhead if the juju main branch ever fully cuts off of errors/v1.
Have both github.com/juju/errors and github.com/juju/errors/v2 modules imports in our go.mod file. All new code written should be written against v2 and 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/errors over to v2. The main hassle here based on the proposal is library usage of ConstErrors defined in v1 such as NotFound or Timeout. 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:
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:
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 stderrors
.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:
If both
operation1
andoperation2
return errors that satisfyerrors.NotFound
then how do we know specifically what wasn't found? As opposed to: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
andTimeout
moved into a new package inJuju
ascore/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:
The 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: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.
Error
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 newerror
in the chain is offering a new piece of information to help the end user make better decisions.The following type
Error
is proposed.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 theError
type offered by this package so the error can be further enriched.The remaining functions already exist in this package with proposed changes indicated.
std 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
HasType
fmt errors
The following
fmt
functions will be included in the set of static functions offered by this package.Tracing errors
To maintain tracing support in the library we will maintain the following functions.
Removals
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.
IsError 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 pkgerror.Is()
idiom.Error 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:
NotFound
error 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.
Reimplement 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 thejuju/errors
package.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:
errors/v2
changes injuju
main only. Our older branches for released versions can stay onerrors/v1
. We will have to take on some associated merge forward overhead if thejuju
main branch ever fully cuts off oferrors/v1
.github.com/juju/errors
andgithub.com/juju/errors/v2
modules imports in ourgo.mod
file. All new code written should be written againstv2
and 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.juju/errors
over tov2
. The main hassle here based on the proposal is library usage of ConstErrors defined inv1
such asNotFound
orTimeout
. 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:Proposed New V2 Interface
The following is the proposed complete interface for the new v2 package:
The text was updated successfully, but these errors were encountered: