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.

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

use const for custom error type #22

Closed
rockmenjack opened this issue Oct 12, 2019 · 4 comments
Closed

use const for custom error type #22

rockmenjack opened this issue Oct 12, 2019 · 4 comments

Comments

@rockmenjack
Copy link

I am more in favor of @davecheney 's constant-errors than using var.

type Error string
const ErrCouldNotOpen = Error("could not open")

is better than

var ErrCouldNotOpen = errors.New("could not open")
@abhinav
Copy link
Collaborator

abhinav commented Oct 13, 2019

The idea of const errors is appealing! A tradeoff is that our exported
ErrCouldNotOpen cannot have the type error anymore.

// fails with: invalid constant type error
const ErrCouldNotOpen error = Error("foo")

A usability issue of this is that if we do err := ErrCouldNotOpen, its type
is now Error, and we can't assign another error to it in the same scope.

err := ErrCouldNotOpen
...

// fails with: cannot assign error to err (type Error)
x, err := anotherOperation()

(https://play.golang.org/p/V2Gyd1mVkb9)

Admittedly, this isn't terribly inconvenient because most of the time we'll do
return ErrCouldNotOpen where the return type is known to be error, and in
the rare cases where we need it to be a variable, var err error = .. will
suffice.

Also, to keep the exported API surface small, we'll likely want to use an
unexported type.

package foo

type fooError string

func (e fooError) Error() string { return string(e) }

const ErrCouldNotOpen = fooError("could not open")

I dislike the idea of exporting an entity with a type that is not meant to be
used directly.

I think we should chew on this a bit and weigh the usability and hygiene
tradeoffs.

@shyiko
Copy link

shyiko commented Oct 13, 2019

type Error in

type Error string
const ErrCouldNotOpen = Error("could not open")

does not have Error() string method implemented and so something as simple as

func f() error {
	return ErrCouldNotOpen
}

won't compile (with cannot use ErrCouldNotOpen (type Error) as type error in return argument: Error does not implement error (missing Error method)) unless

func (e Error) Error() string {
	panic("implement me")
}

is added, which I personally think becomes a little bit too much code for a general case.
If there is a need for a full blown error type, then @abhinav's comment sums up pros/cons pretty well.

Something else to consider: if const Err... becomes a recommendation then it would have to tagged with "for primitive types only" as const Err.. = errorStruct{...} won't compile (which means we'll have both const & var error declarations...).

-1 from me on const Err... recommendation.

@prashantv
Copy link
Contributor

Agree with @abhinav, given the trade-offs, I don't think this is something we should do right now.

@jub0bs
Copy link

jub0bs commented Dec 20, 2023

FWIW, here's a simpler approach that opens the door to "effectively constant" sentinel error values:

var errCouldNotOpen = errors.New("could not open")

func ErrCouldNotOpen() error {
  return errCouldNotOpen
}

When you think about it, this is essentially what context.Background does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants