Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-08-25
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
## Context

The Symphony Java BDK provides a blocking HTTP client (`ApiClientJersey2` using Glassfish Jersey and Apache HttpClient 5) and Resilience4j-based retry wrappers (`RetryWithRecovery` and `AuthenticationRetry`). In reactive or asynchronous environments, BDK calls are executed within thread pools where tasks may be cancelled/interrupted (e.g., `Thread.interrupt()`).

When a thread is interrupted during a socket operation:
1. Jersey/Apache HC throws a `ProcessingException` wrapping a `CancellationException` or `InterruptedException`.
2. `RetryWithRecovery` or `AuthenticationRetry` catches this exception, matches it against its transient network issue predicate (`isNetworkIssueOrMinorError` / `canAuthenticationBeRetried`), and attempts retries.
3. Because the thread's interrupted status is still set, all subsequent socket calls fail instantly and synchronously, printing highly verbose, false-positive connection errors (`log.error(...)` in `RetryWithRecovery.handleRecovery` or warning logs).
4. After all attempts are exhausted, the exception is propagated but often dropped downstream in reactive pipelines, yielding noisy `onErrorDropped` logs.

See `proposal.md` for the core problem statement.

## Goals / Non-Goals

**Goals:**
- Detect thread interruption in both the retry loop and the underlying Jersey client.
- Bypass all retry attempts immediately upon detecting an interruption.
- Suppress warning and error logging for cancelled/interrupted requests.
- Restore the thread's interrupted status to ensure downstream reactive frameworks function correctly.
- Propagate a standard, clean `java.util.concurrent.CancellationException`.

**Non-Goals:**
- Introducing custom unchecked exceptions (we will use `CancellationException` to avoid API expansion).
- Rewriting the entire HTTP exception classification system.

## Decisions

### D1 — Standard Exception Choice (`CancellationException`)
**Decision**: Use `java.util.concurrent.CancellationException` (which extends `IllegalStateException` / `RuntimeException`) as the propagated exception indicating request cancellation or interruption.
- **Rationale**: Since `InterruptedException` is a checked exception, throwing it directly would require changing the signature of many public BDK service and API methods, breaking binary and source compatibility for downstream consumers. `CancellationException` is a standard Java runtime exception, is semantically perfect for async task/call cancellations, and can be thrown transparently.
- **Alternative considered**: Create a custom `BdkInterruptedException` extending `ApiRuntimeException`. Rejected because it introduces new public API surface area for an open-source library that is unnecessary when a standard Java exception fits perfectly.

### D2 — Traversal Exception Interruption Detection Helper
**Decision**: Add a static utility method `isInterruption(Throwable t)` inside `RetryWithRecovery` to inspect the cause chain of any exception for `InterruptedException` or `CancellationException`.
- **Rationale**: Downstream HTTP libraries (such as Jersey) often wrap low-level interruption or socket exceptions into higher-level runtime wrappers (like `ProcessingException` or `RuntimeException`). A recursive cause-chain inspection ensures robust and implementation-agnostic detection of interruptions.
- **Alternative considered**: Matching only direct types. Rejected because it would fail to detect low-level interruptions wrapped by Jersey's client connector.

### D3 — Bypassing Client Logging & Silent Abort
**Decision**: In `ApiClientJersey2`, if a caught `ProcessingException` has an interruption in its cause chain, or if `Thread.currentThread().isInterrupted()` is true, propagate a `CancellationException` directly.
- **Rationale**: Avoids logging network issues in `ApiClientJersey2` or `RetryWithRecovery` for requests that were canceled deliberately. This ensures console logs remain clean during reactive cancellation.

### D4 — Interruption-Aware Services (`com.symphony.bdk.core.service.*`)
**Decision**: In all services exposed to interruptions (specifically real-time datafeed loops like V1/V2, health check service, agent version query, and pagination/bulk retrieval APIs), we check and propagate `CancellationException` or restored interrupted flags immediately when caught, rather than wrapping them in `BdkExtensionException`, swallowing them, or logging them as warnings/errors.
- **Rationale**: Real-time services (like Datafeed loops) are intended to exit cleanly and silently upon task cancellation. Throwing `CancellationException` and logging a simple info shutdown message ensures system resource teardown is clean, responsive, and logs remain silent under high concurrency.

