kratos0718 commented on PR #166: URL: https://github.com/apache/maven-shared-jar/pull/166#issuecomment-5390316265
All three addressed, and the middle one changed the shape of the fix for the better. Catching `RuntimeException` was me reaching for the wrong tool. What the constructor actually needs is a release-on-failure guarantee, not an exception handler, so it now uses a `constructed` flag with `finally` and catches nothing at all. That also covers `Error` paths the old version silently missed. On the entries list and the sort being inside the try: they are still in it, but the block is no longer a catch, so the question changes. Nothing in those two lines throws anything worth handling. They sit inside the `finally` scope only because the `JarFile` is already open by then, and anything that goes wrong between opening it and assigning `jarData` leaks the handle, since the constructor never returns and the caller gets no reference to close. Comment reworded. `mvn test` is 78 passing. -- 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]
