Bug: Transaction Context Detection Swallows Upstream Errors

[sahand12]

https://github.com/terminaldotshop/terminal/blob/cb8950b7d8fd46c3541634e2273487a74f3e037f/packages/core/src/drizzle/transaction.ts#L27C1-L35C1

Bug: Transaction Context Detection Swallows Upstream Errors

Summary

When using try-catch blocks for control flow to detect transaction context availability, all errors thrown within callbacks are silently caught and masked, leading to severe data integrity issues and unpredictable behavior.

This bug can cause:

• Silent data corruption
• Invalid data being written to the database
• Loss of transaction isolation guarantees
• Validation errors being ignored
• Database constraint violations being masked
• Difficult-to-debug production issues

The Problem

Buggy Pattern

Using try-catch for control flow detection catches ALL errors, not just context-not-found errors:

// Catches all errors from the callback
async function useTransaction<T>(cb: (tx: TxOrDb) => Promise<T>) {
  try {
    const { tx } = TransactionContext.use(); // Throws if context not found
    return await cb(tx); // ANY error here is caught below!
  } catch {
    // This catches errors from cb(tx) too!
    return await cb(db);
  }
}

Real-World Impact

Consider this repository code that throws validation errors:

// User repository with validation
export const create = fn(
  z.object({
    email: z.email(),
    password: z.string(),
  }),
  async (input) => {
    await useTransaction(async (tx) => {
      const existing = await tx
        .select()
        .from(userTable)
        .where(eq(userTable.email, input.email));
      
      if (existing.length > 0) {
        // This error gets swallowed!
        throw new AppError({
          type: 'validation',
          code: ErrorCodes.Validation.ALREADY_EXISTS,
          message: `User with email ${input.email} already exists`
        });
      }
      
      await tx.insert(userTable).values({...});
    });
  }
);

What happens with the buggy code:

1. User tries to create account with duplicate email
2. Code detects duplicate and throws AppError
3. Error is caught by try-catch in useTransaction
4. Code silently retries with db instead of tx
5. If inside a nested transaction, transaction isolation is lost
6. Duplicate user might get created anyway
7. No error is reported to the caller

Root Cause Analysis

The pattern uses exceptions for control flow:

// In context.ts
use(): T {
  const result = storage.getStore();
  if (!result) {
    throw new Error('Context not found.'); // Used for control flow!
  }
  return result;
}

The try-catch cannot distinguish between:

• "Context not found" (expected control flow)
• Errors from callback execution (critical business logic errors)

The Fix

Solution 1: Add Safe Control Flow Method (Recommended)

Add a method that returns undefined instead of throwing:

context.ts:

function createContext<T>() {
  const storage = new AsyncLocalStorage<T>();

  return {
    provide<R>(value: T, fn: () => R): R {
      return storage.run(value, fn);
    },
    
    // Throws when context not found (fail-fast)
    use(): T {
      const result = storage.getStore();
      if (!result) {
        // Throws an error if context is not found (fail-fast behavior).
        // Use this when the context MUST exist - the error will propagate to the caller.
        // For optional contexts or control flow decisions, use tryUse() instead.
        throw new Error('Context not found.');
      }
      return result;
    },
    
    // NEW: Safe control flow - returns undefined instead of throwing
    tryUse(): T | undefined {
      return storage.getStore();
    },
  };
}

transaction.ts:

type Transaction = Parameters<Parameters<typeof db.transaction>[0]>[0];
type TxOrDb = Transaction | typeof db;

const TransactionContext = createContext<{
  tx: Transaction;
  effects: (() => void | Promise<void>)[];
}>();

/**
 * Use a transaction if one is available, otherwise use the database.
 */
async function useTransaction<T>(cb: (tx: TxOrDb) => Promise<T>) {
  //  FIXED: Check context without catching callback errors
  const ctx = TransactionContext.tryUse();
  if (ctx) {
    return await cb(ctx.tx);
  }
  return await cb(db);
}

/**
 * Add an effect to run after transaction commits.
 */
async function afterTx(effect: () => void | Promise<void>) {
  // FIXED: Check context without catching effect errors
  const ctx = TransactionContext.tryUse();
  if (ctx) {
    ctx.effects.push(effect);
    return;
  }
  await effect();
}

/**
 * Creates and manages a database transaction with effect scheduling.
 */
async function createTransaction<T>(cb: (tx: Transaction) => Promise<T>): Promise<T> {
  // FIXED: Check context without catching callback errors
  const ctx = TransactionContext.tryUse();
  if (ctx) {
    return cb(ctx.tx);
  }
  
  // Create new transaction
  const effects: (() => void | Promise<void>)[] = [];
  const result = await db.transaction(async (tx) =>
    TransactionContext.provide({ tx, effects }, () => cb(tx))
  );
  
  // Run effects after successful commit
  await Promise.all(effects.map((effect) => effect()));
  return result;
}

Solution 2: Catch Specific Error Type

If adding tryUse() is not feasible, catch only the specific error:

class ContextNotFoundError extends Error {
  constructor(message = 'Context not found') {
    super(message);
    this.name = 'ContextNotFoundError';
  }
}

// In createContext
use(): T {
  const result = storage.getStore();
  if (!result) {
    throw new ContextNotFoundError();
  }
  return result;
}

// In useTransaction
async function useTransaction<T>(cb: (tx: TxOrDb) => Promise<T>) {
  try {
    const { tx } = TransactionContext.use();
    return await cb(tx);
  } catch (error) {
    // Only catch the specific error we expect
    if (error instanceof ContextNotFoundError) {
      return await cb(db);
    }
    // Re-throw all other errors
    throw error;
  }
}

Verification

Before Fix - Error is Swallowed:

await useTransaction(async (tx) => {
  throw new Error("This gets caught and ignored!");
});
// No error thrown! 

After Fix - Error Propagates:

await useTransaction(async (tx) => {
  throw new Error("This properly propagates!");
});
// Error is thrown as expected