Chapter 2 — Callbacks Done Wrong

Zalgo, double calls, lost errors, lost this, and how to convert safely

Prerequisites: Chapter 1 (queues and phases), FE-10 Parts 4–5 (closures, this). Practice: the gym, exercise 09-legacy-callbacks, and promisify in 10-polyfill-kata.

How to Read This Chapter

You will still meet callbacks in every real codebase: old SDKs, fs and net code, event emitters, a wrapper someone wrote in 2016 that the billing code still calls. In a review round they are a gift, because callback code breaks in a few repeatable ways, and each one is easy to name once you know it.

The order of this chapter is on purpose. Callback hell, the nesting everyone complains about, comes first and is dealt with quickly. It is a readability problem. The rest of the chapter covers correctness problems: callbacks that run at an unpredictable time, run twice, never run, or throw where nobody can catch it. Those are what interviewers plant.

Part 1: Error-First Callbacks, and Why "Hell" Is the Smaller Problem

The Node convention has three rules:

1. The callback is the last argument.
2. The first callback argument is an error or null. Success values follow.
3. The callback is called exactly once, and always asynchronously.

Most of this chapter is about code that breaks rule 3.

Callback hell looks like this:

getUser(id, (err, user) => {
  if (err) return done(err);
  getWallet(user.walletId, (err, wallet) => {
    if (err) return done(err);
    charge(wallet, 10, (err, receipt) => {
      if (err) return done(err);
      done(null, receipt);
    });
  });
});

It is hard to read, and every level repeats if (err) return done(err). But it is correct. Named functions or a conversion to async/await fix the shape. In a review, mention the nesting in one sentence and spend your time on the four bugs below. A reviewer who only says "this is callback hell, rewrite it with async/await" has not reviewed it.

Part 2: Zalgo — Sometimes Sync, Sometimes Async

Here is a cache in front of a slow database. It looks reasonable:

const cache = new Map();

function getUser(id, cb) {
  if (cache.has(id)) return cb(null, cache.get(id)); // hit: called now
  db.findUser(id, (err, user) => {                    // miss: called later
    if (!err) cache.set(id, user);
    cb(err, user);
  });
}

The problem: the caller cannot know whether code after getUser(...) runs before or after the callback. On a miss it runs before. On a hit it runs after. Any caller that sets something up after the call is now wrong on one of the two paths.

function openProfile(id) {
  getUser(id, (err, user) => {
    ui.spinner = false;
    render(user);
  });
  ui.spinner = true;   // "show the spinner while it loads"
}

Spinners are the harmless version. The same split breaks counters:

function loadTeam(ids, done) {
  let pending = 0;
  const users = [];
  for (const id of ids) {
    getUser(id, (err, user) => {
      users.push(user);
      pending--;
      if (pending === 0) done(users);
    });
    pending++;
  }
}

The first loadTeam([1, 2]) works: both are misses, so all increments happen before any callback. The second call, with both users cached, never calls done: each callback runs before its own pending++, so pending goes -1, 0, -1, 0 and the check happens at the wrong moments. (Run in Node: the first load completes, the second never does.)

The fix is to make the function always async. Defer the cached path:

function getUser(id, cb) {
  if (cache.has(id)) {
    const user = cache.get(id);
    queueMicrotask(() => cb(null, user));   // or process.nextTick in Node-only code
    return;
  }
  db.findUser(id, (err, user) => {
    if (!err) cache.set(id, user);
    cb(err, user);
  });
}

Now code after the call always runs before the callback, on both paths. Promises enforce this for you: a .then callback never runs synchronously, even on an already-resolved promise. That guarantee is a large part of why promises replaced callbacks.

Zalgo also hides in async code that is not async: a function that returns a value on a cache hit and a promise on a miss (return cached ?? fetchUser(id)). Callers write getUser(id).then(...) and crash on the hit path. Mark it async so it always returns a promise.

Part 3: Called Twice, or Never

Rule 3 says exactly once. There are three common ways to break it.

A missing return after an error.

function readConfig(path, cb) {
  fs.readFile(path, 'utf8', (err, text) => {
    if (err) cb(err);                  // no return
    cb(null, JSON.parse(text));        // runs anyway, JSON.parse(undefined) throws
  });
}

