Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent close from hiding causal exceptions #528 #7764

Merged
merged 1 commit into from
Feb 14, 2023

Conversation

lbergelson
Copy link
Member

@droazen droazen self-assigned this Apr 6, 2022
@droazen droazen self-requested a review April 6, 2022 20:16
@codecov
Copy link

codecov bot commented Apr 27, 2022

Codecov Report

Merging #7764 (7c3e3a2) into master (110f4f5) will increase coverage by 41.856%.
The diff coverage is 100.000%.

❗ Current head 7c3e3a2 differs from pull request most recent head feb9d08. Consider uploading reports for the commit feb9d08 to get more accurate results

Additional details and impacted files
@@               Coverage Diff                @@
##              master     #7764        +/-   ##
================================================
+ Coverage     45.084%   86.941%   +41.856%     
- Complexity     27429     36867      +9438     
================================================
  Files           2337      2211       -126     
  Lines         182973    173379      -9594     
  Branches       20080     18709      -1371     
================================================
+ Hits           82492    150737     +68245     
+ Misses         94620     16057     -78563     
- Partials        5861      6585       +724     
Impacted Files Coverage Δ
...stitute/hellbender/cmdline/CommandLineProgram.java 84.810% <100.000%> (+1.584%) ⬆️
...org/broadinstitute/hellbender/engine/GATKTool.java 90.041% <100.000%> (+0.372%) ⬆️
...stitute/hellbender/engine/ReferenceFileSource.java 63.636% <0.000%> (-27.273%) ⬇️
...bender/tools/walkers/vqsr/VariantRecalibrator.java 61.327% <0.000%> (-24.841%) ⬇️
...hellbender/tools/walkers/sv/CollectSVEvidence.java 50.654% <0.000%> (-24.234%) ⬇️
...ute/hellbender/tools/walkers/fasta/ShiftFasta.java 58.586% <0.000%> (-17.172%) ⬇️
...stitute/hellbender/engine/ReferenceDataSource.java 66.667% <0.000%> (-16.667%) ⬇️
...nder/utils/runtime/ProcessControllerAckResult.java 86.364% <0.000%> (-8.373%) ⬇️
...r/arguments/CopyNumberArgumentValidationUtils.java 73.333% <0.000%> (-8.000%) ⬇️
...lbender/tools/walkers/vqsr/VariantDataManager.java 71.660% <0.000%> (-7.287%) ⬇️
... and 1081 more

* Exceptions thrown by a tool can be hidden by unsafe close methods throwing additional exceptions.
  Avoid this by making use of try-with-resources suppressed exception handling in order to surface the
  primary exception as well as the secondary ones.

* Fixes #528
Copy link
Collaborator

@droazen droazen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@droazen droazen merged commit f7dea3c into master Feb 14, 2023
@droazen droazen deleted the lb_prevent_exception_hiding branch February 14, 2023 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exception in onShutdown mask the actual cause of problems in onStartup.
2 participants