Async Misuse Anti-Patterns — Refactoring Practice¶

Category: Async Anti-Patterns → Misuse — async machinery bolted onto code that doesn't need it (and quietly loses errors when it does). Covers (collectively): Promise Constructor Anti-Pattern · async Without await

These are not "spot the smell" puzzles — find-bug.md does that. Here the code works today but misuses async primitives: it wraps an existing Promise in new Promise for no reason, marks functions async that never await, or hand-rolls a callback-to-Promise bridge that swallows errors on a corner. Your job is to transform it into correct, idiomatic code without changing observable behavior — with one non-negotiable extra constraint that plain refactoring doesn't have:

Errors MUST propagate. The whole danger of these two anti-patterns is that they look fine on the happy path and silently drop rejections on the sad path. A refactor that "reads cleaner" but changes which errors reach the caller has changed behavior. The test you write first is an error-path test — it asserts the promise rejects with the right reason, or that a sync throw still surfaces.

The discipline is identical to structural refactoring, retargeted at async semantics:

1. Pin behavior first — including the error path. Write a characterization test that asserts both the resolved value and the rejection. await expect(fn()).rejects.toThrow(...) is as important as the success assertion.
2. Take small, reversible steps. One named move at a time (Return the Inner Promise, Drop async, Promisify the Callback), tests green after each.
3. Know the legitimate cases. new Promise is correct when bridging a non-Promise API (events, callbacks, timers). async is correct when you must normalize a sometimes-sync, sometimes-throwing API into "always a rejected/resolved Promise." Several exercises below are counter-cases you KEEP — recognizing them is half the skill.

How to use this file: read the "Before," predict (a) what breaks on the error path and (b) the move sequence yourself, then expand the solution. The gap between your prediction and the worked answer is the lesson.

Table of Contents¶

Exercise 1 — Unwrap the needless new Promise¶

