Error returned from external package is unwrapped

I’ve released a Go linter to check for the rule described in this post. You can
find it here: wrapcheck

This is a small post covering error wrapping in Go that will build on the
work from Dave Cheney.

So recently, after spending more time than I should hunting down the source of
an error, I sparked the discussion within my team at work about how we can cut
down on the time to debug, specifically, by making sure that errors aren’t
handed back to the caller without forgetting to attach extra information.

We already have a pretty good practice in place to wrap errors with additional
info before we hand them up the stack, however this was missing in this
particular case.

Essentially, we were seeing something akin to:

time="2020-08-04T11:36:27+02:00" level=error error="sql: no rows in result set"

…not particularly useful.

To help with debugging here, it’s important that additional information is
attached to the error to give the developer more context.

Extending errors

By now you’ve probably already heard the popular Go proverb of:

Don’t just check errors, handle them gracefully

Handling an error in this case would be to add this additional information and
pass it back up the stack. However, we’re not just limited to that. If we take a
closer look at what an error is in Go, we see the following interface.

type Error interface {
  Error() string
}

Very simple, yet very powerful. However this power comes with many
possibilities, and unfortunately many possibilities comes with no standard way
of handling things. Ultimately, it is up to you to define what form of extension
is most suitable for your application.

An example is in Rob Pike’s Upspin project. They use a custom error struct which looks
like:

type Error struct {
  Path upspin.PathName
  User upspin.UserName
  Op   Op
  Kind Kind
  Err  error
}

You can read more about how exactly they use this error definition in a blog
post they
published.

This is a bit intensive for many programs however, so we’ll take a look at a
less intense, more widely used method for adding information to errors, called
wrapping.

Wrapping errors prior to Go 1.13

Go has had error wrapping since the introduction of the
errors package was introduced in
early 2016.

This shortly followed with the errors.[New|Errorf|Wrap|Wrapf] methods
implementing the interface:

type Stack interface {
  Stack() []uintptr
}

The functionality for adding stack traces to existing errors came with the
introduction of the
errors.WithStack()
method on the same package.

This returns an error with a format method which will print the stack trace when
used with the %+v verb.

import "github.com/pkg/errors"

...

func (db *DB) getTansactionByID(tranID string) (Transaction, error) {
  sql := `SELECT * FROM transaction WHERE id = $1;`

  var t Transaction
  if err := db.conn.Get(&t, sql, tranID); err != nil {
    return Transaction{}, errors.Wrap(err, "failed to get transaction")
  }

  return t, nil
}

...

Which, when logging the error with the %+v verb, produces something along the
lines of:

sql: no rows in result set
main.main
  /Users/tom/Documents/tmp/main.go:10
runtime.main
  /usr/local/Cellar/go/1.14.6/libexec/src/runtime/proc.go:203
runtime.goexit
  /usr/local/Cellar/go/1.14.6/libexec/src/runtime/asm_amd64.s:1373

We can use this to our advantage when creating errors to include a stack trace
in them. This can then be logged along with the error in order to aid locating
the source of the error.

One downside of this approach is the performance penalty. The necessary call
into runtime.Stack(buf []byte, all bool) int incurs a non-negligible cost, and
should be avoided in hot paths.

Stack traces are also not particularly human friendly. They will only identify
where the error occurred, not why. Therefore, it’s still good to add extra
context to your errors.

Another minor nitpick is that when wrapping errors multiple times with
github.com/pkg/errors, it will leave you with multiple stack traces, one for
each time you wrap the error. This creates a rather large set of logs when you
finally print the error if your stack is deep 🃏.

Wrapping errors in Go 1.13

In version 1.13, Go introduced the %w verb, as well as a few methods on the
errors package to support error identification and incorporate wrapping into
the standard library.

By using fmt.Errorf() with the %w verb, we’re able to enhance errors with
additional information using only the standard library, while allowing them to
remain inspectable should we need to identify the underlying cause of an error.

It’s important to keep in mind however, when wrapping with %w, you are
implicitly exposing the error to consumers of your package. Only use this when
you plan to support the error type and avoid exposing implementation details.
You can use the %v verb instead for non-inspectable errors (no
errors.Is(), errors.As()).

Now having fmt.Errorf() available is good, however if you never use it in your
programs you’re still going to end up with logs which are hard to decipher. Now
you could most certainly wrap every error returned at every point of your
program. However, you will end up with a lot of redundant wraps which don’t add
a whole lot of value and reduce the signal-to-noise ratio in your logs.

