Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Satisfy errors.Is even if formatting is incorrect #61

Open
wants to merge 2 commits into
base: v1
Choose a base branch
from
+51 −22
Open

Satisfy errors.Is even if formatting is incorrect #61

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4cc5e15
Satisfy errors.Is even if formatting is incorrect
barrettj12 Sep 29, 2023
0593520
Test suffix still present w/ incorrect fmt string
barrettj12 Sep 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 25 additions & 22 deletions errortypes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@
package errors

import (
"errors"
stderror "errors"
"fmt"
"strings"
)

// a ConstError is a prototype for a certain type of error
Expand Down Expand Up @@ -86,12 +84,17 @@ func wrapErrorWithMsg(err error, msg string) error {
return fmt.Errorf("%s: %w", msg, err)
}

func makeWrappedConstError(err error, format string, args ...interface{}) error {
separator := " "
if err.Error() == "" || errors.Is(err, &fmtNoop{}) {
separator = ""
// fmtErrWithType returns an error with the provided error type and format.
func fmtErrWithType(errType ConstError, hideSuffix bool, format string, args ...interface{}) error {
msg := fmt.Sprintf(format, args...)
if !hideSuffix {
msg += " " + errType.Error()
}

return &errWithType{
error: New(msg),
errType: errType,
}
return fmt.Errorf(strings.Join([]string{format, "%w"}, separator), append(args, err)...)
}

// WithType is responsible for annotating an already existing error so that it
Expand All @@ -116,7 +119,7 @@ func WithType(err error, errType ConstError) error {
// interface.
func Timeoutf(format string, args ...interface{}) error {
return newLocationError(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the docs from go tool vet we should do this:

func Timeoutf(format string, args ...interface{}) error {
	if false {
		_ = fmt.Sprintf(format, args...) // enable printf checking
	}
	return newLocationError(
		makeWrappedConstError(Timeout, format, args...),
		1,
	)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do one better, and put this check inside makeWrappedConstError.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only reason I put it at the exported level was to save the linter from recursing further than it needs.

makeWrappedConstError(Timeout, format, args...),
fmtErrWithType(Timeout, false, format, args...),
1,
)
}
Expand All @@ -140,7 +143,7 @@ func IsTimeout(err error) bool {
// Locationer interface.
func NotFoundf(format string, args ...interface{}) error {
return newLocationError(
makeWrappedConstError(NotFound, format, args...),
fmtErrWithType(NotFound, false, format, args...),
1,
)
}
Expand All @@ -164,7 +167,7 @@ func IsNotFound(err error) bool {
// Locationer interface.
func UserNotFoundf(format string, args ...interface{}) error {
return newLocationError(
makeWrappedConstError(UserNotFound, format, args...),
fmtErrWithType(UserNotFound, false, format, args...),
1,
)
}
Expand All @@ -188,7 +191,7 @@ func IsUserNotFound(err error) bool {
// the Locationer interface.
func Unauthorizedf(format string, args ...interface{}) error {
return newLocationError(
makeWrappedConstError(Hide(Unauthorized), format, args...),
fmtErrWithType(Unauthorized, true, format, args...),
1,
)
}
Expand All @@ -212,7 +215,7 @@ func IsUnauthorized(err error) bool {
// the Locationer interface.
func NotImplementedf(format string, args ...interface{}) error {
return newLocationError(
makeWrappedConstError(NotImplemented, format, args...),
fmtErrWithType(NotImplemented, false, format, args...),
1,
)
}
Expand All @@ -236,7 +239,7 @@ func IsNotImplemented(err error) bool {
// the Locationer interface.
func AlreadyExistsf(format string, args ...interface{}) error {
return newLocationError(
makeWrappedConstError(AlreadyExists, format, args...),
fmtErrWithType(AlreadyExists, false, format, args...),
1,
)
}
Expand All @@ -260,7 +263,7 @@ func IsAlreadyExists(err error) bool {
// Locationer interface.
func NotSupportedf(format string, args ...interface{}) error {
return newLocationError(
makeWrappedConstError(NotSupported, format, args...),
fmtErrWithType(NotSupported, false, format, args...),
1,
)
}
Expand All @@ -284,7 +287,7 @@ func IsNotSupported(err error) bool {
// Locationer interface.
func NotValidf(format string, args ...interface{}) error {
return newLocationError(
makeWrappedConstError(NotValid, format, args...),
fmtErrWithType(NotValid, false, format, args...),
1,
)
}
Expand All @@ -308,7 +311,7 @@ func IsNotValid(err error) bool {
// the Locationer interface.
func NotProvisionedf(format string, args ...interface{}) error {
return newLocationError(
makeWrappedConstError(NotProvisioned, format, args...),
fmtErrWithType(NotProvisioned, false, format, args...),
1,
)
}
Expand All @@ -332,7 +335,7 @@ func IsNotProvisioned(err error) bool {
// Locationer interface.
func NotAssignedf(format string, args ...interface{}) error {
return newLocationError(
makeWrappedConstError(NotAssigned, format, args...),
fmtErrWithType(NotAssigned, false, format, args...),
1,
)
}
Expand All @@ -356,7 +359,7 @@ func IsNotAssigned(err error) bool {
// Locationer interface.
func BadRequestf(format string, args ...interface{}) error {
return newLocationError(
makeWrappedConstError(Hide(BadRequest), format, args...),
fmtErrWithType(BadRequest, true, format, args...),
1,
)
}
Expand All @@ -380,7 +383,7 @@ func IsBadRequest(err error) bool {
// and the Locationer interface.
func MethodNotAllowedf(format string, args ...interface{}) error {
return newLocationError(
makeWrappedConstError(Hide(MethodNotAllowed), format, args...),
fmtErrWithType(MethodNotAllowed, true, format, args...),
1,
)
}
Expand All @@ -404,7 +407,7 @@ func IsMethodNotAllowed(err error) bool {
// Locationer interface.
func Forbiddenf(format string, args ...interface{}) error {
return newLocationError(
makeWrappedConstError(Hide(Forbidden), format, args...),
fmtErrWithType(Forbidden, true, format, args...),
1,
)
}
Expand All @@ -428,7 +431,7 @@ func IsForbidden(err error) bool {
// Is(err, QuotaLimitExceeded) and the Locationer interface.
func QuotaLimitExceededf(format string, args ...interface{}) error {
return newLocationError(
makeWrappedConstError(Hide(QuotaLimitExceeded), format, args...),
fmtErrWithType(QuotaLimitExceeded, true, format, args...),
1,
)
}
Expand All @@ -452,7 +455,7 @@ func IsQuotaLimitExceeded(err error) bool {
// and the Locationer interface.
func NotYetAvailablef(format string, args ...interface{}) error {
return newLocationError(
makeWrappedConstError(Hide(NotYetAvailable), format, args...),
fmtErrWithType(NotYetAvailable, true, format, args...),
1,
)
}
Expand Down
26 changes: 26 additions & 0 deletions errortypes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,3 +193,29 @@ func (*errorTypeSuite) TestWithType(c *gc.C) {
c.Assert(err.Error(), gc.Equals, "yes")
c.Assert(errors.Is(err, myErr2), gc.Equals, false)
}

func (*errorTypeSuite) TestBadFormatNotEnoughArgs(c *gc.C) {
errorTests := make([]errorTest, 0, len(allErrors))
for _, errInfo := range allErrors {
errorTests = append(errorTests, errorTest{
errInfo.argsConstructor("missing arg %v"),
".*" + errInfo.suffix,
errInfo,
})
}

runErrorTests(c, errorTests, true)
}

func (*errorTypeSuite) TestBadFormatTooManyArgs(c *gc.C) {
errorTests := make([]errorTest, 0, len(allErrors))
for _, errInfo := range allErrors {
errorTests = append(errorTests, errorTest{
errInfo.argsConstructor("extra arg %v", "foo", "bar"),
".*" + errInfo.suffix,
errInfo,
})
}

runErrorTests(c, errorTests, true)
}
Loading