Anti-pattern: Promise Constructor. Goal: stop wrapping a Promise-returning call in new Promise. Constraints: same resolved value; the rejection must still propagate (it currently does not — that's the latent bug, so preserving behavior here means restoring the lost rejection while keeping the success path identical).

// Before — fetchUser already returns a Promise; this wraps it for no reason.
function getUser(id) {
  return new Promise((resolve) => {
    fetchUser(id).then((user) => {
      resolve(user);
    });
  });
}

test("getUser rejects when fetchUser rejects", async () => {
  fetchUser.mockRejectedValueOnce(new Error("boom"));
  await expect(getUser(1)).rejects.toThrow("boom");
});

// After — the inner Promise IS the result; errors propagate for free.
function getUser(id) {
  return fetchUser(id);
}

Exercise 2 — Drop async from a pure function¶

Anti-pattern: async Without await. Goal: remove async from a function that does only synchronous work. Constraints: callers await the result today and must keep working unchanged.

// Before — async, but there is no await anywhere; it's pure arithmetic.
async function computeDiscount(price: number, pct: number): Promise<number> {
  const factor = 1 - pct / 100;
  return Math.round(price * factor * 100) / 100;
}

// After (preferred — own the callers) — pure, synchronous, allocation-free.
function computeDiscount(price: number, pct: number): number {
  const factor = 1 - pct / 100;
  return Math.round(price * factor * 100) / 100;
}

// After (contract-preserving alternative — can't change the signature):
function computeDiscount(price: number, pct: number): Promise<number> {
  const factor = 1 - pct / 100;
  return Promise.resolve(Math.round(price * factor * 100) / 100);
}

Exercise 3 — The wrapper that eats rejections¶

Anti-pattern: Promise Constructor. Goal: remove a wrapper that resolves but never rejects, then prove error propagation is now correct. Constraints: identical success value; rejections must reach the caller with the original reason.

// Before — manual resolve, no reject handler, plus an accidental swallow.
function loadConfig(path) {
  return new Promise((resolve, reject) => {
    readFilePromise(path)
      .then((raw) => resolve(JSON.parse(raw)))
      .catch(() => resolve({}));  // "default to empty config on any error"
  });
}

test("missing file → empty config", async () => {
  readFilePromise.mockRejectedValueOnce(Object.assign(new Error("nope"), { code: "ENOENT" }));
  await expect(loadConfig("x")).resolves.toEqual({});
});
test("malformed JSON rejects", async () => {
  readFilePromise.mockResolvedValueOnce("{ not json");
  await expect(loadConfig("x")).rejects.toThrow(SyntaxError);
});

// After — no wrapper; only ENOENT is defaulted, parse errors propagate.
async function loadConfig(path) {
  let raw;
  try {
    raw = await readFilePromise(path);
  } catch (err) {
    if (err.code === "ENOENT") return {};
    throw err;                       // any other read error propagates
  }
  return JSON.parse(raw);            // SyntaxError now reaches the caller
}

Exercise 4 — Fix the async Promise executor¶

Anti-pattern: Promise Constructor (the async executor footgun). Goal: remove the async keyword from a new Promise executor that uses it. Constraints: same resolved value; a throw inside the executor's async work must reject the promise, not vanish into an unhandled rejection.

// Before — executor is `async`; a throw before `resolve` is LOST.
function withSetup(task) {
  return new Promise(async (resolve, reject) => {
    await prepare();              // if this rejects, nobody catches it
    const result = await task();  // if task() rejects, the outer Promise hangs forever
    resolve(result);
  });
}

test("rejects when task throws", async () => {
  await expect(withSetup(() => Promise.reject(new Error("x")))).rejects.toThrow("x");
});

// After — async function body; throws become rejections automatically.
async function withSetup(task) {
  await prepare();
  return task();      // task()'s rejection propagates; no manual resolve/reject
}

Exercise 5 — async with no await, but it try/catches¶

Anti-pattern: async Without await. Goal: remove async from a function with no await — while preserving its error semantics. Constraints: the caller relies on a rejected Promise (not a sync throw) when validation fails.

// Before — no await, but it throws; async turns the throw into a rejection.
async function parsePayload(json: string): Promise<Payload> {
  const obj = JSON.parse(json);          // can throw SyntaxError
  if (!obj.id) throw new Error("missing id");
  return obj as Payload;
}

// caller relies on rejection:
parsePayload(input).catch((e) => respond(400, e.message));

test("rejects on bad JSON", async () => {
  await expect(parsePayload("{ bad")).rejects.toThrow(SyntaxError);
});
test("rejects on missing id", async () => {
  await expect(parsePayload("{}")).rejects.toThrow("missing id");
});

// After (option A — keep async to preserve the rejection contract):
// eslint-disable-next-line @typescript-eslint/require-await -- async coerces sync throw into rejection for .catch() callers
async function parsePayload(json: string): Promise<Payload> {
  const obj = JSON.parse(json);
  if (!obj.id) throw new Error("missing id");
  return obj as Payload;
}

// After (option B — sync contract; caller updated to try/catch):
function parsePayload(json: string): Payload {
  const obj = JSON.parse(json);
  if (!obj.id) throw new Error("missing id");
  return obj as Payload;
}
// caller:
try { respondWith(parsePayload(input)); }
catch (e) { respond(400, (e as Error).message); }

Exercise 6 — Promisify a Node callback by hand¶

Anti-pattern: Promise Constructor (hand-rolled, subtly wrong). Goal: replace a hand-written new Promise callback bridge with util.promisify. Constraints: same resolved value on success; the callback's err must reject the Promise.

// Before — hand-rolled bridge; works, but reinvents a standard utility
// and is easy to get subtly wrong (e.g. forgetting `return` after reject,
// or resolving with the wrong arg count).
const fs = require("fs");

function readFileP(path) {
  return new Promise((resolve, reject) => {
    fs.readFile(path, "utf8", (err, data) => {
      if (err) {
        reject(err);
      }
      resolve(data);          // BUG: runs even after reject (no early return)
    });
  });
}

test("resolves file contents", async () => {
  await expect(readFileP(realPath)).resolves.toContain("hello");
});
test("rejects on read error", async () => {
  await expect(readFileP("/no/such/file")).rejects.toThrow(/ENOENT/);
});

// After — standard, correct, one line.
const fs = require("fs");
const { promisify } = require("util");

const readFileP = promisify(fs.readFile);
// usage unchanged: readFileP(path, "utf8")

// Even better — fs already ships Promise versions; prefer them when available:
const fs = require("fs/promises");
// fs.readFile(path, "utf8") already returns a Promise

Exercise 7 — KEEP the new Promise — event bridge¶

Anti-pattern: Counter-case (a correct new Promise). Goal: recognize that this new Promise is legitimate, and harden it rather than removing it. Constraints: resolve on open, reject on error; never leak listeners; settle exactly once.

// Before — bridges an EventEmitter to a Promise. The bridge is NEEDED
// (events are not Promises) but it leaks listeners and can settle twice.
function waitForOpen(socket) {
  return new Promise((resolve, reject) => {
    socket.on("open", () => resolve(socket));
    socket.on("error", (err) => reject(err));
  });
}

// After — same shape, now leak-free and settle-once.
function waitForOpen(socket) {
  return new Promise((resolve, reject) => {
    const onOpen = () => { cleanup(); resolve(socket); };
    const onError = (err) => { cleanup(); reject(err); };
    function cleanup() {
      socket.off("open", onOpen);
      socket.off("error", onError);
    }
    socket.once("open", onOpen);
    socket.once("error", onError);
  });
}

Exercise 8 — KEEP async — normalize a sometimes-sync API¶

Anti-pattern: Counter-case (a correct async without await). Goal: recognize why async earns its keep on a function whose body may not await on every path. Constraints: the function must always return a Promise and always surface failures as rejections, regardless of which branch runs.

// Before — a cache layer: hits return synchronously, misses await a fetch.
// A reviewer flags "async without await on the hit path — drop it?"
async function getProfile(id: string): Promise<Profile> {
  const cached = cache.get(id);     // synchronous
  if (cached) return cached;        // <-- no await on this branch
  const fresh = await api.fetch(id);
  cache.set(id, fresh);
  return fresh;
}

test("cache hit resolves to cached value", async () => {
  cache.set("1", profileA);
  await expect(getProfile("1")).resolves.toBe(profileA);
});
test("cache miss fetches and resolves", async () => {
  api.fetch.mockResolvedValueOnce(profileB);
  await expect(getProfile("2")).resolves.toBe(profileB);
});
test("fetch failure rejects", async () => {
  api.fetch.mockRejectedValueOnce(new Error("down"));
  await expect(getProfile("3")).rejects.toThrow("down");
});

// After — unchanged on purpose; the async keyword normalizes a mixed API.
async function getProfile(id: string): Promise<Profile> {
  const cached = cache.get(id);
  if (cached) return cached;        // wrapped in a resolved Promise by `async`
  const fresh = await api.fetch(id);
  cache.set(id, fresh);
  return fresh;
}

Exercise 9 — Promise.resolve().then ceremony¶

Anti-pattern: Promise Constructor family (needless Promise ceremony). Goal: strip a Promise.resolve(...).then(...) chain that re-implements what async/await does natively, and stop dropping the rejection. Constraints: same resolved value; rejection from the inner call must propagate.

// Before — needless ceremony; the .then re-wraps and the error is dropped.
function fetchAndTag(id) {
  return Promise.resolve()
    .then(() => loadRecord(id))      // loadRecord returns a Promise
    .then((rec) => {
      return { ...rec, fetchedAt: Date.now() };
    });
  // No .catch — and the leading Promise.resolve() adds a pointless tick.
}

test("rejects when loadRecord rejects", async () => {
  loadRecord.mockRejectedValueOnce(new Error("missing"));
  await expect(fetchAndTag(1)).rejects.toThrow("missing");
});

// After — flat, readable, one fewer microtask hop; rejection still propagates.
async function fetchAndTag(id) {
  const rec = await loadRecord(id);
  return { ...rec, fetchedAt: Date.now() };
}

Exercise 10 — Build a correct timeout wrapper¶

Anti-pattern: Promise Constructor (legitimate, must be built correctly). Goal: wrap any Promise so it rejects if it doesn't settle within ms. Constraints: resolve/reject with the original outcome if it's in time; reject with a TimeoutError otherwise; never leak the timer; settle exactly once.

// Before — a first attempt. It "works" but leaks the timer on success
// and reinvents Promise.race poorly.
function withTimeout<T>(p: Promise<T>, ms: number): Promise<T> {
  return new Promise<T>((resolve, reject) => {
    setTimeout(() => reject(new Error("timeout")), ms);  // timer never cleared
    p.then(resolve, reject);
  });
}

test("resolves if in time", async () => {
  await expect(withTimeout(Promise.resolve(7), 50)).resolves.toBe(7);
});
test("rejects with original error if it fails in time", async () => {
  await expect(withTimeout(Promise.reject(new Error("boom")), 50)).rejects.toThrow("boom");
});
test("rejects with TimeoutError if too slow", async () => {
  const slow = new Promise((r) => setTimeout(r, 100));
  await expect(withTimeout(slow, 10)).rejects.toBeInstanceOf(TimeoutError);
});

// After — leak-free, single-settle, distinguishable error.
class TimeoutError extends Error {
  constructor(ms: number) { super(`timed out after ${ms}ms`); this.name = "TimeoutError"; }
}

function withTimeout<T>(p: Promise<T>, ms: number): Promise<T> {
  return new Promise<T>((resolve, reject) => {
    const timer = setTimeout(() => reject(new TimeoutError(ms)), ms);
    p.then(
      (val) => { clearTimeout(timer); resolve(val); },
      (err) => { clearTimeout(timer); reject(err); },
    );
  });
}

// Idiomatic alternative when you don't need a custom error or cleanup symmetry:
// Promise.race([p, rejectAfter(ms)]) — but rejectAfter must ALSO clear its timer,
// so the explicit bridge above is often clearer about the cleanup.

Exercise 11 — Build a cancellation wrapper (AbortSignal)¶

Anti-pattern: Promise Constructor (legitimate, must be built correctly). Goal: make a Promise reject promptly when an AbortSignal fires. Constraints: reject with an AbortError on abort; if already aborted, reject immediately; remove the listener when the promise settles or is aborted.

// Before — listens for abort but never unsubscribes, and ignores
// the already-aborted case (so a pre-aborted signal never rejects).
function abortable<T>(p: Promise<T>, signal: AbortSignal): Promise<T> {
  return new Promise<T>((resolve, reject) => {
    signal.addEventListener("abort", () => reject(new Error("aborted")));
    p.then(resolve, reject);
  });
}

test("rejects when aborted mid-flight", async () => {
  const ac = new AbortController();
  const slow = new Promise((r) => setTimeout(r, 100));
  const wrapped = abortable(slow, ac.signal);
  ac.abort();
  await expect(wrapped).rejects.toMatchObject({ name: "AbortError" });
});
test("rejects immediately if already aborted", async () => {
  const ac = new AbortController();
  ac.abort();
  await expect(abortable(Promise.resolve(1), ac.signal)).rejects.toMatchObject({ name: "AbortError" });
});
test("removes the abort listener after settling", async () => {
  const ac = new AbortController();
  await abortable(Promise.resolve(1), ac.signal);
  // an internal handle/spy proves removeEventListener was called
});

// After — handles pre-abort, names the error, unsubscribes on settle.
class AbortError extends Error {
  constructor() { super("aborted"); this.name = "AbortError"; }
}

function abortable<T>(p: Promise<T>, signal: AbortSignal): Promise<T> {
  return new Promise<T>((resolve, reject) => {
    if (signal.aborted) return reject(new AbortError());   // already-aborted case

    const onAbort = () => { cleanup(); reject(new AbortError()); };
    const cleanup = () => signal.removeEventListener("abort", onAbort);

    signal.addEventListener("abort", onAbort, { once: true });
    p.then(
      (val) => { cleanup(); resolve(val); },
      (err) => { cleanup(); reject(err); },
    );
  });
}

Exercise 12 — Python: async def with no await¶

Anti-pattern: async Without await. Goal: decide whether an async def that never awaits should drop async. Constraints: keep the function callable as it is used today; preserve error behavior.

# Before — async def, but the body is pure and never awaits.
async def normalize_tag(tag: str) -> str:
    return tag.strip().lower().replace(" ", "-")

# call site:
slug = await normalize_tag(raw)

# After (helper you own — drop async):
def normalize_tag(tag: str) -> str:
    return tag.strip().lower().replace(" ", "-")
# call site:
slug = normalize_tag(raw)

# After (counter-case — keep async to satisfy a shared awaitable protocol):
class Normalizer(Protocol):
    async def normalize(self, tag: str) -> str: ...

class SimpleNormalizer:
    async def normalize(self, tag: str) -> str:        # no await, but must match protocol
        return tag.strip().lower().replace(" ", "-")

Exercise 13 — Python: wrap an awaitable by hand¶

Anti-pattern: Promise Constructor analog (needless Future/create_task wrapping). Goal: stop wrapping an existing coroutine/awaitable in a hand-built Future, which loses the exception. Constraints: same resolved value; the inner exception must propagate to the awaiter.

# Before — wraps a coroutine in a Future by hand; the error is dropped.
import asyncio

def fetch_user(uid: int) -> asyncio.Future:
    fut: asyncio.Future = asyncio.get_event_loop().create_future()

    async def runner():
        result = await db.get_user(uid)   # if this raises, nobody sets the exception
        fut.set_result(result)

    asyncio.ensure_future(runner())
    return fut

async def test_propagates_error():
    db.get_user = AsyncMock(side_effect=ValueError("no user"))
    with pytest.raises(ValueError, match="no user"):
        await fetch_user(1)        # hangs against the Before code

# After — just await the awaitable; exceptions propagate naturally.
async def fetch_user(uid: int) -> User:
    return await db.get_user(uid)

Refactoring discipline (async) — the recap¶

The exercises run the same loop as structural refactoring, with one async-specific amendment baked in:

pin BOTH paths (resolve + reject)  →  one named move  →  both paths green  →  commit  →  repeat
                                       (behavioral change, if any, gets its OWN commit)

• Characterize the error path, not just the happy path. The signature failure of both anti-patterns is a lost rejection on a branch nobody tested. await expect(fn()).rejects.toThrow(...) (JS) / pytest.raises under await (Python) is the seatbelt. A wrapper that hangs to timeout is your proof the rejection is being dropped.
• Return the inner Promise/awaitable. The Promise Constructor anti-pattern is, at root, "wrapping a thing that is already a Promise." The fix is almost always to delete the wrapper and return (or await) the inner promise — errors then propagate for free.
• Never make a Promise executor async. The constructor discards the executor's return value, so an async executor's rejection becomes an unhandled rejection and the outer Promise hangs. Use an async function instead.
• Drop async only when no path benefits. async Without await is an anti-pattern for purely-synchronous bodies (Exercise 2) — but it's correct when the keyword normalizes throws into rejections for .catch callers (Exercise 5) or presents a uniform "always a Promise" contract across mixed branches (Exercise 8). Confirm no caller relies on the rejection before stripping it.
• Know the legitimate new Promise. Bridging events, timers, callbacks, or signals into a Promise is its irreplaceable job (Exercises 7, 10, 11). The discipline that comes with it: synchronous executor · pre-settled-state check · reject on the error path · clean up every subscription/timer on every settle.
• Prefer the standard tool. util.promisify (or fs/promises) over a hand-rolled bridge; async/await over Promise.resolve().then() ceremony; await db.get_user() over a hand-built asyncio.Future.

Related Topics¶

• tasks.md — guided exercises building these moves from scratch.
• find-bug.md — the spotting counterpart: identify the misuse, don't fix it.
• Async Anti-Patterns — the chapter overview and the other two categories (Error Handling, Execution Shape).
• Concurrency Anti-Patterns — the multi-thread sibling chapter (locks, races, deadlocks).
• Refactoring → Refactoring Techniques — the mechanical catalog for the named moves above.
• Refactoring → Code Smells — the smell-level view of needless wrappers and ceremony.