Skip to content

convert Runtime exceptions in the CRAM code into CRAMExceptions (also… #1526

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

convert Runtime exceptions in the CRAM code into CRAMExceptions (also… #1526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/main/java/htsjdk/samtools/cram/build/CRAMReferenceRegion.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import htsjdk.samtools.SAMFileHeader;
import htsjdk.samtools.SAMRecord;
import htsjdk.samtools.SAMSequenceRecord;
import htsjdk.samtools.cram.CRAMException;
import htsjdk.samtools.cram.ref.CRAMReferenceSource;
import htsjdk.samtools.cram.ref.ReferenceContext;

Expand Down Expand Up @@ -73,7 +74,7 @@ public byte[] getReferenceBases(final int referenceIndex) {
final SAMSequenceRecord sequence = samFileHeader.getSequence(referenceIndex);
referenceBases = referenceSource.getReferenceBases(sequence, true);
if (referenceBases == null) {
throw new IllegalArgumentException(
throw new CRAMException(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure what the original intent of CRAMException was, but starting with the CRAM rewrite I restricted it's use to internal error conditions, such as malformed input as reported in broadinstitute/picard#1624. Not all of the CRAM codebase is conforming yet, but I don't think this is a case where CRAMException makes sense, since this is a user error.

String.format(
"A reference must be supplied (reference sequence %s not found).",
sequence));
Expand All @@ -92,5 +93,4 @@ public void setEmbeddedReference(final byte[] embeddedReferenceBytes, final int
referenceBasesContextID = embeddedReferenceIndex;
referenceBases = embeddedReferenceBytes;
}

}
7 changes: 4 additions & 3 deletions src/main/java/htsjdk/samtools/cram/build/CompressionHeaderFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
*/
package htsjdk.samtools.cram.build;

import htsjdk.samtools.cram.CRAMException;
import htsjdk.samtools.cram.common.MutableInt;
import htsjdk.samtools.cram.compression.ExternalCompressor;
import htsjdk.samtools.cram.encoding.*;
Expand Down Expand Up @@ -422,7 +423,7 @@ private EncodingDetails buildEncodingForTag(final List<CRAMCompressionRecord> re
new ExternalByteArrayEncoding(tagID)).toEncodingDescriptor();
return details;
default:
throw new IllegalArgumentException("Unknown tag type: " + (char) type);
throw new CRAMException("Unknown tag type: " + (char) type);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This one does seem like an appropriate case for CRAMException.

}
}

Expand Down Expand Up @@ -468,9 +469,9 @@ static int getTagValueByteSize(final byte type, final Object value) {
if (value instanceof long[]) {
return 1 + 4 + ((long[]) value).length * 4;
}
throw new RuntimeException("Unknown tag array class: " + value.getClass());
throw new CRAMException("Unknown tag array class: " + value.getClass());
default:
throw new RuntimeException("Unknown tag type: " + (char) type);
throw new CRAMException("Unknown tag type: " + (char) type);
}
}
}
3 changes: 2 additions & 1 deletion src/main/java/htsjdk/samtools/cram/build/CramContainerIterator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package htsjdk.samtools.cram.build;

import htsjdk.samtools.SAMFileHeader;
import htsjdk.samtools.cram.CRAMException;
import htsjdk.samtools.cram.io.CountingInputStream;
import htsjdk.samtools.cram.structure.Container;
import htsjdk.samtools.cram.structure.CramHeader;
Expand Down Expand Up @@ -71,7 +72,7 @@ public Container next() {

@Override
public void remove() {
throw new RuntimeException("Read only iterator.");
throw new CRAMException("Read only iterator.");
}

public CramHeader getCramHeader() {
Expand Down
8 changes: 5 additions & 3 deletions src/main/java/htsjdk/samtools/cram/build/CramIO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@

import htsjdk.samtools.SAMFileHeader;
import htsjdk.samtools.SAMTextHeaderCodec;
import htsjdk.samtools.cram.CRAMException;
import htsjdk.samtools.cram.common.CramVersions;
import htsjdk.samtools.cram.common.CRAMVersion;
import htsjdk.samtools.cram.structure.*;
import htsjdk.samtools.util.FileExtensions;
import htsjdk.samtools.util.Log;
import htsjdk.samtools.util.RuntimeIOException;

import javax.security.auth.login.CredentialException;
import java.io.*;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
Expand Down Expand Up @@ -88,7 +90,7 @@ public static long writeCramEOF(final CRAMVersion cramVersion, final OutputStrea
throw new RuntimeIOException(e);
}

throw new IllegalArgumentException(String.format("Unrecognized CRAM version %s", cramVersion));
throw new CRAMException(String.format("Unrecognized CRAM version %s", cramVersion));
Copy link
Collaborator

Choose a reason for hiding this comment

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

This one is borderline, but I'd err on leaving it as IllegalArgumentException, or else RuntimeException.

}

/**
Expand Down Expand Up @@ -123,13 +125,13 @@ public static CramHeader readCramHeader(final InputStream inputStream) {
try {
for (final byte magicByte : CramHeader.MAGIC) {
if (magicByte != inputStream.read()) {
throw new RuntimeException("Input does not have a valid CRAM header.");
throw new CRAMException("Input does not have a valid CRAM header.");
}
}

final CRAMVersion cramVersion = new CRAMVersion(inputStream.read(), inputStream.read());
if (!CramVersions.isSupportedVersion(cramVersion)) {
throw new RuntimeException(String.format("CRAM version %s is not supported", cramVersion));
throw new CRAMException(String.format("CRAM version %s is not supported", cramVersion));
}

final CramHeader header = new CramHeader(cramVersion, null);
Expand Down
7 changes: 4 additions & 3 deletions src/main/java/htsjdk/samtools/cram/build/CramSpanContainerIterator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package htsjdk.samtools.cram.build;

import htsjdk.samtools.cram.CRAMException;
import htsjdk.samtools.cram.structure.Container;
import htsjdk.samtools.seekablestream.SeekableStream;
import htsjdk.samtools.util.RuntimeIOException;
Expand All @@ -19,7 +20,7 @@ public final class CramSpanContainerIterator extends CramContainerIterator {
private Iterator<Boundary> containerBoundaries;
private Boundary currentBoundary;

private CramSpanContainerIterator(final SeekableStream seekableStream, final long[] coordinates) throws IOException {
private CramSpanContainerIterator(final SeekableStream seekableStream, final long[] coordinates) throws CRAMException {
super(seekableStream);
this.seekableStream = seekableStream;
final List<Boundary> boundaries = new ArrayList<>();
Expand Down Expand Up @@ -70,7 +71,7 @@ public Boundary(final long start, final long end) {
this.start = start;
this.end = end;
if (start >= end) {
throw new RuntimeException("Boundary start is greater than end.");
throw new CRAMException("Boundary start is greater than end.");
}
}

Expand All @@ -91,7 +92,7 @@ public Container next() {
}

if (!hasNext()) {
throw new RuntimeException("No more containers in this boundary.");
throw new CRAMException("No more containers in this boundary.");
}

return new Container(getCramHeader().getCRAMVersion(), seekableStream, seekableStream.position());
Expand Down