DEV Community

Daniel Pertu
Daniel Pertu

Posted on

Our desktop app reaches our own server from six places, and only two of them can report a failure

Notifio runs on your machine. It watches rental search pages locally, keeps its data in your user data folder, and needs a server for three things only: checking that a licence is valid, sending email, and starting a Stripe Checkout session for the one-time auto-reply upgrade.

Three endpoints, then. /api/validate, /api/notify, /api/checkout/upgrade. I went looking for the call sites expecting three or four, and found six:

app/src/entitlement.ts:104   POST /api/validate          timeout 10s
app/src/entitlement.ts:184   POST /api/checkout/upgrade  timeout 15s
app/src/notify.ts:47         POST /api/notify            timeout 15s
app/src/monitor.ts:281       POST /api/notify            timeout 15s
app/src/monitor.ts:327       POST /api/notify            timeout 15s
app/src/monitor.ts:386       POST /api/notify            timeout 15s
Enter fullscreen mode Exit fullscreen mode

(Two greps, because half of them call the exported serverUrl() helper and half build the base URL inline, which turns out to be the second finding in this post.)

Six fetches, three endpoints, two timeout values, and six independent decisions about what happens when our server does not answer. This is the census.

What each one does when it fails

Call site What it sends On failure
entitlement.ts validate licence email and token returns null, meaning "we do not know"
entitlement.ts upgrade licence plus the currency shown throws, the UI shows the message
notify.ts reply notice one completed auto-reply logs a line, returns
monitor.ts listings alert the new listings found throws, so they stay unseen
monitor.ts auth failure the site we can no longer sign into logs a line, returns
monitor.ts blocked the site we can no longer check logs a line, returns

Two of the six can report a failure. Four cannot, by design, and the design is right in all four cases for the same reason: those four are all sending news about something that has already happened. A reply has already been submitted to a landlord by the time we try to email the user about it, so a failed email must not turn a successful reply into a logged failure. The code says so:

/**
 * Never throws: the reply itself has already happened by the time this runs, so
 * a failed email must not turn a successful send into a logged failure.
 */
Enter fullscreen mode Exit fullscreen mode

The two that throw are the two where throwing does something. The listings alert throws so the caller keeps those listings marked unseen and tries again on the next poll, which makes the email the commit point for the whole diff: nothing is remembered as "seen" until the alert is accepted. I have written about that fan-out before. The upgrade throws because a person is standing there waiting for a browser window to open, and the only useful thing to do is tell them.

The licence check is the interesting middle case, because its failure mode is a third thing: not an error, not silence, but an explicit "unknown".

if (!res.ok) {
  // Only a considered "no" from the server counts as one.
  //
  // A 429 used to land here and be recorded as an invalid licence, which
  // it plainly is not: the endpoint is rate limited per IP, so a burst of
  // checks (or a shared office address) could switch someone's working
  // licence off. Anything that is not the server having looked the licence
  // up and refused is "we do not know".
  if (res.status >= 500 || res.status === 408 || res.status === 429) return null;
  return { valid: false, autoReply: false };
}
Enter fullscreen mode Exit fullscreen mode

null and { valid: false } are different answers to different questions, and collapsing them is how you switch off a paying customer's app because your rate limiter fired.

The budget is spent before the request

Here is the bug the census found. Two of the four silent calls are failure notifications, and both are rate limited to one email per site per hour so that a site which stays broken does not become a mailing list. The limit is kept in memory as a timestamp per search:

const lastAlert = _lastAuthFailures[siteUrl] || 0;

// Rate limit: only alert once per hour per site for auth failures
if (now - lastAlert < 60 * 60 * 1000) {
  return;
}
_lastAuthFailures[siteUrl] = now;

// ... then the SSE frame, then the fetch, in a try/catch that logs
Enter fullscreen mode Exit fullscreen mode

The timestamp is written before the request is made. If that request fails, for any reason at all, the hour has still been consumed. The app logs "Failed to send auth failure email", drops it, and refuses to try again for 60 minutes even though nothing was ever delivered.

Both failure emails have this shape. The reply notice does not, because it has no budget. The listings alert does not, because it throws.

What makes it worth fixing rather than shrugging at is which emails these are. These are the two that exist to say the app has stopped working. A monitor that has quietly stopped checking a site looks exactly like a market with nothing new in it, which is the whole reason those two emails exist. An outage on our side is also one of the more likely moments for them to be dropped.

The mitigation already in place is real, though, and it is why this is a bug and not an incident: the SSE frame to the app window goes out before the fetch, and it cannot fail, because it is a function call in the same process. A user with the window open sees the state change immediately whatever the email does. The email is the channel for a user who is not looking.

The fix is to move the timestamp write to after a response that proves the server considered the request, with the same distinction the licence check already makes: a 4xx means it was considered, a network error or a 5xx means it was not.

Two spellings of one base URL

The other thing a census turns up is duplication nobody would notice reading any single file. entitlement.ts exports the resolver:

export function serverUrl(): string {
  return (process.env.NOTIFIO_SERVER_URL || 'https://notifio.app').replace(/\/$/, '');
}
Enter fullscreen mode Exit fullscreen mode

notify.ts imports it. monitor.ts does this instead, three times:

const serverUrl = process.env.NOTIFIO_SERVER_URL || 'https://notifio.app';
Enter fullscreen mode Exit fullscreen mode

Same default, no trailing-slash strip. So setting NOTIFIO_SERVER_URL=http://localhost:3001/ in development gets you a working licence check and three requests to //api/notify. A development-only sharp edge, which is exactly the sort that costs an hour at the worst possible time.

There is also a claim in the repo that is no longer true. notify.ts opens with this:

/**
 * Outbound notifications to the Notifio server, which owns the email sending.
 *
 * The app never talks to an SMTP provider itself, it posts to /api/notify with
 * the licence, and the server decides what to send. This module is the one place
 * that knows the shape of that request.
 */
Enter fullscreen mode Exit fullscreen mode

It is not the one place. It knows one of the four payload shapes that endpoint accepts; monitor.ts builds the other three inline. The first sentence is still the important one and still true: the app holds no email credentials and cannot send mail itself, which is why the licence travels with every notification.

What maintenance mode makes of all this

There is a committed boolean on the server that takes everything needing the database offline, and its allowlist is the clearest statement of which endpoints matter:

/**
 * API routes that never touch the database, so they stay up during
 * maintenance: the download page's links, the help page's contact form, and
 * inbound email forwarding. Every other API route is refused, so a route added
 * later is safe by default.
 */
const NO_DATABASE_ROUTES = ["/api/download", "/api/contact", "/api/webhooks/resend"];
Enter fullscreen mode Exit fullscreen mode

None of the three endpoints the desktop app uses is on that list, and none of them could be: all three start by looking a licence up. So during maintenance all six call sites get a 503 with a Retry-After, and the six behaviours above are exactly what a user experiences:

  • the licence keeps working, because 503 is "we do not know" and the cached entitlement stands
  • new listings are not lost, because the alert throws and the diff is retried
  • a completed reply's confirmation email is lost
  • a failure notification is lost, and its hour is gone with it

Three out of four of those are the intended outcome of decisions made one at a time, in different files, months apart. The fourth is the one to fix, and I only found it by putting all six in a table.

If you want to see the side of this the user actually touches, the help page covers what each email means and what to do when one arrives, and the app itself is on the download page.

Top comments (0)