Hence, it would be helpful to have a simple set of rules to follow for when
errors should be wrapped. This is by no means an exhaustive set, however should
cover the majority of common cases within most programs.

1. When you have additional context, which you think would be
   useful to give to the developer reading the log. This is important, as rules are
   not infallible.
2. Errors returned from another package. Wrapping these helps
   identify the entry point of the error into your program. e.g.

   ...
   
   func (db *DB) getTansactionByID(tranID string) (Transaction, error) {
     sql := `SELECT * FROM transaction WHERE id = $1;`
   
     var t Transaction
     if err := db.conn.Get(&t, sql, tranID); err != nil {
       return Transaction{}, fmt.Errorf("failed to get transaction with ID %s: %v", tranID, err)
     }
   
     return t, nil
   }
   
   ...

3. Errors returned by interface methods. Interfaces may be
   implemented by a separate package, therefore wrapping the errors here with
   context may be valuable.

This set of rules gives a good starting point for helping you identify where in
your program you should wrap errors. In order to help out with this, I’ve
written a linter to identify areas where your code is not wrapping errors and it
possibly should be.

Wrapcheck

Wrapcheck is a new linter for Go which
I built in order to maintain consistency across a codebase with regards to point
(1) and (2).

For example, the following code which calls into the sql package will be
reported as an error as follows.

func (db *DB) getUserByID(userID string) (User, error) {
  sql := `SELECT * FROM user WHERE id = $1;`

  var u User
  if err := db.conn.Get(&u, sql, userID); err != nil {
    return User{}, err // wrapcheck error: error returned from external package is unwrapped
  }

  return u, nil
}

func (db *DB) getItemByID(itemID string) (Item, error) {
  sql := `SELECT * FROM item WHERE id = $1;`

  var i Item
  if err := db.conn.Get(&i, sql, itemID); err != nil {
    return Item{}, err // wrapcheck error: error returned from external package is unwrapped
  }

  return i, nil
}

The fix is to add wrapping to the errors before returning them. The linter
however is a bit more flexible, and doesn’t mind if you still want to use
pkg/errors. It will be satisfied as long as the error is not returned bare.

The linter was written with the new(ish) go/analysis tooling, which greatly
simplifies AST traversal as well as setup and testing.

The linter has a fair number of tests. You can have a look at all the specified
cases in the
testdata
directory.

Please feel free to use it in your own projects and report any issues you may
come across. As mentioned, the linter is meant to handle gracefully the standard
case, there are most certainly cases which slip past it. But feel free to file
an issue for these as well.

A simple Go linter to check that errors from external packages are wrapped
during return to help identify the error source during debugging.

More detail in this article

Install

Go >= v1.16

$ go install github.com/tomarrell/wrapcheck/v2/cmd/[email protected]

Wrapcheck is also available as part of the golangci-lint meta linter. Docs and
usage instructions are available
here. When used with golangci-lint,
configuration is integrated with the .golangci.yaml file.

Configuration

You can configure wrapcheck by using a .wrapcheck.yaml file in either the
local directory, or in your home directory.

