The Happy Path Trap: Reviewing Error Handling and Edge Cases
The Happy Path Trap: Reviewing Error Handling and Edge Cases
"Copilot completed it. It works."
When you see this in a PR description, experienced reviewers put on their "failure mode" glasses. Because "it works" almost always means "it works on the happy path" — when the user provides correct input, the network responds immediately, the database is responsive, and all external services are alive and well. Production has a very different set of conditions.
Understanding the Happy Path vs. Reality
The happy path is the flow where everything goes right. User provides valid input. Network is responsive. Database returns results quickly. External APIs respond with the expected format. No timeouts. No concurrent requests causing race conditions. No edge cases.
Code that works only on the happy path is a disaster waiting to happen in production. Here's why:
Look at AI-generated code like this:
async function getUserReport(userId: string) {
const user = await userService.findById(userId);
const orders = await orderService.getRecentOrders(userId);
const report = reportBuilder.build(user, orders);
return report;
}It's short, clean, and the logic appears correct. But what happens in production?
- If `userId` doesn't exist → `user` is null → NPE when accessing user properties
- If `orderService` times out → unhandled Promise rejection crashes the request
- If `reportBuilder` fails → error propagates without context
- If `userId` is an empty string → unpredictable behavior
- If the database query is slow → request hangs
This code doesn't handle the real world where things fail constantly and in combinations.
Three Areas AI Systematically Misses
Area 1: Null/Undefined Returns
Functions often return null when "no value is found." But the possibility must be explicitly expressed and must be handled at every call site:
// Before: returns null, but caller doesn't check
public User findByEmail(String email) {
return userRepository.findByEmail(email); // may return null, but no indication
}
// Caller doesn't check
User user = userService.findByEmail(email);
String name = user.getName(); // NPE possible if user is null
String email = user.getEmail(); // Another NPE possible
Better design makes nullability explicit in the type contract:
// After: type shows nullability explicitly
public Optional<User> findByEmail(String email) {
return userRepository.findByEmail(email);
}
// Caller must handle
Optional<User> user = userService.findByEmail(email);
String name = user.map(User::getName).orElse("Unknown");
String email = user.map(User::getEmail).orElse("");
// Or throw an exception if null is unexpected
User user = userService.findByEmail(email)
.orElseThrow(() -> new UserNotFoundException(email));
Area 2: Unhandled Asynchronous Errors
async function loadDashboard() {
const userData = await fetchUser(); // what if fails?
const orderData = await fetchOrders(); // what if fails?
const statsData = await fetchStats(); // what if fails?
renderDashboard(userData, orderData, statsData);
}If any call fails, the entire dashboard is unrendered. There's no error message to the user. Just silence or console warnings that the user never sees.
Area 3: Assuming External Systems Work
External APIs, payment gateways, email services — these fail constantly. Reviewers check whether code handles timeouts, temporary failures, rate limiting, and maintenance windows.
Exception Handling Antipatterns
Antipattern 1: Catching and Doing Nothing
try {
emailService.send(userId, message);
} catch (Exception e) {
// ignore
}This is dangerous and insidious. If sending fails, absolutely no one knows. No log, no alert, no error message. The system silently fails. The user doesn't know. The ops team doesn't know. The email was silently lost.
Better approach: Log the error, and decide whether to propagate it or use a fallback strategy.
Antipattern 2: Catching Everything With One Handler
try {
validateOrder(data);
await orderRepository.save(data);
await paymentService.charge(order);
await emailService.sendConfirmation(order);
} catch (error) {
return { success: false, message: 'An error occurred.' };
}But these are fundamentally different errors:
- Validation failure → user error → 400 Bad Request
- DB save failure → server error → 500, needs retry
- Payment failure → specific error → 402 Payment Required, cancel order
- Email failure → order succeeded, only email failed, don't roll back
Handling them all with one generic message hides what actually went wrong.
Antipattern 3: Exposing Internal Information
catch (error) {
return { error: error.getMessage() }; // DB connection info exposed!
}If the database error is `Connection refused: jdbc:postgresql://internal-db:5432/prod`, you've just exposed your entire infrastructure to an attacker.
Better approach: Log details on the server. Give a generic message to the user.
Questions to Discover Edge Cases
When reviewing code, systematically ask:
"What if the value is not what I expected?"
- What if null or undefined?
- What if empty (empty string, empty array)?
- What if 0 or negative?
- What if very large (millions, billions)?
- What if special characters or Unicode?
"What if things happen at the same time?"
Race conditions often don't show in tests because tests are single-threaded. Two requests reach the same code simultaneously:
async function useCoupon(couponId, userId) {
const coupon = await couponRepository.findById(couponId);
if (coupon.usedAt !== null) throw new CouponAlreadyUsed();
// RACE CONDITION: two requests both pass the check
await couponRepository.markAsUsed(couponId, userId);
}Two requests check → both pass → both try to use the coupon → one coupon is used twice → revenue loss.
"What if the outside world doesn't cooperate?"
- What if a timeout occurs?
- What if the connection is lost?
- What if the response format is different?
- What if there's no response at all?
- What if the API is in maintenance mode?
Defensive coding means trusting nothing from external systems.
Edge Case Reviewer Checklist
□ Inputs
□ Can null or undefined appear?
□ Are empty values (empty string, empty array, 0) handled?
□ Can negative or unusually large values appear?
□ Are strings validated for length and content?
□ External dependencies
□ Is there handling for DB/API call failure?
□ Is a timeout configured?
□ Is the external response shape validated?
□ What if the service is down?
□ Concurrency
□ What happens if two requests arrive simultaneously?
□ Can intermediate state be changed elsewhere?
□ Are there race conditions?
□ State transitions
□ Is the operation idempotent (safe to retry)?
□ Can the previous state be restored if operation fails?
□ What if another process modifies state in between?
Practical Examples: Real Edge Cases That Hurt Production
Case 1: The Null Email Bug
A service sends a welcome email to new users. The code doesn't check if email is null. Ninety-nine percent of users have email. One percent get null from a legacy import. The email service crashes on null, logs errors, and that 1% never gets their welcome email or gets an error when trying to login. This cascades into support tickets.
Case 2: The Race Condition in Inventory
A store has 1 item in stock. Two customers buy it simultaneously. The code checks inventory, finds 1, sells to both. Now the store is obligated to deliver 2 items but has 0. Returns and refunds follow. The fix requires database transactions or atomic operations — things only visible when you ask "what if two requests arrive at the same time?"
Case 3: The Silent Payment Failure
A payment API call fails (network timeout). The code has a try-catch that logs the error but doesn't fail the order. The customer thinks they paid but never actually did. Their order never ships. They get charged by their credit card company for a chargeback. Ops gets an angry call.
The fix would have been visible if someone asked "what happens when the external API fails?"
Building a Mental Model
Expert reviewers develop an intuitive sense of where edge cases hide. They think in fault trees:
- Null/undefined: Every variable could be null. The database returns null. The API returns empty. The cache key is missing.
- Concurrency: Every operation could happen twice. Two requests. Two processes. Two servers.
- External failures: Every external system will fail. Eventually. Often at the worst time.
- Boundaries: Every boundary has edge cases. 0, negative, maximum value, infinity, NaN.
Develop this mental model by asking those questions consistently, across multiple PRs, until they become automatic.
Concurrency Patterns: A Frequent Blind Spot
Many developers write code that's correct in single-threaded scenarios but breaks with concurrency:
// Seemingly safe - but not thread-safe
public boolean useDiscount(Discount discount) {
if (discount.getUsageCount() < discount.getMaxUsages()) {
discount.setUsageCount(discount.getUsageCount() + 1);
discountRepository.save(discount);
return true;
}
return false;
}
// Two concurrent requests:
// Request 1: checks count (0), passes check
// Request 2: checks count (0), passes check ← RACE CONDITION
// Request 1: saves count (1)
// Request 2: saves count (1) ← Same value saved twice
// Discount is now used twice when max is 1Solution: Use database-level constraints:
@Lock(LockModeType.PESSIMISTIC_WRITE) // Lock at database level
public Discount findAndLockDiscount(Long discountId) {
return discountRepository.findById(discountId).orElse(null);
}
public boolean useDiscount(Discount discount) {
if (discount.getUsageCount() < discount.getMaxUsages()) {
discount.setUsageCount(discount.getUsageCount() + 1);
discountRepository.save(discount);
return true;
}
return false;
}Build the Habit
When reviewing code, don't stop after confirming the happy path works. Ask one more time: What if the value is null? What if two requests arrive together? What if the external API fails or times out? What if the database is slow?
That one extra question prevents production incidents at 3 AM. That consistency makes you a reviewer others respect and learn from.
Building Your Mental Model of Failure
Expert reviewers develop intuition about failure modes. They ask the same questions across different codebases because the failure patterns are universal:
- Input validation: Every boundary condition has edge cases
- State management: Every state has a transition that can fail
- External systems: Every external system will eventually fail
- Concurrency: Every concurrent operation has race conditions
- Resource exhaustion: Every resource pool can be exhausted
Learn these patterns. When reviewing code, consciously check each one. Over time, it becomes automatic.