yf0994 / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

Add more detail to ByteStreams.readFully error message #1111

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Consider having ByteStreams#readFully overloads throw a specialized 
EOFException which contains the #bytes read before EOF.

My use case:
I'm reading messages of known length. If I can only read a partial message 
before EOF I want to debug-level log the partial message as a UTF8 string. I 
cannot do this today using the readFully overloads because they don't tell me 
how many bytes were actually read.

The workaround is simple; use the #read method instead. But I wanted to suggest 
it anyway because if I could use #readFully methods I would.

Original issue reported on code.google.com by brianfromoregon on 16 Aug 2012 at 5:07

GoogleCodeExporter commented 9 years ago
And yes, I do realize that the timing of my filing such a minor suggestion is 
indicative of a ploy to claim ownership of issue #1111. Rest assured it was 
entirely coincidental.

Original comment by brianfromoregon on 16 Aug 2012 at 5:20

GoogleCodeExporter commented 9 years ago
That said, there's no denying that I'm on my way.

1. Lay claim to issue #1111
2. ???
3. World domination!

Original comment by brianfromoregon on 16 Aug 2012 at 5:23

GoogleCodeExporter commented 9 years ago
Is this an accurate before-and-after picture?

int read = in.read(bytes);
if (read != bytes.length) {
  throw new CorruptedFileException(copyOrRange(bytes, 0, read));
}

try {
  ByteStreams.readFully(in, bytes);
} catch (InsufficientDataRemainingException e) {
  throw new CorruptedFileException(e.getReadBytes());
}

Attaching data to exceptions is probably an underused technique in general, so 
I kind of like that aspect.  It will be tough to justify a new public class 
just for this, though.

Original comment by cpov...@google.com on 16 Aug 2012 at 5:39

GoogleCodeExporter commented 9 years ago
Note that the aforementioned simple workaround is wrong: There's no guarantee 
that the read method doesn't give up before filling the buffer without hitting 
EOF. At least that's my understanding of `InputStream.read(byte[])`.

I had a similar problem and wrote an int-returning method 
"readAsMuchAsPossible". Note that using it, `readFully` (and also 
`InsufficientDataRemainingException`) can trivially be implemented, but not the 
other way round.

Original comment by Maaarti...@gmail.com on 17 Aug 2012 at 9:28

GoogleCodeExporter commented 9 years ago
Yeah, I had been worried about that.  But then I saw that the implementation of 
ByteStreams.readFully is:

    if (read(in, b, off, len) != len) {
      throw new EOFException();
    }

I expected to see something more like skipFully's loop.

I'm not sure how to interpret the documentation of InputStream.read, but it 
certainly seems open to your interpretation: 
http://docs.oracle.com/javase/6/docs/api/java/io/InputStream.html#read%28byte[]%
29

Original comment by cpov...@google.com on 17 Aug 2012 at 9:37

GoogleCodeExporter commented 9 years ago
I agree with Maaartinus' interpretation, actually.  I...think that might be a 
bug?

Original comment by wasserman.louis on 17 Aug 2012 at 9:49

GoogleCodeExporter commented 9 years ago
No bugs here, both the workaround I mentioned and the readFully impl are 
referencing ByteStreams#read not InputStream#read. ByteStreams#read calls 
InputStream#read in a loop.

Original comment by brianfromoregon on 17 Aug 2012 at 11:27

GoogleCodeExporter commented 9 years ago
Oops.  Thanks for the correction.

Original comment by cpov...@google.com on 18 Aug 2012 at 3:51

GoogleCodeExporter commented 9 years ago
We're going to add some more detail to the error message, but if you care about 
this, you should really be using read, not readFully.

The reasoning is that if your application is supposed to do something 
reasonable in the presence of partial results -- if you can tolerate input that 
doesn't fill up the byte array -- then you really shouldn't be using readFully 
in the first place; you should be using read.

Original comment by lowas...@google.com on 23 Oct 2012 at 4:28

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:13

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:18

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:08