On a missing file the callback gets ENOENT, then JSON.parse(undefined) throws a SyntaxError inside an I/O callback, which crashes the process (Part 4). Change the code a little and you get two calls instead of a crash. If the callback sends an HTTP response, the second call is ERR_HTTP_HEADERS_SENT. If it charges credits, it charges twice.

A try that wraps the callback call.

function parseBody(text, cb) {
  try {
    cb(null, JSON.parse(text));
  } catch (err) {
    cb(err);
  }
}

If JSON.parse throws, fine. But if the callback itself throws, the catch catches the caller's bug and calls the callback a second time with it. Run it with a callback that reads body.items.forEach on a body that has no items, and the callback runs twice: once with the body, once with a TypeError from its own code. Keep the try around your work only:

function parseBody(text, cb) {
  let body;
  try {
    body = JSON.parse(text);
  } catch (err) {
    return cb(err);
  }
  cb(null, body);
}

A path that never calls back. The easy case is an early return that forgets cb. The harder case is an event source:

function readBody(stream, cb) {
  let body = '';
  stream.on('data', (chunk) => { body += chunk; });
  stream.on('end', () => cb(null, body));
}

A stream that errors does not emit end. If the client disconnects halfway, cb is never called, the request hangs until some outer timeout, and anything the caller holds (a DB connection, a lock, a slot in a concurrency pool) is held until then. (Verified: destroying the stream with an error left the callback uncalled.) The fix is to listen for error too and guard against double calls, or better, use the promise helper that already does both:

import { text } from 'node:stream/consumers';
const body = await text(stream);   // rejects if the stream errors

A small once guard is worth knowing for any callback API you must keep:

function once(cb) {
  let called = false;
  return (...args) => {
    if (called) return;
    called = true;
    cb(...args);
  };
}

It stops double calls from doing harm. It hides the bug, though, so log the second call in development.

Part 4: Throwing Inside a Callback

try/catch only catches errors thrown while its block is on the call stack. A callback runs later, in a different task, from an empty stack. By then the try block finished long ago.

try {
  setTimeout(() => { throw new Error('boom from timer'); }, 0);
} catch (e) {
  console.log('caught?', e.message);   // never runs
}
console.log('try block finished');

Output: try block finished, then the process dies with Error: boom from timer and exit code 1. The error goes to process.on('uncaughtException'). With no handler, Node prints it and exits.

In a server this is a full outage, not a 500:

app.get('/report', (req, res) => {
  try {
    fs.readFile(reportPath, 'utf8', (err, text) => {
      const report = JSON.parse(text);   // throws on a corrupt file
      res.json(report);
    });
  } catch (err) {
    res.status(500).json({ error: 'report failed' });
  }
});

Express catches errors thrown synchronously from a route handler. This one is thrown from an I/O callback, so Express never sees it. One corrupt file takes down the process and every request in flight on it (Chapter 8). Inside a callback, catch locally and pass the error on:

fs.readFile(reportPath, 'utf8', (err, text) => {
  if (err) return next(err);
  let report;
  try {
    report = JSON.parse(text);
  } catch (parseErr) {
    return next(parseErr);
  }
  res.json(report);
});

Or convert the whole thing to async with fs/promises, where try/catch around await works because the await resumes inside the try block.

Part 5: this Lost in Callbacks

FE-10 covers the four binding rules. In review, the pattern to spot is a method passed by reference: emitter.on('x', this.handle), setTimeout(this.tick, 1000), arr.forEach(this.check), promisify(client.query). The method loses its object. What this becomes depends on who calls it:

The emitter case is the dangerous one because it does not throw:

class UsageMeter {
  constructor(stream) {
    this.tokens = 0;
    stream.on('chunk', this.onChunk);
  }
  onChunk(chunk) {
    this.tokens += chunk.tokens;
  }
}

After one chunk event, meter.tokens is still 0, and the emitter now has a tokens property equal to NaN (undefined + 5). No error, wrong billing. Fix with an arrow field or a bind, and keep a reference so you can remove the listener later (Chapter 6):

class UsageMeter {
  tokens = 0;
  onChunk = (chunk) => { this.tokens += chunk.tokens; };
  constructor(stream) {
    stream.on('chunk', this.onChunk);
  }
}

Part 6: Closures Capturing a Changing Variable

FE-10 derives the var-in-a-loop bug. The version that survives into modern code uses let or const correctly in the loop, but closes over a variable declared outside it:

