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

proposal: assert of errors #32997

Open
xrfang opened this issue Jul 9, 2019 · 6 comments

Comments

@xrfang
Copy link

commented Jul 9, 2019

I would like add my 2 cents to the disscussion initiated in #32437 ("try") and #32811 ("catch") and then #32825 (do nothing).

My suggestion is to add a built-in function assert(), which does the following:

func assert(err error) {
    if err != nil {
        panic(err)
    }
}

With the assert() function, we can just simply use a defer call for error handling:

func doSomethin() (err error) {
        defer func() {
              if e := recover(); e != nil {
                       err = e.(error) //assuming we always panic with error type.
              }
        }()
        db, err := sql.Open(...)
        assert(err)
        defer db.Close()
        assert(doDBOperation(db))
        ... ...
}

This proposal is very close to #32825 (do nothing), and I think it follows go's philosophy of being simple and explicit. Also I think this mimics exception in other languages, which we can add stack trace using library like github/pkg/errors:

func doSomethin() (err error) {
        defer func() {
              if e := recover(); e != nil {
                       err = errors.Wrapf("doSomething(): %v",  e)
              }
        }()
        ... ...
}

@gopherbot gopherbot added this to the Proposal milestone Jul 9, 2019

@gopherbot gopherbot added the Proposal label Jul 9, 2019

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

I don't see why assert needs to be a builtin function. You could just write it yourself.

@xrfang

This comment has been minimized.

Copy link
Author

commented Jul 9, 2019

I don't see why assert needs to be a builtin function. You could just write it yourself.

That's exactly why I say it is closer to "do nothing". I don't see why try() or catch() is necessary. To be honest, my biggest requirement of error handling is ALWAYS get stack trace of errors, but I don't think adding an easy stack tracing facility, without using pkg/errors etc, will be accepted by the core team :-)

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

We already have one proposal that says we should do nothing (#32825). I'm not sure we need another one.

@bcmills

This comment has been minimized.

Copy link
Member

commented Jul 15, 2019

panic is a control-flow mechanism. If assert were a built-in, I would rather it not panic by default, so that callers would not be tempted to use recovered assertions for control flow.

(I have a proposal to file along these lines, but I'm still building up examples to support it — and, as far as I am concerned, it is completely orthogonal to error-handling, since it is not appropriate for errors that occur in the normal course of program execution.)

@rsc

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2019

This does not need to be in the standard library. It's also the case that we discourage turning expected errors into panics, which this would do. (That's the rationale behind not having a C-like assert too: https://golang.org/doc/faq#assertions. Panic is more for "this program cannot continue." See also https://golang.org/doc/effective_go.html#panic.)

@mvndaai

This comment has been minimized.

Copy link

commented Jul 18, 2019

My concern is with defer and go functions. The proposal ignores the error that could happen in defer db.Close().

What happens if I try this?

defer assert(db.Close())

or worse

go assert(superSlowFunc())

If the function has already returned that go assert has no place to be recovered from.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.