Hi Mike, On Thu, 29 Jun 2023 22:04:01 -0700 (PDT) Mike Schinkel <m...@newclarity.net> wrote:
> There are many aspects of this proposal that I really like, and a few > that I think need to be reconsidered. > > In general I like that it: > > 1. Does not try to handle errors *above *where the errors > occurred, which is the pattern when using both *defer* and > *recover(). *I personally find having to jump back much harder to > reason about than jumping forward to find the error handler. > 2. Does not require handling the error in a closure or other > *func* as the former adds a lot of visual noise and the latter > complicates by creating a different variable scope. > > > It is in many ways similar to a proposal I have been planning on > preparing but have no found the right opportunity. Possibly this is > the right opportunity and maybe taking aspects of this and aspects of > mine we could find a solution beneficial enough that Ian Lance Taylor > would consider it a big enough improvement. I'll try to follow up on > that soon. > > As for the specifics of this proposal: > > 1. I think *when err handle <label>* is a bit too much magic. I > think it would be more Go-like if it were instead: > > *when err!=nil handle <label>* > > I know this makes for more repeated syntax, but within the scope of > this proposal I do not think you can avoid it without it adding > magic. I do think that could be addressed, and what I have in mind > addresses that. But I wanted to first comment on your proposal, not > introduce mine. If we use WHEN like this, then there is no different with IF statement. The purpose of WHEN statement is to check for non-zero value. Using IF statement, it could be written like these if !ZeroValue(err) handle <label> > > 2. As for *check* as you have proposed it — handling zero return > values — it seems to add more magic, and that it would only work in > selected use-cases so it might be best to omit this from the proposal? I am not sure on this one. The "check" keywords is proposed by parent proposal [1], not by this proposal. [1] https://go.googlesource.com/proposal/+/master/design/go2draft-error-handling-overview.md > > 3. I do like the concept of labels that do not fall through to the > code in the next label. However, it is not exactly clear how they > should work. Do your handlers always have to have a *return* > themselves? > > > *func foo()error {* > * ...* > * when err != nil handle cleanup* > * ...* > * return nil* > > :*cleanup:* > *doCleanup()* > * return err* > *}* No. The body of handler is similar with body of function. If the function does not expect return value than the execution of function body will stop before the first handler defined. Case example, func main() { hex, err := ioutil.ReadAll(os.Stdin) when err handle fatal data, err := parseHexdump(string(hex)) when err handle fatal os.Stdout.Write(data) // The function body stop here, the log.Fatal below will not be // executed. :fatal: log.Fatal(err) } > > 4. When you are in a handler, is there any way to correct the problem > and *resume* or *retry* the code where you left off without resorting > to using *goto* and labels? No, but one can use by using recursive call. IMO, the part of code that try handling error by resuming or retry is an indication that it should be moved into separate function. > > 5. I do not think you need the new keyword *handle*, i.e. I think > this would be sufficient to use *goto* instead since the syntax of > your handler label with the preceding colon should be sufficient to > distinguish between the two: > > > *when err!=nil goto <label>* That is possible, yes, I think it will minimise number of new syntax introduced by this proposal. I believe the implementation in the Go internal would be rather complex. > 6. If you use *goto* instead of *handle* then you actually have two > orthogonal features, the latter one being labels that do not fall > through that can be used without when. Those could be proposed as a > feature in their own right. > > 7. How would your proposal handle shared cleanup? As we can see from > the following example I have to call *db.Rollback()* for every error > case, and I would really rather only have to call it in one place in > the case. Does your proposal have a solution for this that I missed? > > > *func (db *Database) Transfer(from, to Account, amount int) (err > error) {* > *err = db.Begin()* *when err != nil **goto noTrans* > *if **from* > *.Balance() >= amount {* > * goto noFunds* *}* > *err = **from* > *.Withdraw(amount)* *when err != nil **goto **noWithdraw* > > *err = to.Deposit(amount)* *when err != nil **goto **no**Deposit* > > *err = db.Commit()* *when err != nil **goto **no**Commit* > > *return nil* > *:noCommit:* > *db.Rollback()* > *return fmt.Errorf("cannot commit; %w", err)* > *:noDeposit:* > *db.Rollback()* > *return fmt.Errorf("cannot deposit; %w", err)* > *:noWithdraw:* > *db.Rollback()* > *return fmt.Errorf("cannot withdraw; %w", err)* > *:noTrans:* > *db.Rollback()* > *return fmt.Errorf("cannot begin transaction; %w", err)* > *:noFunds:* > *db.Rollback()* *return errors.New("no funds")* > *} Assume that we use the current proposal syntax, it could be written like below (I am not fans of multiple joined "if ;" statements) func (db *Database) Transfer(from, to Account, amount int) (err error) { var tx *sql.Tx tx, err = db.Begin() when err handle errDbBegin noBalance = from.Balance() < amount when noBalance handle noFunds err = from.Withdraw(amount) when err handle errWithdraw err = to.Deposit(amount) when err handle errDeposit err = tx.Commit() when err handle errTxCommit return nil :errDbBegin: err = fmt.Errorf("cannot begin transaction: %w", err) return err :errWithdraw: err = fmt.Errorf("cannot withdraw: %w", err) handle failed :errDeposit: err = fmt.Errorf("cannot deposit: %w", err) handle failed :errTxCommit: err = fmt.Errorf("cannot commit: %w", err) handle failed :failed: tx.Rollback() return err } > *8. Using the example above, is there not a way to also > annotate the error in a shared manner vs. having to have all the > different handle labels and duplicated code? If by "annotate the error" you mean adding log prefix before returning the error, then yes. -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/20230709174327.6eef9a2c%40inspiro.localdomain.
pgp6eECW3aPCf.pgp
Description: OpenPGP digital signature