Skip to content

CredentialBinding.revoke swallows all RuntimeExceptions and retries with no backoff, hiding the underlying failure #60

@nficano

Description

@nficano

The private revoke method in arcp-runtime/src/main/java/dev/arcp/runtime/credentials/CredentialBinding.java around line 83 loops up to MAX_REVOKE_ATTEMPTS times calling provisioner.revoke(id).join(). The catch is bare RuntimeException, which means it swallows everything — IllegalStateException from a broken provisioner, NullPointerException from a programming error, CompletionException unwrapped from the join call, anything. On the final attempt it logs a one-line WARN "credential revoke failed after {} attempts for {}" with no exception attached and silently returns. The caller (typically SessionLoop.shutdown via revokeAll) has no way to discover that a revocation failed and no way to know why. The credential remains live with the upstream provider. There is also no backoff between retries: three immediate join() calls against a transient failure (e.g. a network blip to the upstream credentials API) will all fail within microseconds and exhaust the budget before any recovery is possible.\n\nFix prompt: In arcp-runtime/src/main/java/dev/arcp/runtime/credentials/CredentialBinding.java change the catch block to log the actual exception object (log.warn("credential revoke attempt {} failed for {}", attempt, id, e) — the trailing Throwable enables SLF4J's exception logging). On the final-attempt failure, in addition to logging, write the still-outstanding credential id back to the CredentialRevocationStore so a recovery process can retry asynchronously — the store already has a markRevoked method but no "failed to revoke" marker; add a markRevocationFailed(CredentialId, Throwable) method to the interface and a default implementation. Add a backoff between attempts: Thread.sleep(100L * attempt) (or, if Java 21 virtual threads make this acceptable, Thread.ofVirtual().start(...).join() style — staying with sleep is fine on virtual threads). Narrow the catch to CompletionException and unwrap its cause for logging. Add a JUnit test using a CredentialProvisioner stub that fails the first two attempts and succeeds the third, asserting the third attempt succeeds and the store sees markRevoked exactly once.

Metadata

Metadata

Assignees

No one assigned

    Labels

    code-qualityCode quality, maintainability, and API designseverity:mediumMedium severity

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions