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: Go 2: provide symbol (e.g. #) to abort/panic on non-zero return value #22122

Closed
networkimprov opened this Issue Oct 3, 2017 · 20 comments

Comments

Projects
None yet
7 participants
@networkimprov

networkimprov commented Oct 3, 2017

A distinction I have not seen raised in #21161:

There are two broad classes of error

  1. Errors due to Unacceptable Input: A correct program handles them gracefully as they are inevitable.

  2. Errors due to Programming Mistake: A correct program does not see them. If they arise, the program is incorrect and therefore unpredictable; it should abort.

In C, one handles (2) with err = f(a); assert(err==0);
In C++ uncaught exceptions cause abort without explicit encouragement.

In Go today, we see if err != nil { log.Fatal(err) } (or panic if a stack trace is desired) after every function call that cannot fail in a correct program on a sane system. Such statements are essentially meaningless, and make the logic harder to follow. The compiler should handle this for us.

Therefore let # (a variant of _) specify abort-with-stack-trace for a non-zero return value (i.e. it's not error specific).

# = f(a)
x, # = f(a)
#, # = f(a) // abort if either value is non-zero
x, _, # = f(a)

This is not incompatible with any separate feature that adds context when handling (1), as discussed in #21161.

This results in dramatically less ceremony:

aFd, # := os.Open(aPath)
_, # = aFd.Read(aBuf)
# = json.Umarshal(aBuf, &aObject)

// vs

aFd, err := os.Open(aPath)
if err != nil {
  log.Fatal(err)
}
_, err = aFd.Read(aBuf)
if err != nil {
  log.Fatal(err)
}
err = json.Umarshal(aBuf, &aObject)
if err != nil {
  log.Fatal(err)
}

If # is not sufficiently noticeable, ## could be used.

aFd, ## := os.Open(aPath)
_, ## = aFd.Read(aBuf)
## = json.Umarshal(aBuf, &aObject)

As an aside, I use the following instead of log.Fatal(err) or panic(err):

func quit(err error) {
    fmt.Fprintln(os.Stderr, "quit after "+err.Error())
    debug.PrintStack() // why is this not a log.Fatal option?
    os.Exit(3) // because panic exits with 2
}

err = os.Open(file)
if err != nil { quit(err) } // gofmt is mindless, albeit well-intentioned

@gopherbot gopherbot added this to the Proposal milestone Oct 3, 2017

@gopherbot gopherbot added the Proposal label Oct 3, 2017

@ianlancetaylor ianlancetaylor changed the title from proposal: Provide symbol (e.g. #) to abort/panic on non-zero return value to proposal: Go 2: provide symbol (e.g. #) to abort/panic on non-zero return value Oct 3, 2017

@networkimprov

This comment has been minimized.

networkimprov commented Oct 3, 2017

To any detractors of this idea, note that it's purely optional. You're free to carry on writing

err = f(mistake)
if err != nil {
  log.Fatal(err)
}

But if you don't need to write that repeatedly, it's much easier to see those errors that you expect to encounter, and therefore handle gracefully.

@mdempsky

This comment has been minimized.

Member

mdempsky commented Oct 3, 2017

panic(err) is an anti-pattern though. Adding syntax to make it even easier to write instead of proper error handling seems to have negative utility.

@networkimprov

This comment has been minimized.

networkimprov commented Oct 4, 2017

Quoting myself...

  1. Errors due to Unacceptable Input: A correct program handles them gracefully as they are inevitable.

  2. Errors due to Programming Mistake: A correct program does not see them. If they arise, the program is incorrect and therefore unpredictable; it should abort.

panic(err) or log.Fatal(err) is the right way to handle (2). An incorrect program should stop!

What you call "proper error handling" is only proper for (1).

@davecheney

This comment has been minimized.

Contributor

davecheney commented Oct 4, 2017

This an argument for known unknowns; the known programming mistakes are annotated with your # syntax. This once you know all the unknown programming mistakes, you've actually handled all the errors properly.

Can you please give some examples of class (2) programming errors?

@mdempsky

This comment has been minimized.

Member

mdempsky commented Oct 4, 2017

If it's an error the caller should never see, why is the function returning an error instead of just panicking itself?

@networkimprov

This comment has been minimized.

networkimprov commented Oct 4, 2017

@davecheney # annotates calls that will succeed unless there is a) a bug in the program, or b) a rare mishap for which a path to recovery is undefined. Examples:

aFd, # := os.Open(aPath)          // program failed to create file, or user axed it
_, # = aFd.Read(aBuf)             // program mistakenly truncated the file
# = json.Umarshal(aBuf, &aObject) // program wrote incorrect JSON

If it's an error the caller should never see, why is the function returning an error instead of just panicking itself?

@mdempsky The standard and third-party libraries cannot determine whether the issue is due to a mistake in the caller, so they always return error.

@davecheney

This comment has been minimized.

Contributor

davecheney commented Oct 4, 2017

@networkimprov

This comment has been minimized.

networkimprov commented Oct 4, 2017

My argument is that this

aFd, # := os.Open(aPath)
_, # = aFd.Read(aBuf)
# = json.Umarshal(aBuf, &aObject)

Is far more readable than this

aFd, err := os.Open(aPath)
if err != nil {
  panic(err)
}
_, err = aFd.Read(aBuf)
if err != nil {
  panic(err)
}
err = json.Umarshal(aBuf, &aObject)
if err != nil {
  panic(err)
}

Also it's easier to spot errors that are expected, and therefore handled gracefully, in the former.

I currently do the latter, so am not seeking incentive to check for errors. But I should not have to do this when the compiler can; the Go authors made an argument against "ceremony" once upon a time!

@davecheney

This comment has been minimized.

Contributor

davecheney commented Oct 4, 2017

My argument is that this ... Is far more readable than this

This is subjective, and I disagree.

@as

This comment has been minimized.

Contributor

as commented Oct 4, 2017

I understand if err != nil { panic(err); }. I dont understand the proposal after reading it twice. I have used perl too.

@networkimprov

This comment has been minimized.

networkimprov commented Oct 4, 2017

Well there's a learning curve for any language :-)

This is one of those features where, after trying it, everyone will wonder how we tolerated the previous regime for so long -- i.e. meaningless error-checks after function calls that always work in a sane system.

@as

This comment has been minimized.

Contributor

as commented Oct 4, 2017

everyone will wonder how we tolerated the previous regime for so long

History dictates few people actually think in this manner. Everyone thinks their thing is the best, but the best thing is actually the thing with the least things.

I disagree not because I don't want to write it, but because I don't want to read it. Choice breeds entropy and resistance to choice is a powerful undocumented feature of the language!

@networkimprov

This comment has been minimized.

networkimprov commented Oct 5, 2017

It doesn't bother me to write if err != nil { log.Fatal(err) } after most library calls, but it is bothersome to read in a long sequence of calls, as the normal logic flow is repeatedly interrupted with meaningless statements.

One nice feature of C++ exceptions is that an uncaught exception stops execution, with no per-call ceremony. Go would benefit from that. If you enjoy ceremony, keep performing it; just don't impose it on those of us who don't.

@davecheney

This comment has been minimized.

Contributor

davecheney commented Oct 5, 2017

@networkimprov

This comment has been minimized.

networkimprov commented Oct 5, 2017

As I have stated, this proposal's aim is precisely to optimize for reading the code:

  1. by removing statements that are meaningless in a correct program
  2. by making the code to handle legitimate errors more apparent
  3. by flagging certain function calls as "must-not-fail"

It also benefits debugging, because it generates a stack trace from the failure point, which you don't get if you do if err != nil { return err } and then log.Fatal() or panic() higher up the stack.

@Carpetsmoker

This comment has been minimized.

Carpetsmoker commented Nov 28, 2017

To any detractors of this idea, note that it's purely optional. You're free to carry on writing [..]

As a general comment, I do not believe there is such thing as an "optional" feature for a programming language. Several decades of programming experience has shown that if a feature is in a language, it will be used. And once a feature is used, all Go programmers will have to deal with it, as there are very few programmers that work on only code they themselves have written.

@networkimprov

This comment has been minimized.

networkimprov commented Nov 28, 2017

Alright, to be rigorous: It is optional for the Keeper of Code Style for the project.

Notwithstanding the myth of a Single Go Style promoted by gofmt, there are numerous stylistic choices available, and implicit vs explicit abort-on-incorrect-program would be one more.

PS: to those who read the original proposal shortly after I posted, I have since revised it several times for clarity.

@as

This comment has been minimized.

Contributor

as commented Nov 29, 2017

There are numerous stylistic choices available

Which is unfortunate and should be reduced. Freedom from a feature is beneficial as much as, if not more, than the freedom to choose.

@networkimprov

This comment has been minimized.

networkimprov commented Nov 29, 2017

Coding is principally a communications discipline for most coders. Humans tailor communication methodology/style based on their actual circumstances, colleagues, challenges etc. It is thoughtless to assert that an externally-defined scheme would be superior for many, let alone most, such situations.

I filed this proposal because it makes my code (with heavy use of os.File) far easier to read -- and I already use one-line error checks! It does the same for many others using similar libraries. If you don't use a library which returns numerous errors that you will/should never see, you wouldn't see the value on first glance.

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Mar 21, 2018

There doesn't seem to be much support for this specific proposal. There is a lot of discussion of error handling over in #21161, so closing this one in favor of that.

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