Style recommendation for error types in exported APIs

[Jaana Dogan]

Hello all,

Currently, it's not discussed in the https://github.com/golang/go/wiki/CodeReviewComments when it's preferred to return a concrete error type.

Consider the following custom error type:

type StatusError struct {

   // ...

}

func (s *StatusError) Error() string { ... } 

I'd write a function that constructs a new error like below, so the developer can access to the fields and other methods on the StatusError:

func NewStatusError(code int) *StatusError { ... }

Some Go developers assume that it should be written like the following to highlight that the returned value's type is error.

func NewStatusError(code int) error { ... }

I don't know why there is such a perception in the community, but it'd be good to clarify that the second style is not superior to the other. 

Thanks,

jbd

[Ian Davis]

I find concrete error returns to be mildly frustrating to work with since when you call functions with different error return types you need to use separate error variables or predeclare a variable with type error. https://play.golang.org/p/eGjsNmdB8HE

Note that returning error types vs concrete types is also mentioned in the FAQ https://golang.org/doc/faq#nil_error

Ian

[Dan Kortschak]

On Sat, 2021-06-12 at 01:40 +0100, Ian Davis wrote:
> I find concrete error returns to be mildly frustrating to work with
> since when you call functions with different error return types you
> need to use separate error variables or predeclare a variable with
> type error. https://play.golang.org/p/eGjsNmdB8HE
>
> Note that returning error types vs concrete types is also mentioned
> in the FAQ https://golang.org/doc/faq#nil_error
>
> Ian

I think this is a different context. The use case suggested looks more
like it is for when a user wants to construct a paritcular error type
to return in an erorable function, not for the errorable function
itself.

Dan

[Ian Davis]

Oh, I see. A constructor for the error type. Yes, I agree the concrete type is better there. Pardon the noise.

Ian

[roger peppe]

The time this pattern might be problematic is if it's possible that NewStatusError might return nil (for example if code represents an HTTP status and is 2xx). In that case, there would be a strong likelihood that you could get a non-nil-but-nil-inside error created by people that just return the result of calling NewStatusError directly.

 

Thanks,

jbd

--
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-dev/CAMGFQi78JcH6yojLnV9CH8OxK2zCROE_Ys9R8FuVTvxP2F4ZCQ%40mail.gmail.com.

[Jaana Dogan]

On Sat, Jun 12, 2021 at 12:14 PM roger peppe <rogp...@gmail.com> wrote:

On Sat, 12 Jun 2021, 00:04 Jaana Dogan, <rak...@rakyll.org> wrote:

Hello all,

Currently, it's not discussed in the https://github.com/golang/go/wiki/CodeReviewComments when it's preferred to return a concrete error type.

Consider the following custom error type:

type StatusError struct {

   // ...

}

func (s *StatusError) Error() string { ... } 

I'd write a function that constructs a new error like below, so the developer can access to the fields and other methods on the StatusError:

func NewStatusError(code int) *StatusError { ... }

Some Go developers assume that it should be written like the following to highlight that the returned value's type is error.

func NewStatusError(code int) error { ... }

I don't know why there is such a perception in the community, but it'd be good to clarify that the second style is not superior to the other.

The time this pattern might be problematic is if it's possible that NewStatusError might return nil (for example if code represents an HTTP status and is 2xx). In that case, there would be a strong likelihood that you could get a non-nil-but-nil-inside error created by people that just return the result of calling NewStatusError directly.

 

I agree that having a constructor is not a good pattern  and hides the behavior if the constructor ever returns nil. It doesn't in the cases I've seen, but I guess having constructors vs not is an additional conversation to have.