The bug report came in at 9am: a user action sometimes logged a success toast, but the audit row never landed in the database. Sometimes. Not always. On a Friday.
The diff that introduced it looked completely reasonable. A helper called recordEvent(payload) had been synchronous — it wrote to a local buffer and flushed on a timer. The agent was asked to swap the buffer for an HTTP call to the audit service. To do that, it made recordEvent async, updated every caller it could find, and the PR passed review.
What the diff did not show was the one caller in an event handler that looked like this:
button.addEventListener('click', () => {
updateUI();
recordEvent({ action: 'click' });
});
No await. No .catch(). Just a floating promise. In a browser, that promise rejection goes to the console — which nobody watches in production. In Node, depending on the version and config, it either crashes the process or emits a warning that gets filtered out by the logger.
The audit service was returning 500 for about 1 in 20 requests. Before the refactor, that 500 was a synchronous throw, caught by a surrounding try/catch in the request pipeline, and surfaced as a failed audit write that ops could see. After the refactor, the 500 became an unhandled rejection in a click handler, and the user saw a green checkmark.
Why the agent missed it
The agent did what it was told. It found every recordEvent(...) call in the codebase, added await to the ones inside async functions, and moved on. It did not:
- Search for callers inside non-async callbacks (event handlers, promise
.then()chains, array methods). - Run the app and click the button.
- Add a
.catch(() => log(...))as a safety net on the fire-and-forget path. - Flag that the return type changed from
voidtoPromise<void>and that this was a contract break for any caller it couldn't see.
The linter didn't help either. @typescript-eslint/no-floating-promises was not enabled. Even if it had been, the agent would likely have added a // eslint-disable-next-line comment rather than restructuring the handler.
The fix that stuck
We didn't revert to sync. The HTTP call was the right call. What we changed:
-
Every fire-and-forget call site now has an explicit
.catch()that logs to the audit failure queue. If the audit service is down, we know about it, even if the user doesn't. -
We turned on
no-floating-promisesin CI. It's noisy at first, but the noise is exactly the list of call sites that need a decision: await it, catch it, or rename the function to make the contract obvious. -
We renamed the function to
recordEventAsyncfor a release. It was ugly, but it made the missing await impossible to miss in review. We dropped the suffix once the call sites were clean.
The broader pattern
This is a small instance of a larger failure mode: an agent changes a function's contract — sync to async, throw to return-error, required to optional, one shape to another — and updates the call sites it can statically see. The call sites it can't see are the ones that bite you: dynamic dispatch, string-based event emitters, plugin registries, and any caller in a repo it wasn't pointed at.
When the contract change is on the API boundary, the same problem shows up differently. An endpoint that used to return 200 synchronously now returns a 202 Accepted with a job id, and every mobile client that treated the response as "done" silently breaks. The fix isn't to keep the old shape; it's to make the contract change loud. We wound up doing that locally in Powerduck — diff the spec against the live endpoint before review, so a sync-to-async or 200-to-202 shift shows up as a visible change instead of a surprise in production.
What to ask the agent next time
Before merging a "make this function async" PR, ask three questions:
- Show me every caller, including the ones inside callbacks and event handlers.
- What happens to the rejection at each of those call sites?
- If I can't see the caller, what's the safety net?
The first two are mechanical. The third is the one that catches the Friday bugs.
Top comments (0)