### D5 — Fail-Fast Resilience4j Retry Wrapper (`Resilience4jRetryWithRecovery.java`)
**Decision**: Wrap Resilience4j's core `retryOnException` predicate to ensure it returns `false` if `isInterruption(throwable)` is `true` anywhere in the exception cause chain.
- **Rationale**: BDK Core relies on Resilience4j's decoration logic. Even if an exception categorizes as a transient network exception (like a socket timeout), the moment it is caused by a thread interruption or cancellation, Resilience4j must instantly bypass retries and propagate the cancellation without scheduling retries or writing noisy log entries.

### D6 — Centralized InterruptionUtil & Cycle Detection
**Decision**: Create a centralized `InterruptionUtil` utility class under the `:symphony-bdk-http:symphony-bdk-http-api` module. It defines an `InterruptionType` enum (`NONE`, `INTERRUPTION`, `THREAD_INTERRUPTION`) and a single-pass `getInterruptionType(Throwable)` helper with identity-based cycle detection.
- **Rationale**: Traversal of exception cause chains has a risk of infinite loops or CPU exhaustion if custom or third-party exceptions introduce circular references in their chains. By centralizing this logic in a shared module and returning an enum, we completely avoid code duplication, ensure identity-based cycle protection, and allow callers to check both interruption existence and specific thread-interrupted state in exactly one cause-chain traversal pass (saving performance and reducing complexity).

## Risks / Trade-offs

- **[Risk]** Downstream callers might not expect `CancellationException`.
- **Mitigation**: `CancellationException` is standard in Java concurrency (e.g., `Future.get()`, reactive pipelines). Reactive streams and `CompletableFuture` already handle `CancellationException` as standard cancel signals, making this the most robust and standard choice.
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
## Why

The Symphony BDK is frequently executed inside asynchronous thread pools or reactive pipelines (such as Spring WebFlux or Project Reactor). In these environments, when a task is cancelled, the executing thread is typically interrupted. Currently, the BDK's built-in retry wrapper (`RetryWithRecovery`) mistakenly treats the resulting socket exceptions as transient network errors and repeatedly attempts to retry the HTTP call. Because the thread remains interrupted, these retries fail instantly, clogging the console logs with false-positive error stack traces and dropping the final exception in reactive pipelines. The BDK must become interruption-aware to abort calls cleanly and silently upon thread interruption.

## What Changes

- **Implement Interruption Checks in `RetryWithRecovery` and `AuthenticationRetry`**: The core retry handlers will check `Thread.currentThread().isInterrupted()` and `isInterruption(e)`. If an interruption is detected, they will bypass all retry logic, restore the interrupted status (`Thread.currentThread().interrupt()`), and propagate a clean standard `java.util.concurrent.CancellationException`. This ensures that all BDK services, including client API services and authentication services, fail cleanly on interruption without retrying.
- **Suppress Cancellation Logging in HTTP Client**: The HTTP client implementations (e.g., `ApiClientJersey2`) will inspect the cause chain of caught exceptions (like `ProcessingException`). If caused by `CancellationException`, `InterruptedException`, or thread interruption, it will not print error logs and will propagate the exception cleanly.
- **Update BDK Connector Overrides**: The `MessageService.wrapOverrideException()` method will be updated to handle interruptions silently rather than throwing a `BdkExtensionException` with "Message override threw an unexpected exception".
- **Use Standard Exception**: A standard `java.util.concurrent.CancellationException` will be used as the unchecked exception representing thread/task cancellation. This avoids adding a custom BDK exception class, keeping API boundaries clean.

## Capabilities

### New Capabilities
- `request-interruption`: The BDK handles thread interruption gracefully by aborting requests without retries or noisy logging, and propagating a clean standard `CancellationException`.

### Modified Capabilities

## Impact

- **Code**:
- `symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/RetryWithRecovery.java`: check and bypass retries on thread interruption.
- `symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/resilience4j/Resilience4jRetryWithRecovery.java`: wrap Resilience4j's retry predicate to fail-fast and immediately abort retries on thread interruptions or cancellations.
- `symphony-bdk-core/src/main/java/com/symphony/bdk/core/auth/impl/AuthenticationRetry.java`: check and bypass retries on thread interruption in authentication loops.
- `symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/message/MessageService.java`: `wrapOverrideException` maps interruptions to `CancellationException`.
- `symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractAckIdEventLoop.java` / `AbstractDatafeedLoop.java` / `DatafeedLoopV1.java` / `DatafeedLoopV2.java`: detect thread cancellation or interruptions inside real-time datafeed loops to shut down cleanly and silently without false-positive error logs.
- `symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/health/HealthService.java` / `AgentVersionService.java`: ensure exceptions wrapping interruptions are translated and propagated as standard `CancellationException`.
- `symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/pagination/CursorBasedPaginatedService.java` / `OffsetBasedPaginatedService.java`: propagate thread interruption gracefully during bulk pagination queries.
- `symphony-bdk-http/symphony-bdk-http-jersey2/src/main/java/com/symphony/bdk/http/jersey2/ApiClientJersey2.java`: detect and propagate interruptions cleanly without throwing generic `ProcessingException`.
- **APIs**: No breaking API changes, since standard `CancellationException` is a runtime exception.
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
## Purpose

