I shipped a transaction bug, so I built a linter

https://leonh.fr/posts/go-transaction-linter/

by leonhfr • 3 months ago

53 12 3 months ago

mnahkies• 3 months ago

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.

dgunay• 3 months ago

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.

rowyourboat• 3 months ago

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.

SwiftyBug• 3 months ago

Interestingly, modal editing comes very natural to us. It enters muscle memory quite well.

branko_d• 3 months ago

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.

black_knight• 3 months ago

I love how this is done in Software Transactional Memory (STM) in Haskell. There, the transaction code happens in its own type (monad), and there is an explicit conversion function called `atomically :: STM a -> IO a`, which carries out the transaction.

This means that the transaction becomes its own block, clearly separated, but which can reference pure values in the surrounding context.

    do
       …
       some IO stuff 
       …
       res <- atomically $ do
          …
          transaction code
          which can reference
          results from the IO above
          …
       …
       More IO code using res, which is the result of the transaction
       …

cben• 3 months ago

Great walkthrough teaching how to DIY a Go AST linter, thanks

dolmen• 3 months ago

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)
  }

TN1ck• 3 months ago

In that instance, you are right, but there are often cases where you need to do multiple queries / updates spanning multiple tables in a single transaction, then you do need a generic transaction wrapper.

jpfromlondon• 3 months ago

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.

literallyroy• 3 months ago

Hey this is exactly the pattern we have and the bugs we need to catch… yoink?

hbogert• 3 months ago

This is the way, stop relying on somebody else to stop doing dumb things and write your linter today.

I shipped a transaction bug, so I built a linter

Related Stories