-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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.
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
chore: remove cockroach db errors #20386
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,11 +1,11 @@ | ||||||
package depinject | ||||||
|
||||||
import ( | ||||||
"fmt" | ||||||
"reflect" | ||||||
"strings" | ||||||
"unicode" | ||||||
|
||||||
"github.com/cockroachdb/errors" | ||||||
"golang.org/x/exp/slices" | ||||||
) | ||||||
|
||||||
|
@@ -21,12 +21,12 @@ func isExportedType(typ reflect.Type) error { | |||||
pkgPath := typ.PkgPath() | ||||||
if name != "" && pkgPath != "" { | ||||||
if unicode.IsLower([]rune(name)[0]) { | ||||||
return errors.Errorf("type must be exported: %s", typ) | ||||||
return fmt.Errorf("type must be exported: %s", typ) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use To wrap errors correctly and preserve the original error context, use - return fmt.Errorf("type must be exported: %s", typ)
+ return fmt.Errorf("type must be exported: %w", typ) Committable suggestion
Suggested change
|
||||||
} | ||||||
|
||||||
pkgParts := strings.Split(pkgPath, "/") | ||||||
if slices.Contains(pkgParts, "internal") { | ||||||
return errors.Errorf("type must not come from an internal package: %s", typ) | ||||||
return fmt.Errorf("type must not come from an internal package: %s", typ) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use To wrap errors correctly and preserve the original error context, use - return fmt.Errorf("type must not come from an internal package: %s", typ)
+ return fmt.Errorf("type must not come from an internal package: %w", typ) Committable suggestion
Suggested change
|
||||||
} | ||||||
|
||||||
return nil | ||||||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -1,9 +1,10 @@ | ||||||||||
package depinject | ||||||||||
|
||||||||||
import ( | ||||||||||
"errors" | ||||||||||
"fmt" | ||||||||||
Comment on lines
+4
to
+5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use consistent import alias for - "errors"
+ pkgerrors "github.com/pkg/errors" Committable suggestion
Suggested change
|
||||||||||
"reflect" | ||||||||||
|
||||||||||
"github.com/cockroachdb/errors" | ||||||||||
"runtime" | ||||||||||
|
||||||||||
) | ||||||||||
|
||||||||||
// Config is a functional configuration of a container. | ||||||||||
|
@@ -33,7 +34,7 @@ | |||||||||
func ProvideInModule(moduleName string, providers ...interface{}) Config { | ||||||||||
return containerConfig(func(ctr *container) error { | ||||||||||
if moduleName == "" { | ||||||||||
return errors.Errorf("expected non-empty module name") | ||||||||||
return errors.New("expected non-empty module name") | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider providing more context in the error message. - return errors.New("expected non-empty module name")
+ return errors.New("ProvideInModule: expected non-empty module name") Committable suggestion
Suggested change
|
||||||||||
} | ||||||||||
|
||||||||||
return provide(ctr, ctr.moduleKeyContext.createOrGetModuleKey(moduleName), providers) | ||||||||||
|
@@ -44,11 +45,11 @@ | |||||||||
for _, c := range providers { | ||||||||||
rc, err := extractProviderDescriptor(c) | ||||||||||
if err != nil { | ||||||||||
return errors.WithStack(err) | ||||||||||
return fmt.Errorf("%w\n%s", err, getStackTrace()) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid including stack trace in error messages; use - return fmt.Errorf("%w\n%s", err, getStackTrace())
+ return pkgerrors.WithStack(err) Committable suggestion
Suggested change
|
||||||||||
} | ||||||||||
_, err = ctr.addNode(&rc, key) | ||||||||||
if err != nil { | ||||||||||
return errors.WithStack(err) | ||||||||||
return fmt.Errorf("%w\n%s", err, getStackTrace()) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid including stack trace in error messages; use - return fmt.Errorf("%w\n%s", err, getStackTrace())
+ return pkgerrors.WithStack(err) Committable suggestion
Suggested change
|
||||||||||
} | ||||||||||
} | ||||||||||
return nil | ||||||||||
|
@@ -80,7 +81,7 @@ | |||||||||
func InvokeInModule(moduleName string, invokers ...interface{}) Config { | ||||||||||
return containerConfig(func(ctr *container) error { | ||||||||||
if moduleName == "" { | ||||||||||
return errors.Errorf("expected non-empty module name") | ||||||||||
return errors.New("expected non-empty module name") | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider providing more context in the error message. - return errors.New("expected non-empty module name")
+ return errors.New("InvokeInModule: expected non-empty module name") Committable suggestion
Suggested change
|
||||||||||
} | ||||||||||
|
||||||||||
return invoke(ctr, ctr.moduleKeyContext.createOrGetModuleKey(moduleName), invokers) | ||||||||||
|
@@ -91,7 +92,7 @@ | |||||||||
for _, c := range invokers { | ||||||||||
rc, err := extractInvokerDescriptor(c) | ||||||||||
if err != nil { | ||||||||||
return errors.WithStack(err) | ||||||||||
return fmt.Errorf("%w\n%s", err, getStackTrace()) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid including stack trace in error messages; use - return fmt.Errorf("%w\n%s", err, getStackTrace())
+ return pkgerrors.WithStack(err) Committable suggestion
Suggested change
|
||||||||||
} | ||||||||||
err = ctr.addInvoker(&rc, key) | ||||||||||
if err != nil { | ||||||||||
|
@@ -151,7 +152,7 @@ | |||||||||
for _, v := range values { | ||||||||||
err := ctr.supply(reflect.ValueOf(v), loc) | ||||||||||
if err != nil { | ||||||||||
return errors.WithStack(err) | ||||||||||
return fmt.Errorf("%w\n%s", err, getStackTrace()) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid including stack trace in error messages; use - return fmt.Errorf("%w\n%s", err, getStackTrace())
+ return pkgerrors.WithStack(err) Committable suggestion
Suggested change
|
||||||||||
} | ||||||||||
} | ||||||||||
return nil | ||||||||||
|
@@ -162,7 +163,7 @@ | |||||||||
// fail immediately. | ||||||||||
func Error(err error) Config { | ||||||||||
return containerConfig(func(*container) error { | ||||||||||
return errors.WithStack(err) | ||||||||||
return fmt.Errorf("%w\n%s", err, getStackTrace()) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid including stack trace in error messages; use return fmt.Errorf("%w\n%s", err, getStackTrace())
+ return pkgerrors.WithStack(err) Committable suggestion
Suggested change
|
||||||||||
}) | ||||||||||
} | ||||||||||
|
||||||||||
|
@@ -172,7 +173,7 @@ | |||||||||
for _, opt := range opts { | ||||||||||
err := opt.apply(ctr) | ||||||||||
if err != nil { | ||||||||||
return errors.WithStack(err) | ||||||||||
return fmt.Errorf("%w\n%s", err, getStackTrace()) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid including stack trace in error messages; use return fmt.Errorf("%w\n%s", err, getStackTrace())
+ return pkgerrors.WithStack(err) Committable suggestion
Suggested change
|
||||||||||
} | ||||||||||
} | ||||||||||
return nil | ||||||||||
|
@@ -186,3 +187,9 @@ | |||||||||
} | ||||||||||
|
||||||||||
var _ Config = (*containerConfig)(nil) | ||||||||||
|
||||||||||
func getStackTrace() string { | ||||||||||
var stack [4096]byte | ||||||||||
n := runtime.Stack(stack[:], false) | ||||||||||
return string(stack[:n]) | ||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -3,8 +3,6 @@ package depinject | |||||||||||
import ( | ||||||||||||
"fmt" | ||||||||||||
"reflect" | ||||||||||||
|
||||||||||||
"github.com/cockroachdb/errors" | ||||||||||||
) | ||||||||||||
|
||||||||||||
// ErrMultipleImplicitInterfaceBindings defines an error condition where an attempt was made to implicitly bind | ||||||||||||
|
@@ -63,6 +61,6 @@ func (err ErrNoTypeForExplicitBindingFound) Error() string { | |||||||||||
} | ||||||||||||
|
||||||||||||
func duplicateDefinitionError(typ reflect.Type, duplicateLoc Location, existingLoc string) error { | ||||||||||||
return errors.Errorf("duplicate provision of type %v by %s\n\talready provided by %s", | ||||||||||||
return fmt.Errorf("duplicate provision of type %v by %s\n\talready provided by %s", | ||||||||||||
typ, duplicateLoc, existingLoc) | ||||||||||||
} | ||||||||||||
Comment on lines
+64
to
66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use To wrap errors correctly and preserve the original error context, use - return fmt.Errorf("duplicate provision of type %v by %s\n\talready provided by %s",
- typ, duplicateLoc, existingLoc)
+ return fmt.Errorf("duplicate provision of type %v by %s\n\talready provided by %w",
+ typ, duplicateLoc, existingLoc) Committable suggestion
Suggested change
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use
%w
for error wrapping.To wrap errors correctly and preserve the original error context, use
%w
instead of%v
infmt.Errorf
.Committable suggestion