Async Error-Handling Anti-Patterns — Refactoring Practice¶

test("a price-service failure is masked as $0 (documents the bug)", async () => {
  mockFetchPrices.mockRejectedValue(new Error("price service down"));
  await expect(loadCartTotal("u1")).resolves.toBe(0); // wrong: looks like an empty cart
});

// After — linear, and the catch distinguishes "no cart" from "service failed".
async function loadCartTotal(userId: string): Promise<number> {
  try {
    const user = await fetchUser(userId);
    const cart = await fetchCart(user.cartId);
    const prices = await fetchPrices(cart.items);
    return prices.reduce((a, b) => a + b, 0);
  } catch (err) {
    if (err instanceof EmptyCartError) return 0; // the ONE case where 0 is correct
    throw err; // real failures propagate
  }
}

test("service failure now propagates", async () => {
  mockFetchPrices.mockRejectedValue(new Error("price service down"));
  await expect(loadCartTotal("u1")).rejects.toThrow("price service down");
});
test("genuinely empty cart still returns 0", async () => {
  mockFetchCart.mockRejectedValue(new EmptyCartError());
  await expect(loadCartTotal("u1")).resolves.toBe(0);
});

test("audit failure is swallowed and delete still 'succeeds' (bug)", async () => {
  mockWriteAudit.mockRejectedValue(new Error("audit log unavailable"));
  // Resolves cleanly even though the audit write rejected in the background.
  await expect(deleteAccount("u1")).resolves.toBeUndefined();
});

// After — the audit is part of the operation's contract, so we await it.
async function deleteAccount(id: string): Promise<void> {
  await db.deleteUser(id);
  await writeAudit({ action: "delete", id }); // failure now propagates
}

test("audit failure now fails the operation", async () => {
  mockWriteAudit.mockRejectedValue(new Error("audit log unavailable"));
  await expect(deleteAccount("u1")).rejects.toThrow("audit log unavailable");
});

test("forgotten await sends to undefined (bug)", async () => {
  mockGetUser.mockResolvedValue({ name: "Ada" });
  await welcome("u1");
  expect(mockSendEmail).toHaveBeenCalledWith(undefined, "Welcome!"); // wrong recipient
});

// After — deliberately backgrounded, but its failure has an owner: the logger.
async function getProduct(id: string): Promise<Product> {
  const product = await db.getProduct(id);
  void warmRelatedCache(id).catch((err) =>
    logger.warn({ err, id }, "cache warm-up failed (non-fatal)")
  );
  return product;
}

test("warm-up failure is logged, not thrown, and does not affect the response", async () => {
  mockWarm.mockRejectedValue(new Error("redis timeout"));
  const warn = jest.spyOn(logger, "warn");
  const captured: unknown[] = [];
  process.once("unhandledRejection", (e) => captured.push(e));

  await expect(getProduct("p1")).resolves.toEqual(expect.objectContaining({ id: "p1" }));
  await flushMicrotasks();

  expect(warn).toHaveBeenCalledWith(expect.objectContaining({ id: "p1" }), expect.any(String));
  expect(captured).toHaveLength(0); // ← no unhandledRejection
});

// After — a tiny supervisor owns every background task.
class TaskSupervisor {
  private inFlight = new Set<Promise<unknown>>();

  run(label: string, fn: () => Promise<void>): void {
    const p = fn()
      .catch((err) => logger.error({ err, label }, "background task failed"))
      .finally(() => this.inFlight.delete(p));
    this.inFlight.add(p);
  }

  async drain(): Promise<void> {
    await Promise.allSettled([...this.inFlight]); // wait for in-flight work
  }
}

const tasks = new TaskSupervisor();

function handleSignup(req: Req, res: Res) {
  const user = createUser(req.body);
  tasks.run("welcome-email", () => sendWelcomeEmail(user.email)); // tracked + logged
  res.status(201).json({ id: user.id });
}

// shutdown wiring
process.on("SIGTERM", async () => {
  server.close();
  await tasks.drain(); // no in-flight send is killed mid-flight
  process.exit(0);
});

test("a failed send is logged, not lost", async () => {
  mockSend.mockRejectedValue(new Error("smtp down"));
  const err = jest.spyOn(logger, "error");
  handleSignup(reqFixture, resFixture);
  await tasks.drain();
  expect(err).toHaveBeenCalledWith(
    expect.objectContaining({ label: "welcome-email" }), expect.any(String));
});

test("drain waits for in-flight tasks", async () => {
  let settled = false;
  mockSend.mockImplementation(() => delay(50).then(() => { settled = true; }));
  handleSignup(reqFixture, resFixture);
  await tasks.drain();
  expect(settled).toBe(true); // drain didn't return until the send finished
});

// After — order + outbox row committed atomically; a relay does the send durably.
async function placeOrder(req: Req, res: Res) {
  const order = await db.transaction(async (tx) => {
    const o = await tx.insertOrder(req.body);
    await tx.insertOutbox({
      type: "receipt-email",
      payload: { orderId: o.id, email: o.email },
    }); // same tx — atomic with the order
    return o;
  });
  res.status(201).json({ orderId: order.id });
}

// relay (separate process / worker) — at-least-once delivery, restart-safe
async function relayOutbox() {
  const rows = await db.claimPending("outbox", { limit: 100 });
  for (const row of rows) {
    try {
      await dispatch(row); // e.g. sendReceiptEmail
      await db.markDone(row.id);
    } catch (err) {
      logger.error({ err, id: row.id }, "outbox dispatch failed; will retry");
      await db.scheduleRetry(row.id); // backoff; row stays pending
    }
  }
}

