PMD's ExceptionAsFlowControl flagged a throw that was never caught

CI passed while a local PMD run failed on the same commit, PMD version, and ruleset. The difference was a target/ directory.

The problem

A backend Maven project runs PMD 7.26.0 through the maven-pmd-plugin behind a mise task. After bumping PMD from 7.22.0 to 7.26.0, the task surfaced one violation on a fresh worktree:

ContractService:645 Rule:ExceptionAsFlowControl Priority:3
Exception thrown at line 621 is caught in this block.

The flagged method looked like this:

// example-group/example-monorepo/services/api/ContractService.java
public @Nullable String fetchContractIframeUrl(Integer fileId) {
  // ...
  try (Response response = getClient(user).getSignature(file.getExternalSignatureId())) {
    if (response.isSuccessful()) {
      try (ResponseBody responseBody = response.body()) {
        if (responseBody == null) {
          throw new ExternalServiceException("Response body is null"); // line 621
        }
        // ...
      }
    }
  } catch (IOException ex) {                 // line 645
    // ... set error state, persist activity, rethrow ...
    throw new ExternalServiceException("...", ex);
  }
}

PMD claimed the throw at line 621 was caught by the catch (IOException) at line 645.

What the rule actually checks

I decompiled the rule with javap and inspected the ExceptionAsFlowControlRule bytecode. Stripped to pseudocode, the visit method does this for every throw:

thrownType = throwStatement.expression.type
walk up the AST:
  - if we reach a method body declaration, stop
  - if we hit a catch clause, skip past it (a throw inside its own catch is fine)
  - if we hit a try statement, for each of its catch clauses:
      if thrownType.isSubtypeOf(catchType)
         and the catch does more than rethrow:
           report "Exception thrown at line N is caught in this block"

So the rule fires when the thrown type is a subtype of an enclosing catch’s parameter type, in a catch that performs more than a bare rethrow. The match hinges entirely on isSubtypeOf. That depends on PMD resolving both types.

Why it was a false positive

The exception extends RuntimeException, so an enclosing catch (IOException) cannot catch it:

$ javap -classpath target/classes ...ExternalServiceException
public class ExternalServiceException extends java.lang.RuntimeException

At runtime, catch (IOException) cannot catch it. The throw propagates to the request layer. The “exception as flow control” smell (using try/catch as a goto) does not apply here. PMD 7’s type resolution needs the project’s compiled classes on its analysis classpath to prove that subtype relationship. With them, RuntimeException.isSubtypeOf(IOException) is false and the violation disappears.

The violation was conditional on build state. I reproduced both with the unchanged production source:

$ rm -rf services/api/target && mise run pmd     # no compiled classes
ExceptionAsFlowControl ... BUILD FAILURE

$ mvn compiler:compile pmd:check            # classes present
PMD version: 7.26.0
BUILD SUCCESS

The pmd task ran mvn pmd:check directly. PMD analyzes source and therefore did not compile the project first. On a fresh worktree, target/classes did not exist, the project-local exception hierarchy went unresolved, and PMD treated the throw as broadly catchable.

CI did not report the violation because the api:quality job runs test-compile before pmd:check:

# ci/projects/api.yml
- mvn -B -ntp test-compile spotless:check checkstyle:check pmd:check spotbugs:check

So the gate was green in CI and red locally, depending on whether stale target/classes happened to be lying around.

The fix

Compile main classes before PMD. One line in the canonical task:

# mise.toml
[tasks."java:pmd"]
dir = "services/api"
run = "mvn {{vars.maven_base_args}} compiler:compile pmd:check"

And the service wrapper delegates instead of duplicating the bare goal:

# services/api/mise.toml
[tasks.pmd]
alias = "pmd"
depends = ["//:java:pmd"]

Now rm -rf target && mise run pmd is deterministic: compiler:compile runs, then PMD sees the resolved types.

I considered three alternatives and rejected each:

• @SuppressWarnings("PMD.ExceptionAsFlowControl"). PMD supports per-rule suppression, and this codebase already uses it. The annotation would document a nonexistent defect and leave the task non-deterministic for the next person.
• Disable ExceptionAsFlowControl in the ruleset. Weakens the rule globally to address one invocation.
• Return null instead of throwing. The next section covers this.

The tempting code change

My first fix made PMD green by changing the code:

if (responseBody == null) {
  log.error("Response body is null ...");
  return null;                 // was: throw new ExternalServiceException(...)
}

PMD passed. Spotless passed. The existing tests passed. Every configured gate was green, so the change entered review.

On review, I traced the effect of that one-line change on the HTTP contract. A successful response from the external signature service with an empty body is an upstream protocol failure. The original throw propagated to the app’s catch-all handler:

// example-group/example-monorepo/services/api/exceptions/ApiExceptionHandler.java
@ExceptionHandler(RuntimeException.class)
public ResponseEntity<?> handleRuntimeException(RuntimeException ex) {
  return ResponseEntity.badRequest().body(getErrorFrom(ex));  // HTTP 400
}

The contract-iframe-url endpoint turned that into an HTTP 400, and the frontend showed an error toast. My return null turned it into HTTP 200 with an IframeUrlResponse whose url was empty. No toast. The signature modal opened with a blank iframe.

The web client confirms this matters:

// example-group/example-monorepo/frontend-nuxt/.../contracts.service.ts
async getContractIframeUrl(contractId: number): Promise<string> {
  const response = await fetchContractIframeUrl(contractId);
  return typeof response?.url === "string" ? response.url : "";
}

It treats both a thrown error and an empty string as failure, but only the error path produces the toast. A successful 200 with an empty URL left the UI without an error toast and opened a blank iframe.

This showed that a static-analysis change can alter runtime behavior even when the existing tests and formatters pass. The passing pipeline established that its configured checks passed; it did not establish the HTTP contract being edited.

Results

The merged fix contains zero production source changes. The diff against main is three lines across two config files and one doc:

 services/api/docs/pmd-tech-debt.md | 2 +-   (PMD version header 7.22.0 -> 7.26.0)
 services/api/mise.toml         | 2 +-   (api:pmd delegates to //:java:pmd)
 mise.toml                     | 2 +-   (java:pmd runs compiler:compile pmd:check)

The regression check for this defect is the clean-checkout invocation:

rm -rf services/api/target && mise run pmd   # was FAILURE, now BUILD SUCCESS

I did not add a mock-driven unit test for the null-body branch. With runtime behavior unchanged, such a test would assert a one-line edit against a stub and prove nothing about the HTTP contract or the persistence side effects.

Lessons

When a static-analysis tool flags code that has shipped for months, I first check whether the tool’s view matches the source and runtime behavior. Decompiling the rule showed that the match was type-based, which pointed to type resolution.

A lint fix that touches a throw or a return type is a contract edit, not a mechanical change. I should test that contract, including the 400-versus-200 behavior, rather than only asserting a stubbed branch.

References

• PMD ExceptionAsFlowControl rule (Java design). The rule definition and its “caught in this block” message.
• PMD suppressing warnings. The @SuppressWarnings("PMD.<RuleName>") pattern I considered and rejected.
• maven-pmd-plugin. Runs PMD over source. It does not compile the project, so the analysis classpath only contains compiled classes that already exist.