I would question the framework design: the method is called "UpdateUser", so it should be executed in a transaction, so it should be a parameter of the service, and the transaction logic handled by the framework.
func (s \*Service) UpdateUser(ctx context.Context, tx models.Repo, userID string) error {
user, err := tx.GetUser(ctx, userID)
if err != nil {
return err
}
user.Name = "Updated"
return tx.SaveUser(ctx, user)
}This shows, once more, that humans are bad with modes. You have two copies of the repo, one in a transaction and one not in a transaction.
The problem is that the thing you use to build the transaction can also be used to directly manipulate the DB. A better API design would be to separate those two things.
Aside from data consistency issues mentioned, you can also quickly get yourself into connection pool exhaustion issues, where concurrent requests have already obtained a transaction but are asking for another accidentally, then all stall holding the first open until timeouts occur.
the best advice I ever received, nearly ten years ago now, from by far the best programmer I've ever met was "be your own linter", an ethos that persists in my approach to writing every single day.
Great walkthrough teaching how to DIY a Go AST linter, thanks
I wrote exactly this linter a while back after making the same mistake. Very annoying. Unlike you I did try to get it into golangci-lint but the process wore me down. In the age of LLMs maybe it'd be worth another try.
The wording "outside of transaction" irks me. Everything in a relational database is done within a transaction, the only question is whether it's the transaction you think it is, or some other.
I believe this is largely an API design problem. Many client APIs (especially ORMs) will start a transaction implicitly for you if you haven't explicitly specified your own, leading to problems like in the article.
Having implicit transactions is just wrong design, IMO. A better-designed API should make transactions very explicit and very visible in the code: if you want to execute a query, you must start a transaction yourself and then query on that transaction supplied as an actual parameter. Implicit transactions should be difficult-to-impossible. We - the programmers - should think about transactions just as we think about querying and manipulating data. Hiding from transactions in the name of "ergonomy" brings more harm than good.