ImportFailure created in parse() but Main never sees it (get exit 3 instead of 2)

Hello,

I am between jobs, so I have all the time in the world to be confused by a 100-line program.

ledger-import reads a semicolon CSV and writes cleaned rows to a staging file. Nothing is allowed to stay in staging unless the whole file parses. My test file data/march.csv has a broken amount on line 4 (-12,5x). I expected import rejected: line 4: bad amount ... and exit code 2. What I get is this, with exit code 3:

SEVERE: staging problem: staging file build/staging.tmp closed with 2 uncommitted rows

The line number and the bad value appear nowhere. With a temporary System.err.println in the catch block of parse(), it does print, so the ImportFailure really is created. It just never arrives in Main. When I fix line 4 the import works fine.

My first suspects were the catch order in Main and the logging.properties setup. Swapping the two catch blocks changed nothing useful. The relevant parts:

// Importer.run()
StagingWriter writer = new StagingWriter(staging);
try {
    ...
    writer.add(parse(lineNo, line));
    ...
    writer.commit();
    return writer.count();
} finally {
    writer.close();
}

// StagingWriter.close()
out.close();
if (!committed && rows > 0) {
    Files.deleteIfExists(path);
    throw new IOException("staging file " + path + " closed with " + rows + " uncommitted rows");
}

Main catches ImportFailure first (exit 2), then IOException (exit 3).

Is there a possibility that the logging is not the problem at all? Where does my exception go?

The conversation continues 1

Follow the reasoning

brandt_p27 Jul Permalink
Your ImportFailure is not lost in logging. The finally block replaces it.

parse() throws ImportFailure. Then finally runs writer.close(). Rows are uncommitted, so close() throws IOException. A new exception thrown in finally wins. The ImportFailure is dropped, and Main only sees the IOException. That is why you get exit 3.

Fix it with try-with-resources. If the body throws, an exception from close() is added as suppressed and the original stays primary:
```
try (StagingWriter writer = new StagingWriter(staging)) {
    ...
    writer.commit();
    return writer.count();
}
```
StagingWriter must implement AutoCloseable. Eventually also move the "uncommitted" check out of close(). An abort path that just deletes the file is cleaner.

ImportFailure created in parse() but Main never sees it (get exit 3 instead of 2)

The conversation continues 1

Other sides of the same question

`String::from_utf8` gives the bytes back when it fails

except ValueError never runs around gather(return_exceptions=True), then TypeError on the ValueError

`result.map_error` on every line of a use chain