Defines the Symphony BDK's standard behavior and contract when executing blocking HTTP requests or retry operations on an interrupted thread, or when a request is cancelled.

## ADDED Requirements

### Requirement: Interruption-Aware Request Execution

The BDK request execution pipeline and retry recovery handler SHALL immediately abort when an interruption is detected, bypass all retry logic, suppress noisy logs, and propagate a standard `CancellationException`.

#### Scenario: Immediate Abort on Thread Interrupted Flag
- **WHEN** a BDK blocking request or retry operation is executed on a thread whose interrupted status is already set
- **THEN** the operation SHALL immediately bypass all retry logic, restore the thread's interrupted status, and throw a `java.util.concurrent.CancellationException`

#### Scenario: Immediate Abort on Interrupted Exception Cause
- **WHEN** a BDK blocking request throws an exception caused by `java.lang.InterruptedException` or `java.util.concurrent.CancellationException`
- **THEN** the operation SHALL bypass all retry logic and throw a `java.util.concurrent.CancellationException`. The thread's interrupted status SHALL only be restored if the cause chain contains an actual `java.lang.InterruptedException` or the thread's interrupted flag was already set, avoiding thread-pool poisoning from logical task cancellations.

#### Scenario: Silenced Request Cancellation Logging
- **WHEN** a BDK blocking request is cancelled or interrupted
- **THEN** the BDK client and retry handlers SHALL NOT output warning or error log statements related to the request failure or connection refusal

#### Scenario: Silent Interruption in Message Overrides
- **WHEN** a registered message override is executing and is interrupted or cancelled
- **THEN** the override wrapper SHALL restore the thread's interrupted status and propagate a `java.util.concurrent.CancellationException` without logging it as an unexpected override exception
22 changes: 22 additions & 0 deletions openspec/changes/archive/2026-08-25-interrupted-retry-fix/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
## 1. Retry Pipeline Changes

- [x] 1.1 Implement static `isInterruption(Throwable t)` helper in `RetryWithRecovery.java`
- [x] 1.2 Update `executeOnce()` to check for thread interruption and throw `CancellationException` to bypass recovery
- [x] 1.3 Update static `executeAndRetry()` helper to preserve interrupted status and propagate `CancellationException`
- [x] 1.4 Update `AuthenticationRetry.java` to check for thread interruption, bypass retries, and propagate `CancellationException` in authentication loops

## 2. Client Logging Suppression

- [x] 2.1 Update `getResponse()` in `ApiClientJersey2.java` to detect interruption/cancellation and cleanly propagate `CancellationException` silently

## 3. Override and Service Updates

- [x] 3.1 Update `wrapOverrideException()` in `MessageService.java` to handle interruptions silently and throw `CancellationException`
- [x] 3.2 Update other services in `com.symphony.bdk.core.service.*` (specifically Datafeed loops, Health, AgentVersion, and Paginated query services) to check and propagate thread interruption, avoiding logging false-positives or wrapping cancellations

## 4. Verification and Testing

- [x] 4.1 Write unit tests in `Resilience4jRetryWithRecoveryTest.java` verifying that an interrupted thread execution bypasses retries, preserves interrupted flag, and throws `CancellationException`
- [x] 4.2 Write unit tests in `MessageServiceTest.java` (or similar) verifying that `wrapOverrideException` maps interruptions to `CancellationException` silently
- [x] 4.3 Write unit tests in `ApiClientJersey2Test.java` verifying that `ApiClientJersey2` throws `CancellationException` without logging during interruptions
- [x] 4.4 Write unit tests in `AuthenticationRetryTest.java` verifying that `AuthenticationRetry` bypasses retries, preserves interrupted status, and propagates `CancellationException` on thread interruption
25 changes: 25 additions & 0 deletions openspec/specs/request-interruption/spec.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# request-interruption Specification

