tcurdt / jdeb

This library provides an Ant task and a Maven plugin to create Debian packages from Java builds in a truly cross platform manner.
https://github.com/tcurdt/jdeb
Apache License 2.0
476 stars 253 forks source link

Prevent swallowing of error during debian creation with the final close #707

Closed chali closed 1 week ago

chali commented 1 week ago

We have just hit the same problem as #234 What do you think about following approach for those situations?

tcurdt commented 1 week ago

Thanks for the PR!

I guess the question I have is:

Why wouldn't an exception in the producer bubble up to produce the very same error condition? Why is wrapping even necessary?

codecov-commenter commented 1 week ago

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 71.72%. Comparing base (92fee05) to head (f1b2754). Report is 69 commits behind head on master.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #707 +/- ## ============================================ + Coverage 71.03% 71.72% +0.68% - Complexity 96 98 +2 ============================================ Files 7 7 Lines 580 587 +7 Branches 75 76 +1 ============================================ + Hits 412 421 +9 + Misses 121 120 -1 + Partials 47 46 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

chali commented 1 week ago

The situation that is happening is if there is an Exception in between tarOutputStream.putArchiveEntry(fileEntry); and tarOutputStream.closeArchiveEntry(); like full temp space in the linked issue. It leaves tar entry in unclosed state. Then calling tarOutputStream.close() that is in the finally causes another exception. The first one is ignored and the second one is propagated outside. Leaving users without clue what actually happened.

tcurdt commented 1 week ago

Looking better! Thanks for the contribution!

Feel free to also include yourself as contributor https://github.com/tcurdt/jdeb/blob/master/pom.xml#L88 And this also still needs to be added to https://github.com/tcurdt/jdeb/blob/master/HISTORY.md

...and I just noticed a new release is overdue 🙃

chali commented 1 week ago

Feel free to also include yourself as contributor https://github.com/tcurdt/jdeb/blob/master/pom.xml#L88 And this also still needs to be added to https://github.com/tcurdt/jdeb/blob/master/HISTORY.md

Working on those. https://github.com/tcurdt/jdeb/pull/708

...and I just noticed a new release is overdue 🙃

:-D little bit