weiqingy commented on PR #951: URL: https://github.com/apache/flink-agents/pull/951#issuecomment-5175951893
Good catch, that gap was real. I reproduced it before fixing: an `IllegalStateException` out of `registerRepo` left both the earlier repository and the failing one open. Fixed in 59344b0e. The source loop is now wrapped in an outer guard that runs `closeRepos()` on every failure path, attaches any cleanup failure as suppressed, and rethrows the original itself. Which exceptions get wrapped in `IllegalStateException` is unchanged: `IOException` and `IllegalArgumentException` still wrap with the same message and cause, and everything else still propagates with the same type and identity. The only behavior change is that the cleanup now runs, which matches what the Python side does. The handler around the cleanup call itself was also too narrow. `closeRepos()` catches only `Exception` per repository, and `SkillRepository.close()` declares no checked exception, so an `Error` from a repository's `close()` escaped and replaced the primary failure with nothing suppressed. Both handlers now take `Throwable`. Two regression tests. The first is your scenario: `getSkills()` throws `IllegalStateException`, and it asserts both repositories were closed and that the original exception reaches the caller by identity. Identity rather than type matters here, because `IllegalStateException` is also what the wrapping catch builds, so an `assertThrows` on the type alone would pass either way. The second covers the `Error` half you named. Nothing else distinguishes `catch (Throwable)` from `catch (Exception)`, so a future narrowing would otherwise build green. `runtime` 402 passed, `spotless:check` clean. -- 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]
