An upsert that fails when nothing changed
A heartbeat that repeated the stored usage reading threw an error and skipped the alerts behind it. The repeat case needs its own test.
One of our NestJS backends stores a usage reading for each device and source. A small agent on the device sends a heartbeat every so often with its current readings, and the backend writes them to a table keyed by device and source. When a reading crosses a threshold, a second step raises an alert for the owner. The write is an upsert: insert the row, or update it if the key already exists.
For a while the upsert looked fine. Then we found a case where it threw, and the alert step behind it never ran.
The symptom was a missing alert
Nothing crashed in a way anyone would notice. A heartbeat arrived, the usage write failed, the request errored, and the alert logic that comes after the write was skipped. The next heartbeat would normally fix that, because the readings change. The case that did not fix itself was the one where the device reported exactly what was already stored.
So the failure had a narrow shape: the second identical report in a row. The first report inserted a row and worked. A report with new numbers updated a row and worked. Only the repeat failed, and a repeat is easy to miss because few people write a test that says "send the same thing twice and expect nothing to happen".
Why the repeat is different
An upsert on MySQL is an insert with an on duplicate key update clause. When the update would set every column to the value it already has, MySQL reports that no row changed. That is correct, and it is a different outcome from both an insert and a real update.
The repository helper we were calling, TypeORM's upsert, is built for the general case and tries to hand back the saved rows afterwards. I have not traced exactly where in its internals the unchanged case goes wrong, and I would rather say that than invent a tidy explanation. What we established is the behaviour: the helper's generic result handling did not cope with a write that changed nothing, and the error surfaced in our code path.
For this table we never used the returned rows. The service writes readings and returns nothing. We were paying for a feature we did not use, and it was the part that broke.
The test that made it repeatable
We did not start with the fix. We started with an end-to-end test that sends the same report twice. The interesting part is what it asserts between the two beats:
await beat(computerKey, { usage: report }).expect(201);
expect(alertDetails(await today())).toEqual([expect.stringMatching(/^Reported by Studio\./)]);
await renameComputer("Office");
await beat(computerKey, { usage: report }).expect(201);
expect(alertDetails(await today())).toEqual([expect.stringMatching(/^Reported by Office\./)]);
The second beat carries identical readings, but we rename the device in between. That gives the alert step something visibly different to do. If the repeat throws and the alert is skipped, the alert still says "Studio" and the test fails. A test that only checked for a 201 on the second beat could pass while the alert silently went stale, so the rename is what makes the assertion mean something. A final check reads the stored readings back and confirms they match the report.
On the old code the second beat failed. That is the version of the test worth having: it fails before the change and passes after it.
Writing the write by hand
The fix replaces the helper with an explicit query builder insert:
await this.meters
.createQueryBuilder()
.insert()
.into(Meter)
.values(rows)
.orUpdate(["ownerUserId", "label", "planType", "windows", "capturedAt"], ["computerId", "source"])
.updateEntity(false)
.execute();
Two details carry the fix. orUpdate names the columns to refresh and, separately, the key columns that define a conflict, so the intent is on the page rather than inferred. And updateEntity(false) tells TypeORM not to read generated values back into the objects we passed in, the step we had no use for.
What the explicit version gives up
It is more code than a one-line helper, and it names the table's columns in the service, so a new column on the entity will not be refreshed until someone adds it to the list. We accepted that, because the list is short and the service is the only writer. If several places wrote this table we would put the statement in one repository method, not copy it.
The habit worth keeping
Any upsert has three outcomes: it inserts, it changes a row, or it finds a row that already matches. Write a test for each, and make the third one send the same input twice with something downstream that would visibly go wrong. The path where nothing happens is the one nobody exercises, and a periodic job such as a heartbeat can take it often.
0 comments