Level 6.5 ~14 min Target: PHPStan max

Leave it as you found it

A customer asks for two red mugs, and there's only one. The checkout tells them so, which is right. But by then it had already charged their card for the whole basket and reserved the two blue mugs that came first. Nothing undid either. The customer was refunded by hand a few days later, and the blue mugs showed as sold out until someone noticed.

Each line of place() is correct. The bug is in what happens between them, when one fails and the ones before it have already changed the world.

Every step is a change

Placing an order does three things to three different systems: reserve the stock, charge the card, save the order. If the third fails, the first two still happened. An exception doesn't roll anything back. It just stops running your code, wherever it is, and leaves everything else as it was at that moment.

So for any method that changes more than one thing, ask: if this line throws, what has already happened?

Order by how hard it is to undo

The first fix costs nothing: do the steps in a better order.

Things that change nothing go first. Validation, lookups, checks (6.4). If they fail, there's nothing to undo.
Things that are easy to undo go next. Reserving stock can be released.
Things that are hard to undo go last. A charge can be refunded, but the customer sees both on their statement. An email can't be unsent at all.

Reserve, then charge, then save. Now running out of stock happens before anyone's card is touched.

Undo, then rethrow

Ordering helps, but something can still fail after something else has happened. Undo it, in reverse order, and send the exception on:

try {
    $this->orders->save($order);
} catch (Throwable $e) {
    $this->payments->refund($chargeId);
    $this->release($reserved);
    throw $e;
}

That's a catch (Throwable), which 6.2 said never to write. The difference is throw $e. This catch doesn't handle the failure or pretend it didn't happen. It tidies up on the way past, and the original exception carries on, unchanged, to code that can tell the customer.

Power-up

Before

<?php

final readonly class PlaceOrder
{
    public function place(Basket $basket): Order
    {
        $chargeId = $this->payments->charge($basket->customer, $basket->total()); // charged…
        foreach ($basket->lines as $line) {
            $this->stock->reserve($line->sku, $line->quantity);                 // …then OutOfStock
        }
        $order = new Order($basket->customer, $basket->lines, $chargeId);
        $this->orders->save($order);

        return $order;
    }
}

After

<?php

final readonly class PlaceOrder
{
    public function place(Basket $basket): Order
    {
        $reserved = [];
        try {
            foreach ($basket->lines as $line) {                                        // easy to undo
                $this->stock->reserve($line->sku, $line->quantity);
                $reserved[] = $line;
            }
            $chargeId = $this->payments->charge($basket->customer, $basket->total()); // hard to undo
        } catch (Throwable $e) {
            $this->release($reserved);
            throw $e;
        }
        // …and save the order the same way: refund, release, rethrow
    }
}

Quick check

CHALLENGE

A customer asked for two red mugs when there was one. Their card was charged for the whole basket, the blue mugs ahead of the red ones in the basket stayed reserved for an order that never existed, and the shop showed them as sold out for a week.

Make PlaceOrder::place() all or nothing. If anything fails, whether it's the stock, the card or saving the order, put back whatever it had already done and rethrow the exception unchanged, so the checkout page can still tell the customer what went wrong.

REFERENCE

try {
    // the steps
} catch (Throwable $e) {
    // undo what's been done
    throw $e;
}

Rethrowing makes catch (Throwable $e) safe here: it doesn't handle anything, it tidies up on the way out.

<?php

declare(strict_types=1);

interface Stock
{
    /**
     * Holds units for an order, so nobody else can buy them.
     *
     * @param positive-int $quantity
     *
     * @throws OutOfStock
     */
    public function reserve(string $sku, int $quantity): void;

    /**
     * Puts reserved units back on sale.
     *
     * @param positive-int $quantity
     */
    public function release(string $sku, int $quantity): void;
}

<?php

declare(strict_types=1);

interface Payments
{
    /**
     * @return string the charge ID
     *
     * @throws CardDeclined
     */
    public function charge(string $customer, int $pence): string;

    /** Gives a charge back in full. */
    public function refund(string $chargeId): void;
}

<?php

declare(strict_types=1);

interface Orders
{
    /** Stores the order. Like anything that talks to a database, it can fail. */
    public function save(Order $order): void;
}

<?php

declare(strict_types=1);

final readonly class Basket
{
    /** @param non-empty-list<Line> $lines */
    public function __construct(
        public string $customer,
        public array $lines,
    ) {}

    public function total(): int
    {
        $total = 0;
        foreach ($this->lines as $line) {
            $total += $line->quantity * $line->pence;
        }

        return $total;
    }
}

<?php

declare(strict_types=1);

final readonly class Line
{
    /**
     * @param positive-int $quantity
     * @param positive-int $pence    the price of one
     */
    public function __construct(
        public string $sku,
        public int $quantity,
        public int $pence,
    ) {}
}

<?php

declare(strict_types=1);

final readonly class Order
{
    /** @param non-empty-list<Line> $lines */
    public function __construct(
        public string $customer,
        public array $lines,
        public string $chargeId,
    ) {}
}

<?php

declare(strict_types=1);

final class OutOfStock extends RuntimeException
{
    public static function forSku(string $sku): self
    {
        return new self("Not enough {$sku} in stock");
    }
}

<?php

declare(strict_types=1);

final class CardDeclined extends RuntimeException
{
    public static function forCustomer(string $customer): self
    {
        return new self("{$customer}'s card was declined");
    }
}

Running on PHP 8.3…

Run the tests, run PHPStan, or submit for both. Ctrl/⌘ + Enter submits from the editor.