Do not ignore secondary failure

When another exception is thrown while handling another exception, that
exception can be attached to the original exception since Java 7
(Throwable#getSuppressed). Attach the secondary exception to the
original exception instead of throwing it away.

Change-Id: Ia093b8207714f2638e0343bc45a83d4342947505
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>
This commit is contained in:
Masaya Suzuki 2019-03-04 19:01:39 -08:00 committed by Terry Parker
parent 08d2d107ee
commit ca360ea2b5
1 changed files with 22 additions and 31 deletions

View File

@ -1017,27 +1017,23 @@ else if (requestValidator instanceof AnyRequestValidator)
if (!err.isOutput() && err.getMessage() != null) { if (!err.isOutput() && err.getMessage() != null) {
try { try {
pckOut.writeString("ERR " + err.getMessage() + "\n"); //$NON-NLS-1$ //$NON-NLS-2$ pckOut.writeString("ERR " + err.getMessage() + "\n"); //$NON-NLS-1$ //$NON-NLS-2$
err.setOutput(); } catch (IOException e) {
} catch (Throwable err2) { err.addSuppressed(e);
// Ignore this secondary failure (and not mark output). throw err;
} }
err.setOutput();
} }
throw err; throw err;
} catch (IOException | RuntimeException | Error err) { } catch (IOException | RuntimeException | Error err) {
boolean output = false; String msg = err instanceof PackProtocolException ? err.getMessage()
try {
String msg = err instanceof PackProtocolException
? err.getMessage()
: JGitText.get().internalServerError; : JGitText.get().internalServerError;
try {
pckOut.writeString("ERR " + msg + "\n"); //$NON-NLS-1$ //$NON-NLS-2$ pckOut.writeString("ERR " + msg + "\n"); //$NON-NLS-1$ //$NON-NLS-2$
output = true; } catch (IOException e) {
} catch (Throwable err2) { err.addSuppressed(e);
// Ignore this secondary failure, leave output false.
}
if (output) {
throw new UploadPackInternalServerErrorException(err);
}
throw err; throw err;
}
throw new UploadPackInternalServerErrorException(err);
} finally { } finally {
if (!sendPack && !biDirectionalPipe) { if (!sendPack && !biDirectionalPipe) {
while (0 < rawIn.skip(2048) || 0 <= rawIn.read()) { while (0 < rawIn.skip(2048) || 0 <= rawIn.read()) {
@ -2077,31 +2073,26 @@ private void sendPack(PackStatistics.Accumulator accumulator,
// This was already reported on (below). // This was already reported on (below).
throw noPack; throw noPack;
} catch (IOException | RuntimeException | Error err) { } catch (IOException | RuntimeException | Error err) {
if (reportInternalServerErrorOverSideband()) { try {
throw new UploadPackInternalServerErrorException(err); reportInternalServerErrorOverSideband();
} else { } catch (IOException e) {
err.addSuppressed(e);
throw err; throw err;
} }
throw new UploadPackInternalServerErrorException(err);
} }
} else { } else {
sendPack(false, req, accumulator, allTags, unshallowCommits, deepenNots); sendPack(false, req, accumulator, allTags, unshallowCommits, deepenNots);
} }
} }
private boolean reportInternalServerErrorOverSideband() { private void reportInternalServerErrorOverSideband() throws IOException {
try {
@SuppressWarnings("resource" /* java 7 */) @SuppressWarnings("resource" /* java 7 */)
SideBandOutputStream err = new SideBandOutputStream( SideBandOutputStream err = new SideBandOutputStream(
SideBandOutputStream.CH_ERROR, SideBandOutputStream.CH_ERROR, SideBandOutputStream.SMALL_BUF,
SideBandOutputStream.SMALL_BUF,
rawOut); rawOut);
err.write(Constants.encode(JGitText.get().internalServerError)); err.write(Constants.encode(JGitText.get().internalServerError));
err.flush(); err.flush();
return true;
} catch (Throwable cannotReport) {
// Ignore the reason. This is a secondary failure.
return false;
}
} }
/** /**