PDGGK commented on PR #37342:
URL: https://github.com/apache/beam/pull/37342#issuecomment-5231505821
Since this has been sitting a while, let me pre-empt the thing the diff most
likely raises: it deletes the `UserCodeQuotaException` branch. That case is not
lost — it is subsumed, because all three retryable subclasses extend
`UserCodeExecutionException`:
| | `shouldRepeat()` |
| --- | --- |
| `UserCodeExecutionException` | **false** |
| `UserCodeQuotaException` | true |
| `UserCodeTimeoutException` | true |
| `UserCodeRemoteSystemException` | true |
`Repeater` decides purely on that (`Repeater.java:114-117`):
```java
} catch (UserCodeExecutionException e) {
if (!e.shouldRepeat()) {
throw e;
}
latestError = Optional.of(e);
}
```
So today a timeout or remote-system failure is flattened into a bare
`UserCodeExecutionException`, `shouldRepeat()` flips `true` → `false`, and the
repeater gives up on exactly the failures it exists to retry. Quota escapes
only because it was enumerated by hand.
One deliberate behaviour difference worth naming: the current code answers a
quota failure with `new UserCodeQuotaException(cause)`, a fresh instance; this
rethrows the original. `shouldRepeat()` is identical either way, and the
original keeps its own stack trace and any state a caller attached. If you
would rather keep re-wrapping, say so and I will restore that arm — it just is
not needed for correctness.
The branch is `CLEAN` against master, but its green CI is from 2026-07-25.
Happy to rebase for a fresh run if that helps you merge it; I did not want to
churn the branch unprompted.
@ahmedabu98 @kennknowles
--
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]
