Code Review After Porting Java to Rust: 15 Findings, 6 Bugs Carried Over From Java

Porting Java code to Rust and getting a green parity check (same inputs through the old and new versions, outputs compared) doesn't mean there are no bugs.

I ran Claude Code's /code-review at its highest level over the entire port PR and got 15 findings. Six of them were bugs from the Java days that the port carried over as-is.

That was expected. The porting principle was to preserve old behavior, and parity checks can only prove the two sides match. They can't catch places where both are wrong in the same way.

How the backend moved wholesale from Spring Boot to Rust is covered in migrating from Java Spring Boot to Rust in one cutover. This review came after merging to main.

The issues that could actually hurt someone included service outages, operations failing silently, accounting holes, and broken alerts, plus unauthenticated endpoints missing input validation and rate limits. All of them are fixed.

Below are the four most worth taking away.

REQUIRES_NEW, ported from the annotation

Simplified, the Java audit service looked like this:

public void record(/* ... */) {
    recordInTransaction(/* ... */); // self-invocation inside the same object, does not go through the Spring proxy
}

@Transactional(propagation = Propagation.REQUIRES_NEW)
public void recordInTransaction(/* ... */) { /* write the audit record */ }

Reading the annotation, you'd think audit opens a new transaction and writes independently, and the Rust version did exactly that with a separate connection.

But record() calls it from inside the same object without going through Spring's proxy, so REQUIRES_NEW never took effect. Spring's documentation says so:

In proxy mode (which is the default), only external method calls coming in through the proxy are intercepted. This means that self-invocation (in effect, a method within the target object calling another method of the target object) does not lead to an actual transaction at runtime even if the invoked method is marked with @Transactional.

That's from Spring Framework's Using @Transactional. What Java actually did was run audit in the caller's transaction: if the audit write failed, the whole action rolled back.

Following the annotation changed the behavior.

The fix was to go back to the caller's transaction. For events that must be recorded even when the action is rejected, like a failed login, the transaction is ended first and the audit is written after.

Pulling on this thread turned up several more paths that held a transaction while grabbing a second connection from the pool. Those now use a single connection inside the transaction.

When porting, go by actual behavior and treat annotations and comments as a reference only. For me that was the biggest lesson of the whole port.

sqlx returns Ok on a failed transaction

In a Postgres transaction, once any statement fails, the transaction enters the aborted state, and a later COMMIT is treated as a rollback.

Postgres doesn't raise an error. It just returns a ROLLBACK tag, and sqlx returns Ok when it gets one.

The pause-lending loop had a section that swallowed SQL errors and kept going. The final commit returned Ok, the caller assumed success, and in fact none of the data had been written.

The fix was to read the ReadyForQuery status in a vendored copy of sqlx-postgres and return an error when committing an aborted transaction. That message is how Postgres tells the client whether it's idle, in a transaction, or in a failed one.

The pause loop now wraps each lending config in a savepoint, so one failure doesn't drag the others down with it.

Client disconnects kill the handler

In the Servlet world, a request that comes in runs to completion. If the client disconnects partway, the worst case is that the response can't be written.

hyper 1.x is different. When the client disconnects, it drops the handler's future outright, so any await can be the end of the line.

That means if a user clicks pause lending and closes the tab before the transaction commits, the transaction rolls back. If an admin manually triggers the engine and the request gets cut off, the lock it holds is never released.

The fix was to spawn every request into its own tokio task at the outermost layer of the router. A client disconnect now only throws away the response, the same as with servlets.

The circuit breaker's probe permits and the distributed lock were also changed to be returned on drop.

Separate user intent from billing eligibility

The Java version suspended a user by switching off their lending toggle.

The problem is that users press pause and Resume themselves too. One flag meant both "does the user want to lend" and "is this account allowed to lend," so a suspended user could get around the suspension just by pressing Resume.

Now suspension leaves the flag alone, and the engine excludes suspended users when it picks lending configs. Pressing Resume while suspended returns 400, and once payment resumes, any pause the user set themselves is kept.

I also considered keeping the flag flip and storing the pre-suspension value separately. But the two intents would still share one flag, and the bypass would just come back in a different form.

Parallel fixes, then review the whole diff

The 15 findings were split into 7 groups, each fixed in parallel in its own git worktree with its own test DB.

Every finding needed a regression test that fails before the fix and passes after.

At merge time each one was reviewed individually, and after every merge the next PR pulled in main and reran CI, because two PRs with no textual conflict can still fail to compile together.

If the porting principle is to preserve old behavior, green parity only means the two sides match.

After the port, you need another review over the entire diff to find the bugs that came along for the ride.