Bug - Fixed Problem fetching fax configuration files.

fronobulax

Developer
Staff member
I believe this is a a side effect of r19219.

I seem to recall first seeing it a couple months ago.

I have two characters who use the fax daily. I started seeing Premature end of file errors on one of the fax configuration files, a lot, and almost never with the first character to use the fax. I am reasonably certain I have tracked it down to a logic error. I am perfectly capable of coming up with a kludge to address the issue but my solutions seem to lack elegance. So I am posting in hopes veracity (author of r19219) wants to look at it.

Referenced code is in FileUtilities which was reworked.

The first time the fax configurations are updated, the remote versions are usually newer than the local versions so they are downloaded.

But the second time, it is often the case that the remote version has not changed. Nevertheless the old version is replaced with a zero byte file and processing that generates the Premature EOF message.

The problem is that line 426

Code:
OutputStream ostream = DataUtilities.getOutputStream( local );

will overwrite an existing file with a zero byte file.

But after that happens, line 428 is executed

Code:
downloadFileToStream( remote, connection, ostream );
.

downloadFileToStream makes a connection. If the response code is anything other than 200 or a triggered exception, the remote file is never read (into ostream) and thus the associated disk file remains empty. The error is repeatable any time the response code is 304 which essentially means the cached copy is not older than the remote copy.

My inclination would be to fix this by deferring the creation of the output stream until I was sure the local copy was invalid. But changing the signature to pass a file name instead of a stream effects relay file code. Copying downloadFileToStream and renaming a copy with a different signature seems inelegant and potentially harder to maintain. There is probably a way to refactor downloadFileToStream but I am not seeing it easily, so I will post here.

I will do something about this in a day or two but it won't be pretty :)
 

Veracity

Developer
Staff member
Since you are blaming my revision, I’ll take a look. ��

I want to cut a new point release this weekend after I release the “beach” command, but it sounds like this needs to be taken care of too.
 

fronobulax

Developer
Staff member
Since you are blaming my revision, I’ll take a look. ��

I want to cut a new point release this weekend after I release the “beach” command, but it sounds like this needs to be taken care of too.

I am treading on thin ice but I am as comfortable as I can be, given my history of jumping to the wrong conclusion, in asking you about it since the specific lines were last edited by you :) I'll be glad to fix it but it might be a better fix wth your input.
 

Veracity

Developer
Staff member
Revision 19450 fixes this.

In FileUtilities, we had:

private static void downloadFileToStream( final String remote, final HttpURLConnection connection, final OutputStream ostream )

I changed it to:

private static void downloadFileToStream( final String remote, final InputStream istream, final OutputStream ostream )

and created:

private static InputStream getInputStreamFromConnection( final String remote, final HttpURLConnection connection )

which will return null if the file is Not Modified.

I made both of the places which called downloadFileToStream first call getInputStreamFromConnection and give up if it returns null.
Otherwise, they create the output stream and proceed as before.

I tested thusly:

A character opens the "Send a Fax" frame
This fetches fax configs from EasyFax and cheesefax
Both faxbots display their list of offerings

Quit KoLmafia

Same character logs in and opens the "Send a Fax" frame
As we see in the debug log, both config fetches return Not Modified.
Both faxbots display their list of offerings
 

fronobulax

Developer
Staff member
Thank you. I will get a chance to try/verify later although I have little doubt that it will work.
 
Top