---
title: "When Operations Can Fail Partway: Use Rollback, Not Error Handling"
canonical: https://dxdev.com/blog/2026-08-30_rollback-not-error-handling/
datePublished: 2026-08-30
---
The reconcile only ever retries removed rows. That's the whole bug in one sentence, and it took a half-written domain to find it.

The ticket came in as a report from my co-founder: `Domain_SetForCustomer` can fail after the code has already cleared the domain and updated the row, and there's no rollback. The reconciliation sweep that's supposed to catch broken domains only looks at rows where `removedOn IS NOT NULL`. Once this bug fires, the row doesn't have that anymore, so the retry can't see it. The account just sits broken, forever, invisible to the one process built to fix it.

## The three writes

The function is `DomainReconnectRemoved()` in `www/adminApp/DomainReconcile-s.asp`, and it does three separate writes with no transaction wrapping them:

1. `UPDATE leagueapp.customers SET sitemode = ...`
2. Un-retire the domain row: `UPDATE leagueapp.domains SET removedOn = NULL WHERE domainID = ...`
3. `Domain_SetForCustomer(uname, dom, byLabel, 'reconnect')`, which calls the `SetCustomerWebDomain` stored proc and sets `customers.webdomain`

If step 3 fails, the code returns early:

```
if(domErr) { res.ok = false; res.action = 'reconnect-failed'; ... return res; }
```

Steps 1 and 2 are already committed by then. So the account ends up with an active `domains` row, a flipped `sitemode`, and no `webdomain`. And because the reconcile's retry query filters on `removedOn IS NOT NULL`, the un-retire in step 2 is exactly what removes the row from that query. The account isn't just broken, it's broken in a way that erases the trail back to fixing it.

## Why you can't just reorder

The obvious fix is to run the risky call first and only commit the safe stuff after it succeeds. That doesn't work here, for two concrete reasons in the same proc: `SetCustomerWebDomain` reads `customers.sitemode` to decide whether the account is client-managed, so step 1 has to happen before step 3 runs. And the un-retire in step 2 is what makes the row active, which is what makes step 3's `UPDATE` hit the existing row instead of inserting a duplicate. Both writes are ordered on purpose. Swapping them trades a resumable failure for a data-integrity one.

Classic ASP with a SQL Server backend and no ORM here means there's no ambient unit-of-work to lean on either. Wrapping all three in a single SQL transaction was the other option on the table, but step 3 goes through a stored proc that's called from other paths too, and scoping a transaction around a proc call we didn't fully control felt like the wrong place to introduce that risk on a hotfix.

## What shipped: compensating undo

Before touching anything, the fix captures the row's original `removedOn` and the account's original `sitemode`. If the `SetCustomerWebDomain` call fails, both get put back before the function returns:

- `removedOn` gets restored to its original value, not `GETDATE()`. A fresh timestamp would restart the 30-day reconnect window and make an old removal look recent, which is its own kind of lie.
- `sitemode` gets restored so the account isn't left flagged as something it isn't.

That's a rollback implemented as application-level compensation instead of a database transaction, because the constraint here wasn't atomicity in general, it was making the retry query true again. Put the row back exactly where the reconcile expects to find broken rows, and the existing sweep does the rest. No new detection path, no new query, just restoring the state the current mechanism already knows how to act on.

One more layer: if the undo itself fails, that gets said explicitly in `res.error` and carried into the domains@ notice, instead of being swallowed. A failed undo is exactly the half-written row this ticket exists for, so it doesn't get to fail silently twice.

## The bug next to this one, left alone

While in that file, we found that when Cloudflare or relay registration fails partway through a reconnect, the failure messages get collected into `cfMsgs` and reported in `res.error`, but the function still returns `action: 'reconnected'`. Partial success reported as full success, one line away from the code being touched. Same category of bug, and it didn't go in this hotfix. Different behavior change, different blast radius, and this ticket already had two defects in it. Filed as a follow-up instead of folded in, because a hotfix that starts absorbing adjacent bugs is how a one-file, +98/-16 change turns into something nobody wants to review at 5pm.

The second defect that did ship alongside this: the domains@ notification emails build their HTML by string concatenation through a `Clean()` function that, despite the name, doesn't escape anything, it just stringifies nulls into empty strings. There's a real `HTMLEscape` in the same file that nothing was using for these values. Staff-writable field, staff-only reader, so not urgent on its own, but it's the same fix effort as the write-path bug and touches the same lines, so it went out together.

We shipped it as one hotfix branch, one file, verified against production by reading the deployed file on the box after release. The reconcile sweep didn't need a single line changed. It was already correct. It just needed the data handed to it to stop lying about its own state.