# An array of strings which specify substrings of signatures to ignore. If this
# set, it will override the default set of ignored signatures. You can find the
# default set at the top of ./wrapcheck/wrapcheck.go.
ignoreSigs:
- .Errorf(
- errors.New(
- errors.Unwrap(
- .Wrap(
- .Wrapf(
- .WithMessage(
- .WithMessagef(
- .WithStack(

# An array of strings which specify regular expressions of signatures to ignore.
# This is similar to the ignoreSigs configuration above, but gives slightly more
# flexibility.
ignoreSigRegexps:
- .New.*Error(

# An array of glob patterns which, if any match the package of the function
# returning the error, will skip wrapcheck analysis for this error. This is
# useful for broadly ignoring packages and/or subpackages from wrapcheck
# analysis. There are no defaults for this value.
ignorePackageGlobs:
- encoding/*
- github.com/pkg/*

# ignoreInterfaceRegexps defines a list of regular expressions which, if matched
# to a underlying interface name, will ignore unwrapped errors returned from a
# function whose call is defined on the given interface.
ignoreInterfaceRegexps:
- ^(?i)c(?-i)ach(ing|e)

Usage

To lint all the packages in a program:

$ wrapcheck ./...

Testing

This linter is tested using analysistest, you can view all the test cases
under the testdata directory.

TLDR

If you’ve ever been debugging your Go program, and you’ve seen an error like
this pop up in your logs.

time="2020-08-04T11:36:27+02:00" level=error error="sql: error no rows"

Then you know exactly how painful it can be to hunt down the cause when you have
many methods which looks just like the following:

func (db *DB) getUserByID(userID string) (User, error) {
	sql := `SELECT * FROM user WHERE id = $1;`

	var u User
	if err := db.conn.Get(&u, sql, userID); err != nil {
		return User{}, err // wrapcheck error: error returned from external package is unwrapped
	}

	return u, nil
}

func (db *DB) getItemByID(itemID string) (Item, error) {
	sql := `SELECT * FROM item WHERE id = $1;`

	var i Item
	if err := db.conn.Get(&i, sql, itemID); err != nil {
		return Item{}, err // wrapcheck error: error returned from external package is unwrapped
	}

	return i, nil
}

The problem here is that multiple method calls into the sql package can return
the same error. Therefore, it helps to establish a trace point at the point
where error handing across package boundaries occurs.

To resolve this, simply wrap the error returned by the db.Conn.Get() call.

func (db *DB) getUserByID(userID string) (User, error) {
	sql := `SELECT * FROM user WHERE id = $1;`

	var u User
	if err := db.Conn.Get(&u, sql, userID); err != nil {
		return User{}, fmt.Errorf("failed to get user by ID: %v", err) // No error!
	}

	return u, nil
}

func (db *DB) getItemByID(itemID string) (Item, error) {
	sql := `SELECT * FROM item WHERE id = $1;`

	var i Item
	if err := db.Conn.Get(&i, sql, itemID); err != nil {
		return Item{}, fmt.Errorf("failed to get item by ID: %v", err) // No error!
	}

	return i, nil
}

Now, your logs will be more descriptive, and allow you to easily locate the
source of your errors.

time="2020-08-04T11:36:27+02:00" level=error error="failed to get user by ID: sql: error no rows"

A further step would be to enforce adding stack traces to your errors instead
using
errors.WithStack()
however, enforcing this is out of scope for this linter for now.

Why?

Errors in Go are simple values. They contain no more information about than the
minimum to satisfy the interface:

type Error interface {
  Error() string
}

This is a fantastic feature, but can also be a limitation. Specifically when you
are attempting to identify the source of an error in your program.

As of Go 1.13, error wrapping using fmt.Errorf(...) is the recommend way to
compose errors in Go in order to add additional information.

Errors generated by your own code are usually predictable. However, when you
have a few frequently used libraries (think sqlx for example), you may run
into the dilemma of identifying exactly where in your program these errors are
caused.

In other words, you want a call stack.

This is especially apparent if you are a diligent Gopher and always hand your
errors back up the call stack, logging at the top level.

So how can we solve this?

Solution

Wrapping errors at the call site.

When we call into external libraries which may return an error, we can wrap the
error to add additional information about the call site.

e.g.

...

func (db *DB) createUser(name, email, city string) error {
  sql := `INSERT INTO customer (name, email, city) VALUES ($1, $2, $3);`

  if _, err := tx.Exec(sql, name, email, city); err != nil {
    // %v verb preferred to prevent error becoming part of external API
    return fmt.Errorf("failed to insert user: %v", err)
  }

  return nil
}

...

This solution allows you to add context which will be handed to the caller,
making identifying the source easier during debugging.

Contributing

As with most static analysis tools, this linter will likely miss some obscure
cases. If you come across a case which you think should be covered and isn’t,
please file an issue including a minimum reproducible example of the case.

License

This project is licensed under the MIT license. See the LICENSE file for more
details.

Tom’s Blog

I’ve released a Go linter to check for the rule described in this post. You can find it here: wrapcheck

This is a small post covering error wrapping in Go that will build on the work from Dave Cheney.

So recently, after spending more time than I should hunting down the source of an error, I sparked the discussion within my team at work about how we can cut down on the time to debug, specifically, by making sure that errors aren’t handed back to the caller without forgetting to attach extra information.

We already have a pretty good practice in place to wrap errors with additional info before we hand them up the stack, however this was missing in this particular case.

Essentially, we were seeing something akin to:

…not particularly useful.

To help with debugging here, it’s important that additional information is attached to the error to give the developer more context.

Extending errors

By now you’ve probably already heard the popular Go proverb of:

Don’t just check errors, handle them gracefully

Handling an error in this case would be to add this additional information and pass it back up the stack. However, we’re not just limited to that. If we take a closer look at what an error is in Go, we see the following interface.

Very simple, yet very powerful. However this power comes with many possibilities, and unfortunately many possibilities comes with no standard way of handling things. Ultimately, it is up to you to define what form of extension is most suitable for your application.

An example is in Rob Pike’s Upspin project. They use a custom error struct which looks like:

You can read more about how exactly they use this error definition in a blog post they published.

This is a bit intensive for many programs however, so we’ll take a look at a less intense, more widely used method for adding information to errors, called wrapping.

Wrapping errors prior to Go 1.13

Go has had error wrapping since the introduction of the errors package was introduced in early 2016.

This shortly followed with the errors.[New|Errorf|Wrap|Wrapf] methods implementing the interface:

The functionality for adding stack traces to existing errors came with the introduction of the errors.WithStack() method on the same package.

This returns an error with a format method which will print the stack trace when used with the %+v verb.

Which, when logging the error with the %+v verb, produces something along the lines of:

We can use this to our advantage when creating errors to include a stack trace in them. This can then be logged along with the error in order to aid locating the source of the error.

One downside of this approach is the performance penalty. The necessary call into runtime.Stack(buf []byte, all bool) int incurs a non-negligible cost, and should be avoided in hot paths.

Stack traces are also not particularly human friendly. They will only identify where the error occurred, not why. Therefore, it’s still good to add extra context to your errors.

Another minor nitpick is that when wrapping errors multiple times with github.com/pkg/errors , it will leave you with multiple stack traces, one for each time you wrap the error. This creates a rather large set of logs when you finally print the error if your stack is deep 🃏.

Wrapping errors in Go 1.13

In version 1.13 , Go introduced the %w verb, as well as a few methods on the errors package to support error identification and incorporate wrapping into the standard library.

By using fmt.Errorf() with the %w verb, we’re able to enhance errors with additional information using only the standard library, while allowing them to remain inspectable should we need to identify the underlying cause of an error.

It’s important to keep in mind however, when wrapping with %w , you are implicitly exposing the error to consumers of your package. Only use this when you plan to support the error type and avoid exposing implementation details. You can use the %v verb instead for non-inspectable errors (no errors.Is(), errors.As() ).

Now having fmt.Errorf() available is good, however if you never use it in your programs you’re still going to end up with logs which are hard to decipher. Now you could most certainly wrap every error returned at every point of your program. However, you will end up with a lot of redundant wraps which don’t add a whole lot of value and reduce the signal-to-noise ratio in your logs.

Hence, it would be helpful to have a simple set of rules to follow for when errors should be wrapped. This is by no means an exhaustive set, however should cover the majority of common cases within most programs.

1. When you have additional context, which you think would be useful to give to the developer reading the log. This is important, as rules are not infallible.
2. Errors returned from another package. Wrapping these helps identify the entry point of the error into your program. e.g.

This set of rules gives a good starting point for helping you identify where in your program you should wrap errors. In order to help out with this, I’ve written a linter to identify areas where your code is not wrapping errors and it possibly should be.

Wrapcheck

Wrapcheck is a new linter for Go which I built in order to maintain consistency across a codebase with regards to point (1) and (2).

For example, the following code which calls into the sql package will be reported as an error as follows.

The fix is to add wrapping to the errors before returning them. The linter however is a bit more flexible, and doesn’t mind if you still want to use pkg/errors . It will be satisfied as long as the error is not returned bare.

The linter was written with the new(ish) go/analysis tooling, which greatly simplifies AST traversal as well as setup and testing.

The linter has a fair number of tests. You can have a look at all the specified cases in the testdata directory.

Please feel free to use it in your own projects and report any issues you may come across. As mentioned, the linter is meant to handle gracefully the standard case, there are most certainly cases which slip past it. But feel free to file an issue for these as well.

Источник

tomarrell/wrapcheck

Use Git or checkout with SVN using the web URL.

Work fast with our official CLI. Learn more.

Sign In Required

Please sign in to use Codespaces.

Launching GitHub Desktop

If nothing happens, download GitHub Desktop and try again.

Launching GitHub Desktop

If nothing happens, download GitHub Desktop and try again.

Launching Xcode

If nothing happens, download Xcode and try again.

Launching Visual Studio Code

Your codespace will open once ready.

There was a problem preparing your codespace, please try again.

Latest commit

Git stats

Files

Failed to load latest commit information.

README.md

A simple Go linter to check that errors from external packages are wrapped during return to help identify the error source during debugging.

Wrapcheck is also available as part of the golangci-lint meta linter. Docs and usage instructions are available here. When used with golangci-lint, configuration is integrated with the .golangci.yaml file.

You can configure wrapcheck by using a .wrapcheck.yaml file in either the local directory, or in your home directory.

To lint all the packages in a program:

This linter is tested using analysistest , you can view all the test cases under the testdata directory.

If you’ve ever been debugging your Go program, and you’ve seen an error like this pop up in your logs.

Then you know exactly how painful it can be to hunt down the cause when you have many methods which looks just like the following:

The problem here is that multiple method calls into the sql package can return the same error. Therefore, it helps to establish a trace point at the point where error handing across package boundaries occurs.

To resolve this, simply wrap the error returned by the db.Conn.Get() call.

Now, your logs will be more descriptive, and allow you to easily locate the source of your errors.

A further step would be to enforce adding stack traces to your errors instead using errors.WithStack() however, enforcing this is out of scope for this linter for now.

Errors in Go are simple values. They contain no more information about than the minimum to satisfy the interface:

This is a fantastic feature, but can also be a limitation. Specifically when you are attempting to identify the source of an error in your program.

As of Go 1.13, error wrapping using fmt.Errorf(. ) is the recommend way to compose errors in Go in order to add additional information.

Errors generated by your own code are usually predictable. However, when you have a few frequently used libraries (think sqlx for example), you may run into the dilemma of identifying exactly where in your program these errors are caused.

In other words, you want a call stack.

This is especially apparent if you are a diligent Gopher and always hand your errors back up the call stack, logging at the top level.

So how can we solve this?

Wrapping errors at the call site.

When we call into external libraries which may return an error, we can wrap the error to add additional information about the call site.

This solution allows you to add context which will be handed to the caller, making identifying the source easier during debugging.

As with most static analysis tools, this linter will likely miss some obscure cases. If you come across a case which you think should be covered and isn’t, please file an issue including a minimum reproducible example of the case.

This project is licensed under the MIT license. See the LICENSE file for more details.

About

A Go linter to check that errors from external packages are wrapped

Источник

A Go linter to check that errors from external packages are wrapped

Related tags

Overview

A simple Go linter to check that errors from external packages are wrapped during return to help identify the error source during debugging.

Wrapcheck is also available as part of the golangci-lint meta linter. Docs and usage instructions are available here. When used with golangci-lint, configuration is integrated with the .golangci.yaml file.

You can configure wrapcheck by using a .wrapcheck.yaml file in either the local directory, or in your home directory.

To lint all the packages in a program:

This linter is tested using analysistest , you can view all the test cases under the testdata directory.

If you’ve ever been debugging your Go program, and you’ve seen an error like this pop up in your logs.

Then you know exactly how painful it can be to hunt down the cause when you have many methods which looks just like the following:

The problem here is that multiple method calls into the sql package can return the same error. Therefore, it helps to establish a trace point at the point where error handing across package boundaries occurs.

To resolve this, simply wrap the error returned by the db.Conn.Get() call.

Now, your logs will be more descriptive, and allow you to easily locate the source of your errors.

A further step would be to enforce adding stack traces to your errors instead using errors.WithStack() however, enforcing this is out of scope for this linter for now.

Errors in Go are simple values. They contain no more information about than the minimum to satisfy the interface:

This is a fantastic feature, but can also be a limitation. Specifically when you are attempting to identify the source of an error in your program.

As of Go 1.13, error wrapping using fmt.Errorf(. ) is the recommend way to compose errors in Go in order to add additional information.

Errors generated by your own code are usually predictable. However, when you have a few frequently used libraries (think sqlx for example), you may run into the dilemma of identifying exactly where in your program these errors are caused.

In other words, you want a call stack.

This is especially apparent if you are a diligent Gopher and always hand your errors back up the call stack, logging at the top level.

So how can we solve this?

Wrapping errors at the call site.

When we call into external libraries which may return an error, we can wrap the error to add additional information about the call site.

This solution allows you to add context which will be handed to the caller, making identifying the source easier during debugging.

As with most static analysis tools, this linter will likely miss some obscure cases. If you come across a case which you think should be covered and isn’t, please file an issue including a minimum reproducible example of the case.

This project is licensed under the MIT license. See the LICENSE file for more details.

Comments

Configuration to define what is external

Thanks for the nice linter!

I was curious if you considered cases where the meaning of external could mean something other than outside the current package . In my case I would like to find cases where errors from outside the current module or -local string are not wrapped. I see the levels being package , module , organization . If this interests you I’d like to work on it with you otherwise I could simply fork the project

Proposal: ignore returning value of specify package’s function

Hello 👋 Thank you for developing such a great tool!

I’m using a library that wraps and manages errors like below.

Currently, wrapcheck also reports for these librarie’s function.

This is, of course, the correct behavior. But I don’t need these report from wrapcheck. So, I ignore these reports with golangci-lint’s exclude-rules like below.

It would be very helpful to be able to configure these settings on the wrapcheck side.

(This proposal may be similar to #2)

Consider whitelisting cases where function has single error return case

With the following code:

I think the linter should allow to skip error wrapping, as the caller should do the wrapping. If error occurs, it is known that the error comes from this line. What do you think? Or maybe this should be somehow configurable?

ignorePackageGlobs doesn’t appear to be working

Hey, great tool! This has been really useful to track down places I can apply some new error context packages I’ve been working on to provide additional structured information to errors.

After applying error contexts across our project I noticed a few places where it was unnecessary so I decided to disable reporting of imports from local packages (/pkg, etc)

However, maybe I’m misunderstanding this config option, but it doesn’t appear to do what I expected.

Here’s the configuration file: https://github.com/Southclaws/storyden/blob/main/.golangci.yml#L28

And here are the offending lines:

I assumed that a ignorePackageGlobs value of github.com/Southclaws/storyden/* would ignore these two lines as they are functions that are imported from within this pattern:

• i.thread_svc.Create : github.com/Southclaws/storyden/pkg/services/thread.Service.Create
• i.thread_svc.ListAll : github.com/Southclaws/storyden/pkg/services/thread.Service.Create

The actual output from running golangci-lint run :

I wondered if it was golangci lint just using an outdated version but this occurs on the latest standalone version of wrapcheck too.

I tried with a .wrapcheck.yaml file with:

but I can’t seem to get any patterns to ignore correctly.

dev: refactor `regexp` usage

This PR intends to bring the next features/fixes to wrapcheck :

1. avoid compiling regexp rules into regexp.Regexp for checking each error returning function.
2. removing os.Exit(1) in case fail to compile regexp rule from the codebase ( we can’t handle such cases in golangci-lint : output is hidden, so no log messages received, which ends in no issues or errors comes from wrapcheck if bad regexp provided).

P.S. The rest of the coding is changed by gofumpt automatically. P.P.S. It would be also great to release fix changes asap (so we update golangci-lint ).

Great linter! Just one question about %w vs %v

Forgive my question but I think there is a reason you are using %v rather than %w on errorf to wrap errors but I am not sure I understand it.

I have always been using %w with returning to new errors that wrap previous ones but I notice here you are using %v.

What is the reason ?

wrapcheck on goerr113 wrapping %w

Wrapcheck on goerr113 wrapping %w

I understand that Wrapcheck linter is all about wrapping the error coming from external packages but I think the error coming from the external package should never be used as %w reason being that this forces DevUsers to import the package A where the error is being returned as well as the internal package B where the error is being thrown and possible where the Sentinel lives if we ever need to check on the error type with Error.Is and Error.As creating a form of coupling to the internal detail implementation of package A

I think Sentinel errors need to be created to wrap the err coming from the external package so only one import is needed to check on the Error.Is or Error.As meanwhile, we make sure the external package error is wrapped into a more meaningful error at this package level without exposing any type of possible coupling

New release?

I’d like to use the new ignorePackageGlobs feature

But it wasn’t released yet.

Do you plan to release it?

Support for errors.Wrap(f)

In our project we use golangci-lint and have a lot of similar code:

Starting from version golangci-lint 1.39.0 built from 9aea4aee on 2021-03-26T08:02:53Z all those lines cannot pass wrapcheck :

Is that a false positive error ? Or am I missing something ?

false positive messages on legal errors from the same file

file internal/modifier/modifiers_aggregate.go contains code:

causes the following: internal/modifier/modifiers_aggregate.go:197:15: error returned from external package is unwrapped (wrapcheck) return nil, ErrPlaylistsNotTwo

Linter triggers on wrapper functions

Considering following code:

Right now linter triggers on it, but it seems very counter-intuitive.

Used via golangci-lint version v1.39.0

Support Gorm-style error checking

Gorm uses non-standard error format to support it’s chainable API. It would be nice if there was a way to configure wrapcheck to match a *..Error regex

Embedded interfaces are hard to ignore

When an interface is embedded, the reported interface name isn’t what you need to ignore. Concretely, extend wrapcheck/testdata/config_ignoreInterfaceRegexps/main.go with the following, and run the test.

Unexpectedly, you’ll see the following:

— FAIL: TestAnalyzer (5.40s) — FAIL: TestAnalyzer/config_ignoreInterfaceRegexps (0.25s) analysistest.go:452: /home/murman/src/wrapcheck/wrapcheck/testdata/config_ignoreInterfaceRegexps/main.go:33:9: unexpected diagnostic: error returned from interface method should be wrapped: sig: func (_/home/murman/src/wrapcheck/wrapcheck/testdata/config_ignoreInterfaceRegexps.errorer).Decode(v interface<>) error

Note that the ignores include errorer and the reported interface is config_ignoreInterfaceRegexps.errorer . However to suppress this you actually need to suppress embedder .

I believe this comes down to the difference between name and fnSig in reportUnwrapped, which come from sel.Sel and sel.X respectively, but the path forward is unclear to me. https://github.com/tomarrell/wrapcheck/blob/213318509af6a003be2be752da826d269149ba4d/wrapcheck/wrapcheck.go#L257-L260

Flag to ignore main module

As has been requested, ignoring the main module in a project via a flag to prevent having to configure the package ignore for each project.

Something along the lines of ignore: go list -m .

False-positive for anonymous functions

Wrapcheck throws an error for the next case:

error returned from external package is unwrapped: sig: func errors.New(text string) error

Named return values not reported

You’ll see that wrap check does not detect that err is not being wrapped.

consider flagging errors.Wrap(err) where err is known to be nil

I found several cases in my codebase (https://github.com/kopia/kopia/pull/747) that follow the following buggy pattern:

The second errors.Wrap() is incorrect, because it’s not wrapping any error (but it’s easy to copy/paste it as such).

This is quite hard to spot in code reviews because lines that return errors will always have errors.Wrap() and it looks ok at first glance, until you notice that in this case is err is always nil . Because errors.Wrap(nil, «something») always returns nil this one returns success, which is unintended.

Based on flow analysis, it should be possible to determine that err == nil in this case, and it would be really amazing if the linter could flag this pattern is as misuse.

Источник

Welcome

• Yes, I’m using a binary release within 2 latest major releases. Only such installations are supported.
• Yes, I’ve searched similar issues on GitHub and didn’t find any.
• Yes, I’ve included all information below (version, config, etc).
• Yes, I’ve tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)

Description of the problem

wrapcheck conflicts with errorlint on this:

fmt.Errorf("trying to wrap some error: %w", someErr)

wrapcheck: error returned from external package is unwrapped: sig: func fmt.Errorf(format string, a …interface{}) error

when I change %w to %v, then happens this:

errorlint: non-wrapping format verb for fmt.Errorf. Use %w to format errors

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.42.0 built from c6142e38 on 2021-08-17T11:47:22Z

Configuration file

linters-settings:
  govet:
    check-shadowing: true
    settings:
      printf:
        funcs:
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf
          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf
  gocyclo:
    #  https://groups.google.com/g/golang-nuts/c/HNNUjE5VWos
    min-complexity: 10
  dupl:
    threshold: 100
  goconst:
    min-len: 2
    min-occurrences: 2
  misspell:
    locale: US
  lll:
    line-length: 140

  goimports:
    local-prefixes: github.com/golangci/golangci-lint
  gocritic:
    enabled-tags:
      - performance
      - style
      - experimental
  wrapcheck:
    ignoreSigs:
      - fault.


linters:
  enable-all: true
  disable:
    - gochecknoglobals
    - dupl
    - golint
    - maligned
    - scopelint
    - interfacer
    - gci
    - tagliatelle

run:
  # default concurrency is a available CPU number
  concurrency: 4

  # timeout for analysis, e.g. 30s, 5m, default is 1m
  deadline: 1m

  tests: false

  skip-dirs:
    - testdata
    - scripts
    - contrib
    - bin
    - vendor

Go environment

set GO111MODULE=on
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:UsersuserAppDataLocalgo-build
set GOENV=C:UsersuserAppDataRoaminggoenv
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:Usersusergopkgmod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:Usersusergo
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:Program FilesGo
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:Program FilesGopkgtoolwindows_amd64
set GOVCS=
set GOVERSION=go1.16.6
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:UsersuserDocumentsgranpaparenprojgo.mod
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:UsersuserAppDataLocalTempgo-build154680675=/tmp/go-build -gno-record-gcc-switches

Verbose output of running

level=info msg="[config_reader] Used config file .golangci.yml"
level=info msg="[lintersdb] Active 69 linters: [asciicheck bodyclose cyclop deadcode depguard dogsled durationcheck errcheck errname errorlint exhaustive exhaustivestruct exportloopref forbidigo forcetypeassert funlen gochecknoinits gocog
nit goconst gocritic gocyclo godot godox goerr113 gofmt gofumpt goheader goimports gomnd gomoddirectives gomodguard goprintffuncname gosec gosimple govet ifshort importas ineffassign lll makezero misspell nakedret nestif nilerr nlreturn n
octx nolintlint paralleltest prealloc predeclared promlinter revive rowserrcheck sqlclosecheck staticcheck structcheck stylecheck testpackage thelper tparallel typecheck unconvert unparam unused varcheck wastedassign whitespace wrapcheck
wsl]"
level=info msg="[loader] Go packages loading at mode 575 (compiled_files|deps|exports_file|name|types_sizes|files|imports) took 850.0753ms"
level=info msg="[runner/filename_unadjuster] Pre-built 0 adjustments in 8.6887ms"
level=info msg="[linters context] importas settings found, but no aliases listed. List aliases under alias: key."
level=info msg="[linters context/goanalysis] analyzers took 0s with no stages"
level=info msg="[runner/max_same_issues] 18/21 issues with text "return statements should not be cuddled if block has more than two lines" were hidden, use --max-same-issues"
level=info msg="[runner/max_same_issues] 7/10 issues with text "assignments should only be cuddled with other assignments" were hidden, use --max-same-issues"
level=info msg="[runner/max_same_issues] 6/9 issues with text "return with no blank line before" were hidden, use --max-same-issues"
level=info msg="[runner/max_same_issues] 4/7 issues with text "hugeParam: a is heavy (816 bytes); consider passing it by pointer" were hidden, use --max-same-issues"
level=info msg="[runner/max_same_issues] 3/6 issues with text "declarations should never be cuddled" were hidden, use --max-same-issues"
level=info msg="[runner/max_same_issues] 3/6 issues with text "only one cuddle assignment allowed before if statement" were hidden, use --max-same-issues"
level=info msg="[runner/max_same_issues] 2/5 issues with text "append only allowed to cuddle with appended value" were hidden, use --max-same-issues"
level=info msg="[runner] Issues before processing: 593, after processing: 73"
level=info msg="[runner] Processors filtering stat (out/in): skip_files: 593/593, identifier_marker: 593/593, max_same_issues: 73/116, severity-rules: 73/73, path_prefixer: 73/73, cgo: 593/593, exclude: 593/593, nolint: 152/198, max_from_
linter: 73/73, sort_results: 73/73, path_prettifier: 593/593, skip_dirs: 593/593, autogenerated_exclude: 593/593, uniq_by_line: 116/152, diff: 116/116, filename_unadjuster: 593/593, exclude-rules: 198/593, max_per_file_from_linter: 116/11
6, source_code: 73/73, path_shortener: 73/73"
level=info msg="[runner] processing took 93.6486ms with stages: max_same_issues: 31.0185ms, exclude-rules: 24.9975ms, path_prettifier: 19.629ms, identifier_marker: 9.0001ms, nolint: 6.0036ms, autogenerated_exclude: 2.0004ms, skip_dirs: 99
9.5µs, path_prefixer: 0s, cgo: 0s, max_per_file_from_linter: 0s, diff: 0s, exclude: 0s, max_from_linter: 0s, source_code: 0s, path_shortener: 0s, severity-rules: 0s, filename_unadjuster: 0s, skip_files: 0s, uniq_by_line: 0s, sort_results:
 0s"
level=info msg="[runner] linters took 318.614ms with stages: goanalysis_metalinter: 197.6084ms"
# i removed the rest of the errors
pkgrepositorydatabasesomefile.go:968:10: error returned from external package is unwrapped: sig: func fmt.Errorf(format string, a ...interface{}) error (wrapcheck)
                return fmt.Errorf("couldn't map values: %w", err)
                       ^
level=info msg="File cache stats: 12 entries of total size 56.5KiB"
level=info msg="Memory: 20 samples, avg is 41.5MB, max is 81.9MB"
level=info msg="Execution took 1.9022268s"

Code example or link to a public repository

just as I described already

fmt.Errorf("trying to wrap some error: %w", someErr)
fmt.Errorf("trying to wrap some error: %v", someErr)