let current;
for (const job of jobs) {
  current = job;
  save(job, (err) => {
    if (err) log.error('save failed', current.id);
    else log.info('saved', current.id);
  });
}

With three jobs, all three callbacks log c. The callbacks run after the loop has finished, and they all read the single current binding, which now holds the last job. The fix is to use the loop's own binding (job), which is fresh per iteration. The same bug appears with a this.currentRequest field read inside a callback, and in React with state read inside a stale effect callback. The question to ask in review is always: which binding does this callback read, and who writes to it before the callback runs?

Part 7: Converting Callback APIs to Promises

In order of preference:

1. Use the promise API that already exists: node:fs/promises, node:timers/promises, node:stream/promises (pipeline, finished), node:stream/consumers, events.once(emitter, 'ready').
2. util.promisify(fn) for a well-behaved error-first function. Bind methods first.
3. A hand-written new Promise wrapper when the API does not fit: multiple success values, events instead of a callback, or a callback that is not error-first.

import { once } from 'node:events';

// An event-based API: resolve on 'ready', reject on 'error'
async function connect(client) {
  client.connect();
  await once(client, 'ready');   // events.once rejects if 'error' fires first
  return client;
}

// A non-error-first callback: geolocation calls success and failure separately
const getPosition = (options) =>
  new Promise((resolve, reject) => navigator.geolocation.getCurrentPosition(resolve, reject, options));

Two conversion mistakes to look for:

• Mixing both styles in one function: it returns a promise and calls a callback. A caller that uses both handles each result twice; a caller that uses neither leaves an unhandled rejection.
• Wrapping without handling every path: a new Promise around an event source that listens for data and end but not error has the "never called" bug from Part 3 in promise form: a promise that never settles.

Spot the Bug

Drill 1: the promisified legacy client

import { promisify } from 'node:util';
import { LegacyDb } from './legacy-db.js';

// legacy-db: query(sql, cb) calls cb(err, rows, fields) and reads this.pool
const db = new LegacyDb(config);
const query = promisify(db.query);

export async function listQueuedJobs() {
  const [rows, fields] = await query("SELECT id FROM jobs WHERE status = 'queued'");
  log.debug('columns', fields.map((f) => f.name));
  return rows;
}

What's wrong? There are two bugs.

const query = (sql) =>
  new Promise((resolve, reject) => {
    db.query(sql, (err, rows, fields) => (err ? reject(err) : resolve({ rows, fields })));
  });

const { rows, fields } = await query('SELECT ...');

Drill 2: the route that takes the server down

app.post('/webhooks/stripe', (req, res) => {
  try {
    verifySignature(req, (err, event) => {
      if (err) return res.status(400).end();
      const amount = event.data.object.amount_received;
      ledger.credit(event.data.object.metadata.userId, amount, (err) => {
        if (err) throw err;
        res.status(200).end();
      });
    });
  } catch (err) {
    log.error(err);
    res.status(500).end();
  }
});

What's wrong?

ledger.credit(userId, amount, (err) => {
  if (err) {
    log.error({ err, eventId: event.id }, 'credit failed');
    return res.status(500).end();    // Stripe will retry
  }
  res.status(200).end();
});

Drill 3: the memoised provider lookup

const regionCache = new Map();

export function getProviderRegion(provider, cb) {
  const hit = regionCache.get(provider);
  if (hit) return cb(null, hit);
  dns.resolveTxt(`_region.${provider}.internal`, (err, records) => {
    if (err) return cb(err);
    const region = records[0].join('');
    regionCache.set(provider, region);
    cb(null, region);
  });
}

// elsewhere
export function routeRequest(req, done) {
  let region;
  getProviderRegion(req.provider, (err, r) => {
    if (err) return done(err);
    done(null, { url: endpoints[region], region: r });
  });
  region = pickFallbackRegion(req);
}

What's wrong?

if (hit) {
  queueMicrotask(() => cb(null, hit));
  return;
}

Drill 4: the batch writer

export function flushEvents(events, cb) {
  let done = 0;
  let failed = false;
  for (const event of events) {
    analytics.write(event, (err) => {
      if (err) {
        failed = true;
        return cb(err);
      }
      if (++done === events.length && !failed) cb(null, done);
    });
  }
}

