proposal: bufio: add Scanner.Tokens iterator

[ianlancetaylor]

Proposal Details

The bufio.Scanner type breaks an input stream into a series of tokens. It is convenient to use but has a flaw: when an error occurs, it stores the error internally, and makes it available via the Err method. This is fine in principle but in practice people routinely fail to call the Err method.

A particular error that can occur is ErrTooLong which means that the next token is too long for the buffer. On balance it's a good idea that the scanner does not permit arbitrary size tokens. Scanners are routinely used to parse input that is not under the program's control, and arbitrarily large tokens would open up a door to allocating large amounts of memory in the program. But since people routinely don't check the Err method, it's easy for an overly long token to be silently discarded.

Now that we have iterators, we can make things slightly better by using an iterator that returns a token and an error. It's still possible to ignore the error. But it's harder. It's comparable to forgetting err != nil, rather than being comparable to just continuing the loop calling scan.Scan.

The proposal is to add this method:

// Tokens returns an iterator over a pair of the next token and an error.
// At most one of the token and error will be non-nil.
// If the error is not nil, the iterator will not yield any further values.
// This is like a loop that calls the Scan method,
// except that the error is explicitly returned (as well as being stored for the Err method).
func (s *Scanner) Tokens() iter.Seq2[[]byte, error]

To use it, instead of writing

    for scan.Scan() {
        tok := scan.Bytes()
        // process tok
    }
    if err := scan.Err(); err != nil {
        // handle error
    }

people will write

    for tok, err := range scan.Tokens() {
        if err != nil {
            // handle error
            break // or return
        }
        // process tok
    }

[robpike]

I'm not enamored with this, as part of the point of the design of Scanner was that the error only had to be checked once the loop was complete. You've gone back to the old way here, which I find sad.

But there are many sadnesses in how things advance, so maybe your way is better.

[jimmyfrasche]

I'd prefer a linter to check for calling Err and:

    for tok := range scan.Tokens() {
        // process tok
    }
    if scan.Err() != nil {
        // handle error
    }

[gabyhelp]

Related Issues

• proposal: bufio: Scanner.{Text,Bytes}Seq #70657 (closed)
• proposal: encoding/xml: add iterators Decoder.Tokens, Decoder.RawTokens #68259
• proposal: bufio: add option to Scanner to skip tokens that are too long #26466 (closed)
• proposal: slices: add CollectError #70631
• proposal: Go 2: iterators #40605 (closed)
• proposal: fmt: add iterator for format tokens #68279 (closed)
• bufio: Support empty final token from a SplitFunc #11836 (closed)
• bytes, strings: add iterator forms of existing functions #61901 (closed)
• proposal: iter: SeqError type, Throw function #67924 (closed)

Related Discussions

• proposal: new interface for scanning lines in bufio

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[ianlancetaylor]

@robpike I'll note that before iterators there was no good way to write the loop to permit error checking at each step.

@jimmyfrasche The vet check is #17747. That proposal is on hold, though it shouldn't be.

[robpike]

@ianlancetaylor I don't understand your first point. The type was designed for the language as it was at the time. We could have asked the user to check each iteration but chose not to. I don't see what iterators change in that.

[ianlancetaylor]

I mean that writing the loop is awkward. Today you write

    for scan.Scan() {
        // call Bytes or Text
    }

which is really nice. If Scan also returned an error, there is no good way to write that loop. You wind up with something like

    for err := scan.Scan(); err == nil; err = scan.Scan() {
        ...
    }

or

    for {
        if err := scan.Scan(); err != nil {
            // handle error
        }
        ...
    }

Those are not nearly as nice. As you say, the type was designed for the language at the time, and it was the right choice.

With iterators, you can write

    for val, err := range scan.Tokens() {
        // check error
        // handle val
    }

which is more painful than the current loop, but I think is better than the alternatives that were available when you designed the type. And, I'm arguing, it's less error prone.

[cespare]

We should think carefully before establishing a pattern in the standard library of iterators that return errors as the second value in a Seq2. Right now the "standard" use of iterators seems to be for things that can't fail. But a pattern of Seq2[X, error] opens a whole can of worms. Should we have an iterator version of os.ReadDir? fs.Walk? Should we have an iterator-based alternative to database/sql.Row? And so on.

The farther we get from what I think of as the core strength of iterators (writing data structures that are as syntactically convenient to use as Go's built-in data types), the smaller the benefit seems to be, and the larger the potential for wide-scale code churn.

[nickajacks1]

I encountered this same error handling dilemma in a personal project where I made an iterator that yielded resources from a paged HTTP API. I ended up going with Seq[T] and a check to a Err() function after the loop because it fit my use case marginally better, roughly something like this:

func pageIter[T any](c *Client, endpoint string) iter.Seq[T] {
	return func(yield func(T) bool) {
		next := endpoint
		for next != "" {
			r, err := fetchPage[T](c, next)
			if err != nil {
				c.err = err
				return
			}
			for _, v := range r.Values {
				if !yield(v) {
					return
				}
			}
			next = r.Next
		}
	}
}

I didn't like how I would be redundantly checking if err != nil even though only every Nth iteration would actually return an error (admittedly, this wouldn't have had any tangible impact on my program). But I can definitely see the benefit of yielding an error, especially when you might not want to stop on the first error encountered, such as when listing a directory. I think there are very valid uses for both approaches.

[apparentlymart]

This is one of a growing family of issues that are implicitly introducing some pattern for "fallible sequences" only as a side-effect of meeting some other goal, rather than as the core of the proposal. Therefore the discussion about the different ways that "fallible sequences" could be represented has been spread awkwardly over all of those separate proposals rather than happening in a central place.

Perhaps it would be worth putting all of the existing proposals that involve "fallible sequences" temporarily on hold, and then having a central discussion about a proposal about introducing a single consistent pattern for fallible sequences across the entire standard library. I expect that whatever the standard library adopts would then also become idiom in the ecosystem.

I'm going to resist the temptation to repeat all of my previous concerns about this iter.Seq2[T, error] pattern in particular here, since I already took up a bunch of space on some of the other issues talking about that. But I do hope that a central discussion about it could finally settle on one idiomatic way to deal with it, and then all of the other downstream proposals can adapt to that. 🤞

Edit: I remembered after posting this that there was some discussion about this in #71901, though that issue has a broader scope than just fallible sequences.

[earthboundkid]

Does Google have internal experience with using the iter.Seq2[T, error] idiom? Iterators have been around for a little bit now, so I'd expect we have some real world data on it by now. In my personal attempt to write a nice API fallible iterators, I ended up having a function consume an error policy instead.

// Walk filesystem and halt on error
tr := walker.New(fsys, ".", walker.OnErrorHalt)
for path := range tr.FilePaths() {
	fmt.Println(path)
}
if tr.HasError() {
	// something
}

---

Separate Q: Does this reuse the byte slice? Seems like it should say the byte slice is only valid for the current iteration.

[ianlancetaylor]

@earthboundkid The iter.Seq2[T, error] pattern is used quite a bit in the golang.org/x/oscar code.

[mvdan]

#71901 tracks documenting general guidance on iterators, including what to do with errors.

I've been using iter.Seq2[T, error] in a number of libraries and internal packages and I'm generally happy with it. It reminds me of io.Reader where one loops over int, error results. This interface is also flexible enough to accomodate for iterators where errors can be non-fatal and the user may wish to continue iterating.