## Purpose
Defines the Symphony BDK's standard behavior and contract when executing blocking HTTP requests or retry operations on an interrupted thread, or when a request is cancelled.
## Requirements
### Requirement: Interruption-Aware Request Execution

The BDK request execution pipeline and retry recovery handler SHALL immediately abort when an interruption is detected, bypass all retry logic, suppress noisy logs, and propagate a standard `CancellationException`.

#### Scenario: Immediate Abort on Thread Interrupted Flag
- **WHEN** a BDK blocking request or retry operation is executed on a thread whose interrupted status is already set
- **THEN** the operation SHALL immediately bypass all retry logic, restore the thread's interrupted status, and throw a `java.util.concurrent.CancellationException`

#### Scenario: Immediate Abort on Interrupted Exception Cause
- **WHEN** a BDK blocking request throws an exception caused by `java.lang.InterruptedException` or `java.util.concurrent.CancellationException`
- **THEN** the operation SHALL bypass all retry logic and throw a `java.util.concurrent.CancellationException`. The thread's interrupted status SHALL only be restored if the cause chain contains an actual `java.lang.InterruptedException` or the thread's interrupted flag was already set, avoiding thread-pool poisoning from logical task cancellations.

#### Scenario: Silenced Request Cancellation Logging
- **WHEN** a BDK blocking request is cancelled or interrupted
- **THEN** the BDK client and retry handlers SHALL NOT output warning or error log statements related to the request failure or connection refusal

#### Scenario: Silent Interruption in Message Overrides
- **WHEN** a registered message override is executing and is interrupted or cancelled
- **THEN** the override wrapper SHALL restore the thread's interrupted status and propagate a `java.util.concurrent.CancellationException` without logging it as an unexpected override exception

Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.symphony.bdk.core.auth.impl;

import static com.symphony.bdk.core.retry.RetryWithRecovery.networkIssueMessageError;
import static com.symphony.bdk.http.api.util.InterruptionUtil.checkInterruptionAndThrow;

import com.symphony.bdk.core.auth.exception.AuthUnauthorizedException;
import com.symphony.bdk.core.config.model.BdkRetryConfig;
Expand Down Expand Up @@ -89,6 +90,7 @@ public T executeAndRetry(String name, String address, SupplierWithApiException<T
}
throw new ApiRuntimeException(e);
} catch (Throwable t) {
checkInterruptionAndThrow(t, "Execution interrupted: " + t.getMessage());
throw new RuntimeException(networkIssueMessageError(t,address), t);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
import com.symphony.bdk.core.retry.function.SupplierWithApiException;
import com.symphony.bdk.http.api.ApiException;
import com.symphony.bdk.http.api.ApiRuntimeException;
import com.symphony.bdk.http.api.util.InterruptionUtil;
import com.symphony.bdk.http.api.util.InterruptionUtil.InterruptionType;
import static com.symphony.bdk.http.api.util.InterruptionUtil.checkInterruptionAndThrow;

import java.util.concurrent.CancellationException;

import lombok.extern.slf4j.Slf4j;
import org.apiguardian.api.API;
Expand Down Expand Up @@ -63,6 +68,7 @@ public static <T> T executeAndRetry(
} catch (ApiException e) {
throw new ApiRuntimeException(e);
} catch (Throwable t) {
checkInterruptionAndThrow(t, "Execution interrupted: " + t.getMessage());
throw new RuntimeException(networkIssueMessageError(t, address), t);
}
}
Expand Down Expand Up @@ -101,9 +107,16 @@ public RetryWithRecovery(
* @throws Throwable in case an exception has been thrown by the {@link #supplier} or by the recovery functions.
*/
protected T executeOnce() throws Throwable {
if (Thread.currentThread().isInterrupted()) {
InterruptedException ie = new InterruptedException("Thread was interrupted prior to execution");
CancellationException ce = new CancellationException("Thread was interrupted prior to execution");
ce.initCause(ie);
throw ce;
}
try {
return supplier.get();
} catch (Exception e) {
checkInterruptionAndThrow(e, "Execution interrupted: " + e.getMessage());
if (ignoreException.test(e)) {
log.debug("{} ignored: {}", e.getClass().getCanonicalName(), e.getMessage());
return null;
Expand Down
Loading
Loading