skip to content

As a principal engineer, how would you prevent and detect the whole class of 'silently non-transactional method' bugs across a large codebase?

level: principalimportance: nice to knowfreq 22%

answer

  1. Failure is silent -> manufacture a signal
  2. ArchUnit rule: no @Transactional on private/final/protected/static
  3. Rollback integration test is the gold proof
  4. TransactionInterceptor DEBUG log
  5. AspectJ = deliberate documented escape hatch

basics

~20 s

Combine prevention and detection: keep @Transactional on public non-final methods only, enforce it with static analysis/lint rules, add integration tests that assert rollback, and turn on transaction-interceptor DEBUG logging or startup validation to catch silently-skipped advice.

solid answer

~40 s

I treat it as a governance problem, not a one-off fix. Prevention: standardize on proxy mode with a coding rule that @Transactional lives only on public, non-final methods on a service bean, never on internal helpers or self-invoked calls; in Kotlin, mandate the kotlin-spring all-open plugin. Detection: add a custom ArchUnit/Checkstyle/detekt rule that flags @Transactional on private, protected, final, or static methods, so CI fails before merge. At runtime, enable DEBUG on org.springframework.transaction.interceptor in a smoke test to confirm advice actually fires, and write integration tests that force an exception and assert the row was rolled back — the only test that truly proves the boundary works. For legitimate non-public cases, opt into AspectJ weaving deliberately, documented, rather than accidentally. The core insight: the failure is silent, so you must manufacture a signal.

code

java · 14 lines
java
// ArchUnit guard: fail the build if @Transactional lands where the proxy can't advise it.
@AnalyzeClasses(packages = "com.katajob.backend")
class TransactionalVisibilityTest {

    @ArchTest
    static final ArchRule transactional_methods_must_be_public_and_non_final =
        methods()
            .that().areAnnotatedWith(Transactional.class)
            .should().bePublic()
            .andShould().notHaveModifier(JavaModifier.FINAL)
            .andShould().notHaveModifier(JavaModifier.STATIC)
            .because("proxy-mode @Transactional is silently ignored on "
                   + "private/final/protected/static methods");
}

go deeper

for a junior

Out of scope; only the 'make it public' fix is expected.

for a middle

Might suggest a rollback test but not a governance strategy.

for a senior

Should propose rollback tests plus DEBUG logging and the extract-bean fix.

for a principal

Should design multi-layer guardrails (static ArchUnit rule, fail-fast startup check, rollback tests) and treat AspectJ as a deliberate exception, generalizing across proxy-based annotations.

### Framing The defining hazard is **silence**: a misplaced `@Transactional` produces no compile error, no startup error, and no log — the method simply isn't transactional. A senior individual fix (make it public) doesn't scale; a principal builds **guardrails and signals** so the whole team can't reintroduce it. ### 1. Prevention (make the bad state hard to write) - **Coding standard**: `@Transactional` only on **public, non-final** methods of a bean, invoked **from another bean** (never self-invocation). Internal transactional work goes into a separate collaborator bean so the call crosses the proxy boundary. - **Kotlin**: require the **`kotlin-spring` (all-open)** plugin so beans aren't final-by-default; forbid manually re-`final`-ing advised methods. - **Choose the proxy strategy explicitly** and document it; don't let `proxyTargetClass` drift. ### 2. Static detection (fail fast in CI) - **ArchUnit** rule: reject methods annotated `@Transactional` that are `private`, `protected`, package-private, `final`, or `static`. This runs in the normal test phase and blocks merges. (This project already enforces module boundaries with ArchUnit/Spring Modulith, so adding a transaction-visibility rule fits the existing gate.) - **detekt (Kotlin) / Checkstyle-PMD (Java)** custom rules as a lighter-weight lint alternative. - Optionally a **BeanPostProcessor** at startup that inspects each bean's annotated methods and throws if any advised annotation sits on a non-advisable method — converts silent-skip into a loud fail-fast. ### 3. Runtime / test detection (prove it actually works) - **Rollback integration test**: the gold standard. In a `@SpringBootTest` (or `@DataJpaTest` with a real transaction manager), call the method so it throws a `RuntimeException`, then assert the database change was **not** persisted. If the annotation was silently skipped, the row survives and the test fails. - **DEBUG logging** on `org.springframework.transaction.interceptor.TransactionInterceptor` (and `org.springframework.orm.jpa` / `JpaTransactionManager`) in a smoke profile: you'll see `Getting transaction for [...]` only for methods that were truly advised — a missing line reveals a skipped method. - Beware the **false-negative** where a skipped private method silently participates in an outer transaction; design the test so the method is the transaction entry point, with no ambient transaction. ### 4. Deliberate escape hatch When non-public transactional boundaries are genuinely needed, adopt **AspectJ weaving** (`@EnableTransactionManagement(mode = AdviceMode.ASPECTJ)` + `spring-aspects` + LTW/CTW) as a **conscious, documented** architectural choice — never as an accidental default. Weigh the added build/agent complexity against the rare need. ### 5. Organizational leverage - Bake the ArchUnit rule and a canonical rollback-test template into the project's starter/archetype so every new service inherits them. - Add the rule to code review checklists and to the CLEAN_CODE rubric. - Remember the rule generalizes: **`@Async`, `@Cacheable`, `@Retryable`, `@Validated`** all share the proxy limitation, so one ArchUnit rule family covers the whole category. ### Key takeaway The engineering move is converting an **invisible failure into a visible one** — via static rules, fail-fast startup checks, and rollback tests — while keeping a documented AspectJ path for the legitimate exceptions.

  • Why is a rollback integration test more trustworthy than checking DEBUG logs for confirming a transaction boundary?
    Logs confirm advice fired, but a rollback test proves the end-to-end semantics: it forces an exception and asserts the data was actually not committed. It catches misconfigured propagation, wrong rollback rules (checked exceptions), and skipped advice all at once.
  • A startup BeanPostProcessor that throws on misplaced @Transactional — what's the risk versus an ArchUnit rule?
    It only fires at runtime startup (later than CI, can crash a deploy) and must correctly resolve the effective proxy strategy and Kotlin openness, which is fiddly. ArchUnit fails earlier in CI and is simpler, though it inspects bytecode statically and may miss dynamically-registered beans.

context

resolving links…