From 9712fed7a53267d5916e03ccd3f27d849fbbcb25 Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Mon, 27 Feb 2012 13:34:44 -0500 Subject: [PATCH 1/7] Trap SAMFormatException and rethrow as MalformatedBAM exception -- Trap errors in header and rethrow -- Wrap underlying iterator in MalformatedBAMErrorReformattingIterator --- .../gatk/datasources/reads/SAMDataSource.java | 3 ++ .../MalformedBAMErrorReformatingIterator.java | 45 +++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 public/java/src/org/broadinstitute/sting/gatk/iterators/MalformedBAMErrorReformatingIterator.java diff --git a/public/java/src/org/broadinstitute/sting/gatk/datasources/reads/SAMDataSource.java b/public/java/src/org/broadinstitute/sting/gatk/datasources/reads/SAMDataSource.java index bba5c21e2..b215763b5 100755 --- a/public/java/src/org/broadinstitute/sting/gatk/datasources/reads/SAMDataSource.java +++ b/public/java/src/org/broadinstitute/sting/gatk/datasources/reads/SAMDataSource.java @@ -570,6 +570,7 @@ public class SAMDataSource { inputStream.submitAccessPlan(new SAMReaderPosition(id,inputStream,(GATKBAMFileSpan)shard.getFileSpans().get(id))); } iterator = readers.getReader(id).iterator(shard.getFileSpans().get(id)); + iterator = new MalformedBAMErrorReformatingIterator(id.samFile, iterator); if(shard.getGenomeLocs().size() > 0) iterator = new IntervalOverlapFilteringIterator(iterator,shard.getGenomeLocs()); mergingIterator.addIterator(readers.getReader(id),iterator); @@ -873,6 +874,8 @@ public class SAMDataSource { throw new UserException.CouldNotReadInputFile(readerID.samFile, e); else throw e; + } catch ( SAMFormatException e ) { + throw new UserException.MalformedBAM(readerID.samFile, e.getMessage()); } reader.setSAMRecordFactory(factory); reader.enableFileSource(true); diff --git a/public/java/src/org/broadinstitute/sting/gatk/iterators/MalformedBAMErrorReformatingIterator.java b/public/java/src/org/broadinstitute/sting/gatk/iterators/MalformedBAMErrorReformatingIterator.java new file mode 100644 index 000000000..f5dee4961 --- /dev/null +++ b/public/java/src/org/broadinstitute/sting/gatk/iterators/MalformedBAMErrorReformatingIterator.java @@ -0,0 +1,45 @@ +package org.broadinstitute.sting.gatk.iterators; + +import net.sf.samtools.SAMFormatException; +import net.sf.samtools.SAMRecord; +import net.sf.samtools.util.CloseableIterator; +import org.broadinstitute.sting.utils.exceptions.UserException; + +import java.io.File; +import java.util.Iterator; + +/** + * Traps BAM formatting errors in underlying iterator and rethrows meaningful GATK UserExceptions + */ +public class MalformedBAMErrorReformatingIterator implements CloseableIterator { + File source; + CloseableIterator it; + + public MalformedBAMErrorReformatingIterator(final File source, final CloseableIterator it) { + this.it = it; + this.source = source; + } + + public boolean hasNext() { + try { + return this.it.hasNext(); + } catch ( SAMFormatException e ) { + throw new UserException.MalformedBAM(source, e.getMessage()); + } + } + + public SAMRecord next() { + try { + return it.next(); + } catch ( SAMFormatException e ) { + throw new UserException.MalformedBAM(source, e.getMessage()); + } + } + + public void remove() { + throw new UnsupportedOperationException("Can not remove records from a SAM file via an iterator!"); + } + + public void close() { it.close(); } + public Iterator iterator() { return this; } +} From 4d9582de777f3f9c9543ed1102e6da7bee2c95e9 Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Mon, 27 Feb 2012 14:02:26 -0500 Subject: [PATCH 2/7] More general catching of Exceptions in interval reading to throw MalformedFile exception in all cases -- Now throws UserException no matter what happens during the reading of the intervals file. --- .../org/broadinstitute/sting/commandline/IntervalBinding.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/public/java/src/org/broadinstitute/sting/commandline/IntervalBinding.java b/public/java/src/org/broadinstitute/sting/commandline/IntervalBinding.java index 9e2c9a818..0c6096e0c 100644 --- a/public/java/src/org/broadinstitute/sting/commandline/IntervalBinding.java +++ b/public/java/src/org/broadinstitute/sting/commandline/IntervalBinding.java @@ -98,8 +98,8 @@ public final class IntervalBinding { intervals.add(toolkit.getGenomeLocParser().createGenomeLoc(feature)); line = lineReader.readLine(); } - } catch (IOException e) { - throw new UserException("Problem reading the interval file " + featureIntervals.getSource() + "; " + e.getMessage()); + } catch (Exception e) { + throw new UserException.MalformedFile(featureIntervals.getSource(), "Problem reading the interval file", e); } } else { From 729bb954e29512c95956e4ec957a451245bd2b47 Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Mon, 27 Feb 2012 15:09:00 -0500 Subject: [PATCH 3/7] Throws ReviewedStingException for a bug when parent VariantContext argument is null --- .../sting/utils/variantcontext/VariantContextBuilder.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/public/java/src/org/broadinstitute/sting/utils/variantcontext/VariantContextBuilder.java b/public/java/src/org/broadinstitute/sting/utils/variantcontext/VariantContextBuilder.java index b79584df8..8a3f22017 100644 --- a/public/java/src/org/broadinstitute/sting/utils/variantcontext/VariantContextBuilder.java +++ b/public/java/src/org/broadinstitute/sting/utils/variantcontext/VariantContextBuilder.java @@ -29,6 +29,7 @@ import org.broad.tribble.Feature; import org.broad.tribble.TribbleException; import org.broad.tribble.util.ParsingUtils; import org.broadinstitute.sting.utils.codecs.vcf.VCFConstants; +import org.broadinstitute.sting.utils.exceptions.ReviewedStingException; import java.util.*; @@ -102,9 +103,10 @@ public class VariantContextBuilder { * Returns a new builder based on parent -- the new VC will have all fields initialized * to their corresponding values in parent. This is the best way to create a derived VariantContext * - * @param parent + * @param parent Cannot be null */ public VariantContextBuilder(VariantContext parent) { + if ( parent == null ) throw new ReviewedStingException("BUG: VariantContext parent argument cannot be null in VariantContextBuilder") this.alleles = parent.alleles; this.attributes = parent.getAttributes(); this.attributesCanBeModified = false; From 5f7ccdcc01c289b7d73c134762fb4a36bb28d060 Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Mon, 27 Feb 2012 15:12:25 -0500 Subject: [PATCH 4/7] Avoid calling getBasePileup when there's no pileup in NBaseCount annotation --- .../sting/gatk/walkers/annotator/NBaseCount.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/public/java/src/org/broadinstitute/sting/gatk/walkers/annotator/NBaseCount.java b/public/java/src/org/broadinstitute/sting/gatk/walkers/annotator/NBaseCount.java index 891e9ae56..acd3b9e35 100755 --- a/public/java/src/org/broadinstitute/sting/gatk/walkers/annotator/NBaseCount.java +++ b/public/java/src/org/broadinstitute/sting/gatk/walkers/annotator/NBaseCount.java @@ -27,13 +27,15 @@ public class NBaseCount extends InfoFieldAnnotation { int countNBaseSolid = 0; int countRegularBaseSolid = 0; - for( final Map.Entry sample : stratifiedContexts.entrySet() ) { - for( final PileupElement p : sample.getValue().getBasePileup()) { - if( p.getRead().getReadGroup().getPlatform().toUpperCase().contains("SOLID") ) { - if( BaseUtils.isNBase( p.getBase() ) ) { - countNBaseSolid++; - } else if( BaseUtils.isRegularBase( p.getBase() ) ) { - countRegularBaseSolid++; + for( final AlignmentContext context : stratifiedContexts.values() ) { + if ( context.hasBasePileup() ) { // must be called as getBasePileup may throw error when pileup has no bases + for( final PileupElement p : context.getBasePileup()) { + if( p.getRead().getReadGroup().getPlatform().toUpperCase().contains("SOLID") ) { + if( BaseUtils.isNBase( p.getBase() ) ) { + countNBaseSolid++; + } else if( BaseUtils.isRegularBase( p.getBase() ) ) { + countRegularBaseSolid++; + } } } } From 100ddef930d44bb270eada443dc3f3f6eea0c252 Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Mon, 27 Feb 2012 17:06:45 -0500 Subject: [PATCH 5/7] Fix typo in VariantContextBuilder --- .../sting/utils/variantcontext/VariantContextBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/java/src/org/broadinstitute/sting/utils/variantcontext/VariantContextBuilder.java b/public/java/src/org/broadinstitute/sting/utils/variantcontext/VariantContextBuilder.java index 8a3f22017..4e16db482 100644 --- a/public/java/src/org/broadinstitute/sting/utils/variantcontext/VariantContextBuilder.java +++ b/public/java/src/org/broadinstitute/sting/utils/variantcontext/VariantContextBuilder.java @@ -106,7 +106,7 @@ public class VariantContextBuilder { * @param parent Cannot be null */ public VariantContextBuilder(VariantContext parent) { - if ( parent == null ) throw new ReviewedStingException("BUG: VariantContext parent argument cannot be null in VariantContextBuilder") + if ( parent == null ) throw new ReviewedStingException("BUG: VariantContext parent argument cannot be null in VariantContextBuilder"); this.alleles = parent.alleles; this.attributes = parent.getAttributes(); this.attributesCanBeModified = false; From f9e8e82e3335a8bdb68e38217d2ae9c527b0dad1 Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Mon, 27 Feb 2012 17:07:19 -0500 Subject: [PATCH 6/7] Removed unused class variable from VCFHeaderLineTranslator --- .../sting/utils/codecs/vcf/VCFHeaderLineTranslator.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/public/java/src/org/broadinstitute/sting/utils/codecs/vcf/VCFHeaderLineTranslator.java b/public/java/src/org/broadinstitute/sting/utils/codecs/vcf/VCFHeaderLineTranslator.java index 16f2a3d5a..e39a09cb1 100755 --- a/public/java/src/org/broadinstitute/sting/utils/codecs/vcf/VCFHeaderLineTranslator.java +++ b/public/java/src/org/broadinstitute/sting/utils/codecs/vcf/VCFHeaderLineTranslator.java @@ -31,8 +31,6 @@ interface VCFLineParser { * a class that handles the to and from disk for VCF 4 lines */ class VCF4Parser implements VCFLineParser { - Set bracketed = new HashSet(); - /** * parse a VCF4 line * @param valueLine the line From 0b29d549375f84cbb150dcec7706a3355faa9167 Mon Sep 17 00:00:00 2001 From: Mark DePristo Date: Mon, 27 Feb 2012 17:08:41 -0500 Subject: [PATCH 7/7] Changed most BAMSchedule ReviewedStingExceptions to UserExceptions -- As these represent the bulk of the StingExceptions coming from BAMSchedule and are caused by simple problems like the user providing bad input tmp directories, etc. --- .../gatk/datasources/reads/BAMSchedule.java | 34 +++++++++++++++---- .../sting/utils/exceptions/UserException.java | 4 +++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/public/java/src/org/broadinstitute/sting/gatk/datasources/reads/BAMSchedule.java b/public/java/src/org/broadinstitute/sting/gatk/datasources/reads/BAMSchedule.java index 762722fcd..657c70aaa 100644 --- a/public/java/src/org/broadinstitute/sting/gatk/datasources/reads/BAMSchedule.java +++ b/public/java/src/org/broadinstitute/sting/gatk/datasources/reads/BAMSchedule.java @@ -31,6 +31,7 @@ import net.sf.samtools.GATKChunk; import net.sf.samtools.util.CloseableIterator; import org.broadinstitute.sting.utils.GenomeLoc; import org.broadinstitute.sting.utils.exceptions.ReviewedStingException; +import org.broadinstitute.sting.utils.exceptions.StingException; import org.broadinstitute.sting.utils.exceptions.UserException; import java.io.File; @@ -193,7 +194,28 @@ public class BAMSchedule implements CloseableIterator { scheduleFileChannel.close(); } catch(IOException ex) { - throw new ReviewedStingException("Unable to close schedule file."); + throw makeIOFailureException(true, "Unable to close schedule file.", ex); + } + } + + /** + * Convenience routine for creating UserExceptions + * @param wasWriting + * @param message + * @param e + * @return + */ + private final StingException makeIOFailureException(final boolean wasWriting, final String message, final Exception e) { + if ( wasWriting ) { + if ( e == null ) + return new UserException.CouldNotCreateOutputFile(scheduleFile, message); + else + return new UserException.CouldNotCreateOutputFile(scheduleFile, message, e); + } else { + if ( e == null ) + return new UserException.CouldNotReadInputFile(scheduleFile, message); + else + return new UserException.CouldNotReadInputFile(scheduleFile, message, e); } } @@ -297,7 +319,7 @@ public class BAMSchedule implements CloseableIterator { return scheduleFileChannel.read(buffer); } catch(IOException ex) { - throw new ReviewedStingException("Unable to read data from BAM schedule file..",ex); + throw makeIOFailureException(false, "Unable to read data from BAM schedule file.", ex); } } @@ -305,10 +327,10 @@ public class BAMSchedule implements CloseableIterator { try { scheduleFileChannel.write(buffer); if(buffer.remaining() > 0) - throw new ReviewedStingException("Unable to write entire buffer to file."); + throw makeIOFailureException(true, "Unable to write entire buffer to file.", null); } catch(IOException ex) { - throw new ReviewedStingException("Unable to write data to BAM schedule file.",ex); + throw makeIOFailureException(true, "Unable to write data to BAM schedule file.", ex); } } @@ -321,7 +343,7 @@ public class BAMSchedule implements CloseableIterator { return scheduleFileChannel.position(); } catch(IOException ex) { - throw new ReviewedStingException("Unable to retrieve position of BAM schedule file.",ex); + throw makeIOFailureException(false, "Unable to retrieve position of BAM schedule file.", ex); } } @@ -334,7 +356,7 @@ public class BAMSchedule implements CloseableIterator { scheduleFileChannel.position(position); } catch(IOException ex) { - throw new ReviewedStingException("Unable to position BAM schedule file.",ex); + throw makeIOFailureException(false, "Unable to position BAM schedule file.",ex); } } diff --git a/public/java/src/org/broadinstitute/sting/utils/exceptions/UserException.java b/public/java/src/org/broadinstitute/sting/utils/exceptions/UserException.java index a2816b58f..2ece3b077 100755 --- a/public/java/src/org/broadinstitute/sting/utils/exceptions/UserException.java +++ b/public/java/src/org/broadinstitute/sting/utils/exceptions/UserException.java @@ -140,6 +140,10 @@ public class UserException extends ReviewedStingException { super(String.format("Couldn't write file %s because %s with exception %s", file.getAbsolutePath(), message, e.getMessage())); } + public CouldNotCreateOutputFile(File file, String message) { + super(String.format("Couldn't write file %s because %s", file.getAbsolutePath(), message)); + } + public CouldNotCreateOutputFile(String filename, String message, Exception e) { super(String.format("Couldn't write file %s because %s with exception %s", filename, message, e.getMessage())); }