A payment call started timing out in production about an hour after a Thursday merge. The change that broke it had been reviewed, approved and green. So had the change it collided with. Neither of them was wrong on its own, and no test in the repository would have failed if you ran them separately, which is exactly what we had done.
One pull request changed a shared retry helper so that its timeout argument was expressed in milliseconds rather than seconds, and updated all seventeen call sites. The other, opened nine days earlier and merged forty minutes later, added an eighteenth call site passing 30, meaning thirty seconds. Git merged both cleanly because they touched different files. The result compiled, passed type checks, and gave our payment provider thirty milliseconds to respond.
The reason CI did not catch it is worth being precise about. Our checks run on the pull request's merge result, which sounds like it covers this, but that merge result is computed when the event fires. The second branch had last been pushed to nine days earlier. Its green check was a statement about a main branch that had received sixty commits since. Nobody had turned on the setting requiring a branch to be up to date before merging, because years ago somebody measured how much rebasing that would cost and decided against it.
We turned on a merge queue instead. Pull requests enter a queue, the queue builds the actual combination it is about to create, and it merges only if that combination is green. Where the queue was too heavy for the repository we settled for the up to date requirement plus a check that expires: any required status older than twenty-four hours is treated as absent, so a stale green cannot authorise a merge.
The unit change got a smaller guard too. That argument is now a duration type rather than a number, so the eighteenth call site would not have compiled at all. Types are the cheaper half of this fix and I would reach for them first.
Green on your branch means your change works next to the code you started from. Something has to check it next to the code that is actually there.
– Sergey Shinder
Top comments (0)