test("order and outbox row commit atomically", async () => {
  const { order } = await placeOrderTx(fixture);
  const rows = await db.query("SELECT * FROM outbox WHERE payload->>'orderId' = $1", [order.id]);
  expect(rows).toHaveLength(1);
});
test("a crash before the send loses nothing — relay still delivers", async () => {
  await placeOrderTx(fixture); // simulate: handler returns, process restarts
  mockDispatch.mockResolvedValue(undefined);
  await relayOutbox();
  expect(mockDispatch).toHaveBeenCalledWith(expect.objectContaining({ type: "receipt-email" }));
});
test("a transient send failure is retried, not dropped", async () => {
  await placeOrderTx(fixture);
  mockDispatch.mockRejectedValueOnce(new Error("smtp 4xx"));
  await relayOutbox();
  const [row] = await db.query("SELECT * FROM outbox WHERE status = 'pending'");
  expect(row).toBeDefined(); // still pending → will retry
});

test("one failing widget blanks the whole dashboard (bug)", async () => {
  mockFetchWidget.mockImplementation((id) =>
    id === "w3" ? Promise.reject(new Error("widget down")) : Promise.resolve({ id }));
  await expect(loadDashboard(["w1", "w2", "w3"])).rejects.toThrow("widget down");
  // user sees nothing — even though w1 and w2 were fine
});

// After — successes returned, failures reported (not swallowed).
async function loadDashboard(
  widgetIds: string[]
): Promise<{ data: WidgetData[]; failures: { id: string; error: Error }[] }> {
  const settled = await Promise.allSettled(widgetIds.map((id) => fetchWidget(id)));

  const data: WidgetData[] = [];
  const failures: { id: string; error: Error }[] = [];
  settled.forEach((r, i) => {
    if (r.status === "fulfilled") data.push(r.value);
    else failures.push({ id: widgetIds[i], error: r.reason });
  });

  for (const f of failures) logger.warn({ err: f.error, widget: f.id }, "widget failed");
  return { data, failures }; // caller can render partial UI + an error badge
}

test("partial results survive one failure, and the failure is reported", async () => {
  mockFetchWidget.mockImplementation((id) =>
    id === "w3" ? Promise.reject(new Error("widget down")) : Promise.resolve({ id }));
  const { data, failures } = await loadDashboard(["w1", "w2", "w3"]);
  expect(data.map((d) => d.id)).toEqual(["w1", "w2"]);
  expect(failures).toEqual([{ id: "w3", error: expect.any(Error) }]);
});

// After (TS) — abort the rest on first failure; reject with the real error.
async function fetchAll(ids: string[]): Promise<Item[]> {
  const ac = new AbortController();
  try {
    return await Promise.all(
      ids.map((id) =>
        fetchWidget(id, { signal: ac.signal }).catch((err) => {
          ac.abort(); // cancel siblings on first failure
          throw new Error(`fetch ${id} failed: ${(err as Error).message}`);
        })
      )
    );
  } catch (err) {
    ac.abort();
    throw err; // propagates — not swallowed, not a silent partial result
  }
}

// After — every async outcome has an owner.
async function createOrder(req: Req, res: Res, tasks: TaskSupervisor) {
  try {
    const user = await getUser(req.userId);                 // (A) awaited
    const order = await db.insert({ ...req.body, userName: user.name });

    await chargeCard(user, order.total);                    // (B) awaited — decline reaches catch

    tasks.run("order-confirmation", () => sendConfirmation(order)); // (C) supervised + logged

    res.status(201).json({ id: order.id });
  } catch (err) {
    if (err instanceof CardDeclinedError) {                 // (D) narrowed, not swallowed
      return res.status(402).json({ error: "card_declined" });
    }
    if (err instanceof NotFoundError) {
      return res.status(404).json({ error: "user_not_found" });
    }
    logger.error({ err, reqId: req.id }, "createOrder failed"); // real reason captured
    res.status(500).json({ error: "internal_error" });
  }
}

test("decline → 402, not a swallowed 500", async () => {
  mockCharge.mockRejectedValue(new CardDeclinedError());
  await createOrder(req, res, tasks);
  expect(res.status).toHaveBeenCalledWith(402);
});
test("userName is the real name (forgotten-await fixed)", async () => {
  mockGetUser.mockResolvedValue({ name: "Ada" });
  await createOrder(req, res, tasks);
  expect(mockInsert).toHaveBeenCalledWith(expect.objectContaining({ userName: "Ada" }));
});
test("confirmation failure is logged, response still 201", async () => {
  mockSendConfirmation.mockRejectedValue(new Error("smtp down"));
  const err = jest.spyOn(logger, "error");
  await createOrder(req, res, tasks);
  await tasks.drain();
  expect(res.status).toHaveBeenCalledWith(201);
  expect(err).toHaveBeenCalled();
});

test("response is not delayed by the metric", async () => {
  mockMetric.mockImplementation(() => delay(10_000)); // pathologically slow
  const start = Date.now();
  await handleSearch(req, res);
  expect(Date.now() - start).toBeLessThan(100); // response did NOT wait
});
test("a metric failure never crashes or alters the response", async () => {
  mockMetric.mockRejectedValue(new Error("statsd down"));
  const captured: unknown[] = [];
  process.once("unhandledRejection", (e) => captured.push(e));
  await handleSearch(req, res);
  await flushMicrotasks();
  expect(res.json).toHaveBeenCalled();   // response unaffected
  expect(captured).toHaveLength(0);      // no unhandledRejection
});