Hi Martin,
It is reasonable to assume creating an output stream may also throw
|IOException|. The code handles this situation but it doesn't re-throw
the exception because its contract states it returns null in this case.
The third option looks as the best solution. Any usages of the API are
still expected to handle |IOException|.
At the same time, there could be reasons why it's not the best choice. I
didn't look thoroughly into the code.
Regards,
Alexey
On 13/09/2022 10:53, Martin Desruisseaux wrote:
Hello all
The ImageIO class contains the following method (copying only relevant
parts). The key point to notice is that any IOException is assumed to
be a problem with the cache file, never with the main output stream:
/**
* …snip…
* @return an {@code ImageOutputStream}, or {@code null}.
* …snip…
* @exception IOException if a cache file is needed but cannot be created.
*/
public static ImageOutputStream createImageOutputStream(Object output)
throws IOException {
// …snip…
try {
return spi.createOutputStreamInstance(output,
usecache,
getCacheDirectory());
} catch (IOException e) {
throw new IIOException("Can't create cache file!", e);
}
// …snip…
return null;
}
In the particular case where the output is a java.io.File, the SPI
implementation is as below (augmented with a copy of the Javadoc from
public API). Note that the javadoc does not mention that
ImageOutputStreamSpi.createOutputStreamInstance(…) may return null.
/**
* …snip…
* @return an {@code ImageOutputStream} instance.
* …snip…
* @exception IOException if a cache file is needed but cannot be created.
*/
public ImageOutputStream createOutputStreamInstance(…snip…) throws
IOException {
// …snip…
try {
return new FileImageOutputStream((File)output);
} catch (Exception e) {
e.printStackTrace();
return null;
}
// …snip…
}
In case of failure to create the output stream, ImageIO.write(…)
throws an exception with the "Can't create an ImageOutputStream!"
message, but we don't know why. The cause (for example no space left
on device) is hidden in the System.err outputs. If we replace the
com.sun.imageio.spi.FileImageOutputStreamSpi implementation by our own
provider propagating the IOException instead of returning null, it is
slightly better but the exception is caught by
ImageIO.createImageOutputStream and rethrown with the misleading
"Can't create cache file!" message. The consequence is that when
writing millions of images (e.g. Earth Observation data) and the
process fails somewhere at ImageOutputStream creation time, it is
currently difficult to identify the cause.
Possible actions for improving the situation could be (from less
disruptive to most disruptive):
* Replace the e.printStackTrace() call by the use of logger.
* Remove the try … catch block in
ImageIO.createImageOutputStream(…), so if an SPI choose to throw
IOException (at the cost of violating current Image I/O contract),
ImageIO does not misleadingly said that it is a problem with the
cache.
* Modify ImageIO.createImageOutputStream(…) contract by saying that
IOException may be thrown because of failure to create the main
output stream, not only because of failure to create the cache
file. Adapt FileImageOutputStreamSpi implementation accordingly.
Is there any thought about what would be reasonable, what would be too
much disruption?
Regards,
Martin
