Bonjour,
Throwing errors with panic and catching them was not initially a feature of the Go language. To this day libraries are still expected to return errors instead of using panic. Even relatively large applications such as Gitea are mostly implemented by functions and methods that return errors.
Code inflation
Every function call follows a pattern similar to:
err, value := myfunction()
if err != nil {
return err
}
instead of the simpler:
value := myfunction()
when myfunction() calls panic on error instead of returning. This artificially inflates the codebase with lines that are almost exactly the same with every function call. Here is how the deleteBoardByID function currently looks in Gitea:
func deleteBoardByID(ctx context.Context, boardID int64) error {
board, err := GetBoard(ctx, boardID)
if err != nil {
if IsErrProjectBoardNotExist(err) {
return nil
}
return err
}
if err = board.removeIssues(ctx); err != nil {
return err
}
if _, err := db.GetEngine(ctx).ID(board.ID).NoAutoCondition().Delete(board); err != nil {
return err
}
return nil
}
and here is how it could look if using panic instead:
func deleteBoardByID(ctx context.Context, boardID int64) {
if board := GetBoard(ctx, boardID); board != nil {
board.removeIssues(ctx)
db.GetEngine(ctx).ID(board.ID).NoAutoCondition().Delete(board)
}
}
Seasoned Gitea developers adding if err { return err } got used to it and, to be fair, it is only marginally error prone. But it requires a very strict discipline otherwise the errors will be cryptic.
Creating meaningful errors is difficult
When an error happens deep down the call stack of Gitea, for instance if a color is badly formatted, it will craft an error that looks like this:
return fmt.Errorf("bad color code: %s", label.Color)
and it will bubble up to the caller via a series of if err { return err } until it is displayed, in a log file, as an error in a unit test, as a message wrapped in a JSON object sent back via the API, etc. And eventually a human being is left with the question: where does that come from?
A grep in the codebase will show it could come from a few different lines that create exactly the same message:
models/issue_label.go:150: return fmt.Errorf("bad color code: %s", label.Color)
models/issue_label.go:182: return fmt.Errorf("bad color code: %s", label.Color)
models/issue_label.go:194: return fmt.Errorf("bad color code: %s", l.Color)
models/project/board.go:127: return fmt.Errorf("bad color code: %s", board.Color)
models/project/board.go:201: return fmt.Errorf("bad color code: %s", board.Color)
and each line is potentially called by multiple code path. In the best case scenario the context in which the error happens allows the developers to uniquely pinpoint where and how it occurred. Otherwise they are left guessing and speculating.
To mitigate this problem, it is recommended that instead of returning the error, some context is added to help with debugging. For instance this is good practice:
if err := issues.loadPullRequests(ctx); err != nil {
return fmt.Errorf("issue.loadAttributes: loadPullRequests: %v", err)
}
and this would be frowned upon:
if err := issues.loadPullRequests(ctx); err != nil {
return err
}
The goal here is that the message is meaningful to the reader and uniquely identifies both the code path and the line from which it originates.
Unfortunately this requires a discipline that is so difficult that thousands of bare return err are in the Gitea code base and more are added on a regular basis, with no sign that the situation is improving.
Panic to the rescue
The primary reason why I like to be able to throw an error that will be caught upper in the call stack, via raise in Python or panic in Go is that it not only removes a considerable amount of unnecessary code but because it provides a stack trace.
The stack trace provides all the information needed to identify the line where the error actually happens regardless of how careful the developer was when crafting the error message. And it also uniquely identify the code path that was used to reach the error, even when there can be dozens.
The gofff library is meant to be used in Gitea and the API it provides returns errors. But it uses panic internally which allows it to log meaningful errors. Here is what showed today while running the Gitea dump restore integration test with gofff instead of the Gitea migration code:
Without panic that’s all the developer would have to work with. But since gofff uses panic internally, it also logs the stack trace:
The developer immediately knows it originates from this line and can start debugging.
Gitea would be better off using panic
It is my understanding that the debate between using panic instead of returning errors sometime meets passionate feelings that can be seen when people discuss about tabulations and spaces in their code. I hope that by explaining as clearly as I could where I stand and why it will help others make an informed decision on the matter.