PDGGK opened a new pull request, #37341:
URL: https://github.com/apache/beam/pull/37341
## Description
Fixes #37176
The `parseAndThrow` method in `Call.java` was wrapping retryable exceptions
(`UserCodeTimeoutException`, `UserCodeRemoteSystemException`) in a generic
`UserCodeExecutionException`, which breaks the retry logic that depends on
`exception.shouldRepeat()` returning true.
## Problem
When user code throws a `UserCodeTimeoutException` or
`UserCodeRemoteSystemException` (which have `shouldRepeat() = true`), the old
implementation would wrap these in a generic `UserCodeExecutionException`
(which has `shouldRepeat() = false`), causing the `Repeater` to not retry the
operation as intended.
## Solution
- Scan the full causal chain using Guava's `Throwables.getCausalChain()`
- Preserve all specific retryable exception types
(Quota/Timeout/RemoteSystem)
- Prefer specific types over generic `UserCodeExecutionException` when both
exist in the chain to prevent masking of retryable exceptions
- Handle circular causal chains gracefully by catching
`IllegalArgumentException`
## Changes
### Modified Files
**`sdks/java/io/rrio/src/main/java/org/apache/beam/io/requestresponse/Call.java`**
(+31/-7 lines)
- Added `Throwables` import for causal chain traversal
- Rewrote `parseAndThrow` method to scan full exception chain
- Added logic to prefer specific exception types over generic ones
- Added circular reference handling
**`sdks/java/io/rrio/src/test/java/org/apache/beam/io/requestresponse/CallTest.java`**
(+264/-2 lines)
- Added 10 new unit tests covering various exception scenarios
## Testing
Added comprehensive test coverage for:
- ✅ Direct retryable exceptions (Timeout, RemoteSystem, Quota)
- ✅ Nested exceptions wrapped in `UncheckedExecutionException`
- ✅ Generic `UserCodeExecutionException` masking specific types (3 scenarios)
- ✅ Triple-nested exceptions
- ✅ Circular reference in causal chain
- ✅ Non-UserCode exceptions (RuntimeException)
**Test Results:**
- `CallTest`: All tests passing
- Full `rrio` test suite: 90 tasks passing ✅
- Code formatting: `spotlessCheck` passing ✅
## Impact
**Behavior Change:** Code that previously saw a generic
`UserCodeExecutionException` may now see the specific subtype
(`UserCodeTimeoutException`/`UserCodeRemoteSystemException`). This is the
**intended fix** to restore proper retry behavior.
**Performance:** Minimal impact - exception chain traversal only occurs on
error paths.
**Backwards Compatibility:** The change improves correctness. Any code that
relied on exceptions being wrapped was working around a bug.
## Example
**Before:**
```java
// User code throws UserCodeTimeoutException
throw new UserCodeTimeoutException("timeout");
// parseAndThrow wraps it
throw new UserCodeExecutionException(cause); // shouldRepeat() = false ❌
// Repeater sees generic exception and doesn't retry
```
**After:**
```java
// User code throws UserCodeTimeoutException
throw new UserCodeTimeoutException("timeout");
// parseAndThrow preserves it
throw (UserCodeTimeoutException) throwable; // shouldRepeat() = true ✅
// Repeater sees timeout exception and retries
```
## Checklist
- [x] Addresses issue #37176
- [x] All retryable exceptions are preserved
- [x] Circular references handled gracefully
- [x] Generic exception masking prevented
- [x] Comprehensive test coverage added (10 new tests)
- [x] All existing tests pass
- [x] Code formatting clean (spotless)
- [x] No breaking changes to public API
- [x] Commit message follows conventions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