What's wrong?

export function flushEvents(events, cb) {
  if (events.length === 0) return queueMicrotask(() => cb(null, 0));
  let remaining = events.length;
  let finished = false;
  const finish = (err, value) => {
    if (finished) return;
    finished = true;
    cb(err, value);
  };
  for (const event of events) {
    analytics.write(event, (err) => {
      if (err) return finish(err);
      if (--remaining === 0) finish(null, events.length);
    });
  }
}

Build It: promisify and callbackify

Spec.

• promisify(fn, { multiArgs }) returns a function that calls fn with the same this and arguments plus an error-first callback, and returns a promise.
  • Rejects with err if it is truthy. Otherwise resolves with the first value, or with an array of all values if multiArgs is true.
  • If fn[promisify.custom] is set, return that function instead. promisify.custom is Symbol.for('nodejs.util.promisify.custom'), the same symbol Node uses, which is how promisify(setTimeout) gives you a working sleep.
  • A synchronous throw from fn becomes a rejection. A second callback call is ignored.
• callbackify(fn) returns a function whose last argument is an error-first callback.
  • Calls back with (null, value) on fulfilment and (reason) on rejection.
  • A falsy rejection reason (null, 0, '') is wrapped in an Error with a reason property, because cb(null) would look like success.
  • Calls the callback outside the promise chain, so an exception thrown by the callback is an uncaught exception, not an unhandled rejection, and cannot cause a second call.

const kCustom = Symbol.for('nodejs.util.promisify.custom');

export function promisify(fn, { multiArgs = false } = {}) {
  if (typeof fn !== 'function') throw new TypeError('promisify: argument must be a function');

  if (fn[kCustom]) {
    const custom = fn[kCustom];
    if (typeof custom !== 'function') throw new TypeError('promisify.custom must be a function');
    return custom;
  }

  function promisified(...args) {
    return new Promise((resolve, reject) => {
      // A throw here happens inside the executor, so it becomes a rejection.
      fn.call(this, ...args, (err, ...values) => {
        if (err) return reject(err);
        resolve(multiArgs ? values : values[0]);
        // A second call does nothing: a promise settles once.
      });
    });
  }

  Object.setPrototypeOf(promisified, Object.getPrototypeOf(fn));
  // Promisifying the result again returns the same function.
  Object.defineProperty(promisified, kCustom, { value: promisified });
  return promisified;
}
promisify.custom = kCustom;

export function callbackify(fn) {
  if (typeof fn !== 'function') throw new TypeError('callbackify: argument must be a function');

  return function callbackified(...args) {
    const cb = args.pop();
    if (typeof cb !== 'function') throw new TypeError('callbackify: last argument must be a function');

    const onFulfilled = (value) => process.nextTick(cb, null, value);
    const onRejected = (reason) => {
      if (!reason) {
        const wrapped = new Error('Promise was rejected with a falsy value');
        wrapped.reason = reason;
        reason = wrapped;
      }
      process.nextTick(cb, reason);
    };

    // Starting from a resolved promise turns a sync throw in fn into an error callback.
    Promise.resolve()
      .then(() => fn.apply(this, args))
      .then(onFulfilled, onRejected);
  };
}

Edge cases interviewers check.

One deliberate difference from Node: util.callbackify lets a synchronous throw from a non-async fn escape synchronously. This version turns it into an error callback. Either is defensible; say which one you chose.

The Causal Chains

Review Checklist

1. Functions with a cache or fast path that call back synchronously. Look for return cb( before any async call.
2. if (err) cb(err) without return. Search for cb(err) and callback(err) and check the next line.
3. try { cb(...) } catch blocks that wrap the callback call.
4. Event-based waits that ignore error: on('end'), on('finish'), on('ready') with no error listener, inside callbacks or new Promise.
5. throw inside any callback, including if (err) throw err in route handlers.
6. Methods passed by reference: .on('…', this.x), setTimeout(this.x), promisify(obj.x).
7. Callbacks reading variables declared outside the loop or instance fields that change.
8. Hand-written promise wrappers that miss the error path, or resolve with only one of several values.
9. Functions that both accept a callback and return a promise.

What's Next

Promises fix Zalgo and double calls by design, and add their own failure modes: forgotten returns, errors that vanish, and rejections that crash Node. Chapter 3 — Promises in Real Code.