How to use 'try'
The recent try proposal by Robert Griesemer sparked a lot of controversy. While the proposal is a subject to reasonable criticism, I didn’t feel like everyone was always on the same page.
This post isn’t pro-try or anti-try. It just shows how try could be used so that you can form a more informed opinion.
Suppose we are grad students of social sciences and need to come up with idea for a poll. Since we are programmers, too, we’ve come up with this poll: find correlations between person’s gender, favorite OS, and favorite programming language.
This is, of course, a contrived example. The goal of this post isn’t to solve a real-world scenario, but to show multiple aspects of try in a small program.
The data from the poll will look like this:
name: Boris Bateman
gender: man
os: Windows
lang: PHP
name: Darin May
gender: woman
os: OSX
lang: JavaScript
name: Shea Wilks
gender: nonbinary
os: Linux
lang: Emacs LISP
name: Robert Griesemer
gender: man
os: Linux
lang: Go
Each respondent has four required fields: name, gender, os, and lang. There is an obligatory empty line between respondents.
We want to parse this file into a slice of Respondent structs, defined like this:
type Respondent struct {
Name string
Gender string
OS string
Lang string
}
Let’s get on to it, step by step!
func parseRespondents(path string) (resps []Respondent, err error) {
return nil, nil
}
I know, I could have used an io.Reader. But, for the purposes of demonstrating try, let’s keep it this way.
First, we’re gonna use a (not yet existing) function fmt.HandleErrorf:
func parseRespondents(path string) (resps []Respondent, err error) {
defer fmt.HandleErrorf(&err, "parse %s", path)
return nil, nil
}
It adds context to the returned error, if it’s not nil. It’s defined like this:
func HandleErrorf(err *error, format string, args ...interface{}) {
if *err != nil {
*err = errors.Wrapf(*err, format, args...)
}
}
Every error returned from parseRespondents("respondents.txt") will now be prefixed with parse respondents.txt:.
func parseRespondents(path string) (resps []Respondent, err error) {
defer fmt.HandleErrorf(&err, "parse %s", path)
f := try(os.Open(path))
defer f.Close()
return nil, nil
}
Normally, os.Open(path) returns two values: the file and an error. By wrapping it in try, we drop the error from the return values and get an automatic return if the error wasn’t nil.
What kind of error do we get now if the file doesn’t exist?
parse respondnts.txt: open respondnts.txt: no such file or directory
The error has all the context it needs, because os.Open adds some context on its own.
func parseRespondents(path string) (resps []Respondent, err error) {
defer fmt.HandleErrorf(&err, "parse %s", path)
file := try(os.Open(path))
defer file.Close()
scanner := bufio.NewScanner(file)
for {
//TODO: parse fields
}
return resps, nil
}
Now it’s time to create a helper function for parsing the individual fields.
func parseField(scanner *bufio.Scanner, name string) (value string, err error) {
defer fmt.HandleErrorf(&err, "parse field %s", name)
if !s.Scan() {
return "", io.EOF
}
prefix := name + ":"
if !strings.HasPrefix(s.Text(), prefix) {
return "", fmt.Errorf("expected %q", prefix)
}
return strings.TrimPrefix(s.Text(), prefix), nil
}
Once again, we use fmt.HandleErrorf to add a context to all errors returned from this function. This time, we tell which field we were trying to parse.
We try scanning the line. If successful, we try and trim the required prefix (e.g. "name:", "gender:", …) from the beginning of the line and return the resulting value of the field.
Let’s now use this helper function in parseRespondents.
First, we need to try and parse the first field of a respondent: the name. If we reached the end of file, that’s fine, we need to stop parsing. Otherwise, we need to report the error. This is not a situation for try, because we need to deal with the error in a more complex way than just returning it.
for {
name, err := parseField(scanner, "name")
if errors.Is(err, io.EOF) { // can't use ==, because the error is wrapped
break
}
if err != nil {
return resps, err
}
//TODO: parse the remaining fields
}
Once we parsed the name, we need to parse all the remaining fields. These are required to be there, so we can use try in this situation:
for {
name, err := parseField(scanner, "name")
if errors.Is(err, io.EOF) { // can't use ==, because the error is wrapped
break
}
if err != nil {
return resps, err
}
resp := Respondent{
Name: name,
Gender: try(parseField(scanner, "gender")),
OS: try(parseField(scanner, "os")),
Lang: try(parseField(scanner, "lang")),
}
resps = append(resps, resp)
}
Without try, we would have to manually check the error for each of the fields, resulting in many lines of code. Here, we can simply use the value returned from the try directly, try will take care of returning the possible error.
All that’s remaining is the obligatory empty line:
for {
name, err := parseField(scanner, "name")
if errors.Is(err, io.EOF) { // can't use ==, because the error is wrapped
break
}
if err != nil {
return resps, err
}
resp := Respondent{
Name: name,
Gender: try(parseField(scanner, "gender")),
OS: try(parseField(scanner, "os")),
Lang: try(parseField(scanner, "lang")),
}
resps = append(resps, resp)
if !s.Scan() {
break
}
if s.Text() != "" {
return resps, errors.New("expected empty line")
}
}
And here’s the whole function:
func parseRespondents(path string) (resps []Respondent, err error) {
defer fmt.HandleErrorf(&err, "parse %s", path)
f := try(os.Open(path))
defer f.Close()
s := bufio.NewScanner(f)
for {
name, err := parseField(s, "name")
if errors.Is(err, io.EOF) {
break
}
if err != nil {
return resps, err
}
resp := Respondent{
Name: name,
Gender: try(parseField(s, "gender")),
OS: try(parseField(s, "os")),
Lang: try(parseField(s, "lang")),
}
resps = append(resps, resp)
if !s.Scan() {
break
}
if s.Text() != "" {
return resps, errors.New("expected empty line")
}
}
return resps, nil
}
What do the error messages look like? Here are some of them:
parse respondents.txt: parse field gender: expected "gender:"
parse respondents.txt: expected empty line
parse respondents.txt: parse field lang: EOF
Not that bad. One thing that’s definitely missing is line numbers. Could we add them?
Let’s add a line counter variable:
defer fmt.HandleErrorf(&err, "parse %s", path)
f := try(os.Open(path))
defer f.Close()
s := bufio.NewScanner(f)
line := 1
We need to increment it each time we read a line, so let’s change the parseField function to do that:
func parseField(s *bufio.Scanner, line *int, name string) (value string, err error) {
defer fmt.HandleErrorf(&err, "scan field '%s'", name)
if !s.Scan() {
return "", io.EOF
}
prefix := name + ":"
if !strings.HasPrefix(s.Text(), prefix) {
return "", fmt.Errorf("expected %q", prefix)
}
*line++
return strings.TrimPrefix(s.Text(), prefix), nil
}
And modify all its usages:
name, err := parseField(s, &line, "name")
if errors.Is(err, io.EOF) {
break
}
if err != nil {
return resps, err
}
resp := Respondent{
Name: name,
Gender: try(parseField(s, &line, "gender")),
OS: try(parseField(s, &line, "os")),
Lang: try(parseField(s, &line, "lang")),
}
We also need to increment the counter when scanning the empty line:
if !s.Scan() {
break
}
if s.Text() != "" {
return resps, errors.New("expected empty line")
}
line++
And now, let’s use fmt.HandleErrorf to add the line to the error. How do we do that?
This won’t work:
line := 1
defer fmt.HandleErrorf(&err, "line %d", line)
This would always add line 1 because the arguments to a deferred function are evaluated immediately.
This also wouldn’t work:
line := 1
defer fmt.HandleErrorf(&err, "line %d", &line)
Instead of adding a line number, this would will the address of the line variable. Not what we wanted.
This would be a solution:
defer func() {
if err != nil {
err = errors.Wrapf(err, "line %d", line)
}
}()
And it’s a perfectly fine solution.
However, there is a neater (although a little hackier) solution. What if we made a type called Ref that would hold a pointer and would dereference it when required to print itself? Then we could do this:
defer fmt.HandleErrorf(&err, "line %v", Ref{&line})
We can make such a type quite easily. Here’s what it looks like when we only care about pointers to int:
type Ref struct {
Pointer *int
}
func (r Ref) String() string {
return fmt.Sprint(*r.Pointer)
}
And here’s a general solution for all types:
type Ref struct {
Pointer interface{}
}
func (r Ref) String() string {
val := reflect.ValueOf(r.Pointer).Elem().Interface()
return fmt.Sprint(val)
}
Not that hard.
The errors are now more helpful:
parse respondents.txt: line 12: parse field gender: expected "gender:"
parse respondents.txt: line 9: expected empty line
parse respondents.txt: line 4: parse field lang: EOF
Finally, here’s the whole program:
package main
import (
"bufio"
"fmt"
"io"
"os"
"reflect"
"strings"
"github.com/pkg/errors"
)
type Ref struct {
Pointer interface{}
}
func (r Ref) String() string {
val := reflect.ValueOf(r.Pointer).Elem().Interface()
return fmt.Sprint(val)
}
type Respondent struct {
Name string
Gender string
OS string
Lang string
}
func parseField(s *bufio.Scanner, line *int, name string) (value string, err error) {
defer fmt.HandleErrorf(&err, "scan field '%s'", name)
if !s.Scan() {
return "", io.EOF
}
prefix := name + ":"
if !strings.HasPrefix(s.Text(), prefix) {
return "", fmt.Errorf("expected %q", prefix)
}
*line++
return strings.TrimPrefix(s.Text(), prefix), nil
}
func parseRespondents(path string) (resps []Respondent, err error) {
defer fmt.HandleErrorf(&err, "parse %s", path)
f := try(os.Open(path))
defer f.Close()
s := bufio.NewScanner(f)
line := 1
defer fmt.HandleErrorf(&err, "line %v", Ref{&line})
for {
name, err := parseField(s, &line, "name")
if errors.Is(err, io.EOF) {
break
}
if err != nil {
return resps, err
}
resp := Respondent{
Name: name,
Gender: try(parseField(s, &line, "gender")),
OS: try(parseField(s, &line, "os")),
Lang: try(parseField(s, &line, "lang")),
}
resps = append(resps, resp)
if !s.Scan() {
break
}
if s.Text() != "" {
return resps, errors.New("expected empty line")
}
line++
}
return resps, nil
}
func main() {
resps, err := parseRespondents("respondents.txt")
if err != nil {
fmt.Println(err)
return
}
fmt.Println(resps)
}
In summary, here are the main points of this post:
- Don’t decorate what’s already decorated (e.g.
os.Open). - Use
fmt.HandleErrorfto add context. - For more granularity, break it into more functions (e.g.
parseField). - There’s no shame in falling back to
if err != nilif appropriate.
I hope I illustrated multiple aspects of the proposed try function and associated error handling constructs like fmt.HandleErrorf and showed cases where it’s usable and also cases where it’s not. Form your own opinion on it and feel free to speak about it in the comments here or on Reddit :). Thanks for reading!
32 archived comments
Posted on the previous version of this blog. They're read-only now; new comments go below.
You should totally decorate your open, just add missing information instead of repeating the filename. Why are you opening a file?
"Cannot start application: random_file.txt: No such file or directory"
What does it mean?!
It should be:
"Cannot start application: cannot read configuration file: random_file.txt: No such file or directory"
Ah much better now!
I really cannot find a place where "try" is a good idea.
That's roughly what's there, if you haven't noticed:
parse respondnts.txt: open respondnts.txt: no such file or directoryIt's not. "parse" is very generic: why are you parsing it?
So put something else there, no problem.
And how about all the other 'try's, anything wrong with them?
Yes, while 'return' is on the very left of the statement, 'try' hides in the middle-right, creating a sort of "spooky action at a distance".
With a bit of ingenuity, you can contextualize all error returns and the contextualization is a form of documentation (you can really use it instead of comments to describe your function step-by-step!)
For the boring parts (many Write in a row) there is the "errors as values" blog post which explains how to avoid the boring if-err-return.
Please don't use this discussion as a new place to post all the arguments that have already been posted everywhere.
This is a specific example that I provided and this example is definitely improved by 'try'. If it doesn't apply to your code, that's fine. As I said in the article, there's nothing wrong with 'if err != nil'.
Oh I see, you have it all sorted out already :)
Please be intellectually honest and remove this sentence from the post: "This post isn’t pro-try or anti-try."
You clearly are one sided.
I have a personal opinion. I didn't express the opinion in the article. Therefore, the article is unbiased just as it says.
It's just unproductive to use the same arguments over and over, when those arguments don't resonate with proponents of try.
Here's an argument: https://github.com/golang/g...
Griesemer's tryhard tool found that 70% of all error handling code in the standard library can be directly rewritten to use try.
Which of the error messages produced by this program do you consider insufficient?
He meant this nonsense.
As you can see in the post, I do actually add context to `os.Open`. But, I add context that's applicable to all errors returned from `parseRespondents`. I don't add context that just repeats what `os.Open` already returns.
I actually see the place: it is Go itself.
But it is really narrow case: Go's application area are networks services and CLI utilities and try is not needed there indeed.
This is pretty much a big sign of Go type system being inadequate for writing languages BTW.
Thanks for an example.
1. Ref is not necessary. You can use closure for defer. (I mean using fmt.HandleErrorf(), instead of if err != nil and Wrapf())
2. There are ways to remove repeated err != nil naturally. bufio.Scanner is an example for that. Interestingly the code is using Scanner which we can avoid handling io.EOF, but the code reintroduce that complexity again.
3. The given code totally ignores scanner error like disk read error, etc.
I think I can still write clean code without try at all. I can basically copy the behavior of the given code and make a working code in today's Go (or 1.13).
Here's the example. I think this way is better because there's no need to handle the special case for the 1st field "name", and it's cleaner in the caller's side using the API. What do you think?
https://play.golang.org/p/I...
I would do a little differently in my own project, but for demonstration purpose, I made all errors look just like the original code except that I removed "EOF" and added proper scanner errors handling. Those defer func() lines in the code are to mimic your fmt.HandleErrorf() to get the code work.
I don't like try() because it is harder to see where the function returns. When I see only 1 line of return statement like return nil in the function, I expect it always returns nil. But it doesn't work that way with try(). Make it even worse, try isn't always at the beginning of the line, so I really have to scan the whole characters to see where it could return what.
The example code I linked is not trivial to follow that either, but when I see return rs.Err(), I know it may return nil or non-nil and I can start reading what that means and how it works.
Hey, thanks for rewriting the code like that! You did an awesome job, I must say your result is really elegant.
However, it's not necessarily easier to follow than the code with try. It took a minute to figure out how the control flow in your program works and how error context works.
For example, I look at your code and I can't tell if a function will be executed at a first sight, because that depends on the internal error state. So I would say your solution has similar problems as the problems you're attributing to try, but nevertheless, it's a very nice solution.
Thanks for the comment. The *control flow* is obvious because it only returns at return statement. For example, if I put fmt.Println("something") at line 88, it will print unless the function returns early at line 79. If this was implemented with try, it may return early at try and it's harder to know that quickly.
You're right it depends on the internal error state and that may be hard to understand sometimes. But _basically what scanner does is that it doesn't do anything if it's already done_ (just like bufio.Scanner). That's it.
Does the internal error state give the same complexity? The difference is that try() changes the control flow but method on object is about behavior, and the implementation details are layered.
The out layer, the usage of RespendentScanner is same as bufio.Scanner and I can easily illustrate line 116 to 120 as Example test documentation and it's easy to understand. And that's actually the essence of the rewritten parseRespendents() nothing more than that.
Uncovering another layer, line 77 Scan() function is easy to understand. It just does what it says and early return for done scanner is just like bufio.Scanner.Scan().
One more layer, the parseField() method. Looks a bit verbose but line 48-53 is HandleErrorf, and line 54-57 is just Scan() call, but handling the error properly which was missing in the original code. The rest parsing code is not touched much.
How to read this parseField() code is to see return statements. It has 3 early returns which are indented. One is no-error return early for done scanner, next 2 are error conditions (one is bufio.Scanner error and the other is format error) and error value is set and return. When error happens the scanner is done and further scanning does nothing. That's it. Actually for this case, I may just repeat the defer contents to those 2 error returns just like what I did in parseEmptyLine() method. That's actually one line fewer and easier to read. Just did defer to mimic the original code HandleErrorf() pattern.
We looked all 3 layers. 1st is just a simple for loop, 2nd is just says what it does, 3rd is to return early for done condition and 2 error cases. That's it.
In the original code, because it does return early with try(), we're forced to use defer HandleErrorf(). In the rewritten code, I don't need to depend on defer much. I may copy repeat the code if I think that's cleaner.
You are right that your solution doesn't impact control flow, but only if we're speaking in very strict terms. The internal state of the scanner affects whether the method is going to be executed at all, which I think has a very similar complexity.
I know the article by Rob Pike and I love it :). However, that (and yours) solution has a big shortcoming: you have to reimplement it for everything, it's not reusable.
A gopher will understand `try` once and see it everywhere. With your approach, one has to first understand that specific implementation of the idea from Rob Pike and then proceed.
Also, my code is 2/3 of yours, mostly because I didn't have to implement any architecture for dealing with errors.
I understand your points and admire your code. But I still think `try` is better here and it's complexity in affecting control flow really isn't too much of a complexity.
Actually I disagree. Try isn't that reusable. You had to implement your code anyway. The rewritten code doesn't add much. It's not like 50% increase. I'll make the API easy to use anyway and add lined scanner anyway even with try.
And actually we already have a solution. I've tried using something that's almost identical to try for a while like 3 years ago. Don't have time right now I'm traveling, but I can show you almost identical code of yours that is working in Go 1, if you're interested.
If you count the lines, your code is actually 50% increase :D
And yeah sure, I'd love to see that!
About # of lines. To compare, you should at least remove mustPrepareFile() function which has all the input embedded. The number of lines are the same and it's more readable. And the rewritten code works in Go1 without try and HandleErrorf.
Okay, I only removed the file content before. So, after removing the whole `mustPrepareFile()` and its invocation, it becomes 94 LOC vs 131 LOC, which still makes my code 71% of your original one.
Your new code is 96 LOC, so it's the same length.
Here's it. This code is like 99% identical to your code and it works today.
https://play.golang.org/p/g...
I think try() is similar to panic() that it can change the control flow without explicit return. If there's a reason not to use panic, I think that's similar for try.
The try-like feature is implemented using panic, but it doesn't consume other panics that must package didn't create, so it should be safe to use.
I've just added HandleErrorf to the package today, but more robust HandleErr was there for 3 years and it could also do what HandleErrorf could do. I found that I didn't like the hidden returns, so I avoided using it.
Wow, that's actually really impressive :D. Doesn't scale as well as `try` because it needs to have a separate implementation for each type, but very nice anyway! (Btw, generics wouldn't fix that, because `try` is also variadic.)
I don't think that the reason not to use panic is because it affects control flow. I think it's more like that it's pretty cumbersome to use and authors of Go teach us that it's the best practice to use it for critical failures only.
Well, I guess we need to put `try` to a practical test. See how it works, I'm not really sure we can solve it's usefulness and impact just by hypothesising. Your package will, of course, help me personally with that evaluation before `try` is added (if it will be at all) to the language. Thanks!
In most cases, must.Any will be sufficient, it can cover all "one value, one error" cases without much stress. If there are 3 or more return values, it's normally not easy to use it in one line anyway within the expression (not just assignments). There's a way to make it "scale" better, not really difficult to be honest.
I think the reason not to use panic is because it affects control flow and that's why authors of Go teach us that way. In case of runtime errors like divide by zero or index out of bound, it's actually not possible to proceed the logic in the normal control flow structs, so it's unavoidable to use panic. Don't panic means we should minimize that control flow confusion whenever it's possible to avoid that. That's all the background of best practice and so on.
You can test it. I had done and what I found was that it doesn't add that much value. It looks useful in the beginning, but later, I found that there are better ways. It may be useful for some people. Every feature is useful for someone, that's how many languages evolved and became more and more complex. I think Go is very unique, avoiding that mistake.
Btw, I somehow missed this in your previous post:
> there's no need to handle the special case for the 1st field "name"
There still is. Your code functions incorrectly if there are no respondents in the file, or if there is a newline after one respondent and EOF follows.
Special casing the 1st field is because it may not be there at all.
Edited: Never mind, see the above (next) comment instead.
Sorry, I don't understand. Like I said I made all errors look just like the original code except that I removed "EOF". It doesn't tell "EOF" but it still report errors for the cases you're mentioning.
Try the code with your cases:
- No respondents in the file: https://play.golang.org/p/r...
- One respondent: https://play.golang.org/p/R...
- One respondent and a new line: https://play.golang.org/p/c...
- Two respondents and others work as well.
Ah, I see what you mean now. You mean it should handle empty file as a non-error case. I thought there should be at least one entry. It's quite easy to fix it. Could you try with this?
https://play.golang.org/p/9...
Yes, that fixes it, thanks! Although, it took me about 3 minutes to understand how it fixes it :D
And this blog post by Rob Pike may be helpful as well: https://blog.golang.org/err...
I would do this in the following format: https://gist.github.com/ner...
(Without try)
Seems cleaner than the `try` solution, which is pretty much substandard
I added an other approach which "lifts" the error.
Go Lift: https://www.youtube.com/wat...