UploadPack: Introduce ErrorWriter

ErrorWriter writes an error message to the user. The implementation is
swapped once it detects that the client supports sideband. By default it
uses the protocol level ERR packet, which was introduced recently.

In total the error output is done in two different places;
UploadPack#upload and UploadPack#sendPack. These will be consolidated in
the next change.

Change-Id: Ia8d72e31170bbeafc8ffa8ddb92702196af8a587
Signed-off-by: Masaya Suzuki <masayasuzuki@google.com>

1 files changed, 35 insertions, 17 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
index 20b45882df..b664cfe7bc 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/UploadPack.java
@@ -44,6 +44,7 @@
 package org.eclipse.jgit.transport;
 
 import static java.util.Collections.unmodifiableMap;
+import static java.util.Objects.requireNonNull;
 import static org.eclipse.jgit.lib.Constants.R_TAGS;
 import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_REF_IN_WANT;
 import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_SERVER_OPTION;
@@ -282,6 +283,8 @@ public class UploadPack {
 
 	private OutputStream msgOut = NullOutputStream.INSTANCE;
 
+	private ErrorWriter errOut = new PackProtocolErrorWriter();
+
 	/**
 	 * Refs eligible for advertising to the client, set using
 	 * {@link #setAdvertisedRefs}.
@@ -812,9 +815,9 @@ public class UploadPack {
 			// nothing.
 			throw err;
 		} catch (ServiceMayNotContinueException err) {
-			if (!err.isOutput() && err.getMessage() != null && pckOut != null) {
+			if (!err.isOutput() && err.getMessage() != null) {
 				try {
-					pckOut.writeString("ERR " + err.getMessage() + "\n"); //$NON-NLS-1$ //$NON-NLS-2$
+					errOut.writeError(err.getMessage());
 				} catch (IOException e) {
 					err.addSuppressed(e);
 					throw err;
@@ -823,12 +826,12 @@ public class UploadPack {
 			}
 			throw err;
 		} catch (IOException | RuntimeException | Error err) {
-			if (pckOut != null) {
+			if (rawOut != null) {
 				String msg = err instanceof PackProtocolException
 						? err.getMessage()
 						: JGitText.get().internalServerError;
 				try {
-					pckOut.writeString("ERR " + msg + "\n"); //$NON-NLS-1$ //$NON-NLS-2$
+					errOut.writeError(msg);
 				} catch (IOException e) {
 					err.addSuppressed(e);
 					throw err;
@@ -2115,6 +2118,8 @@ public class UploadPack {
 		boolean sideband = caps.contains(OPTION_SIDE_BAND)
 				|| caps.contains(OPTION_SIDE_BAND_64K);
 		if (sideband) {
+			errOut = new SideBandErrorWriter();
+
 			try {
 				sendPack(true, req, accumulator, allTags, unshallowCommits,
 						deepenNots, pckOut);
@@ -2124,7 +2129,7 @@ public class UploadPack {
 					message = JGitText.get().internalServerError;
 				}
 				try {
-					reportInternalServerErrorOverSideband(message);
+					errOut.writeError(message);
 				} catch (IOException e) {
 					err.addSuppressed(e);
 					throw err;
@@ -2132,8 +2137,7 @@ public class UploadPack {
 				throw new UploadPackInternalServerErrorException(err);
 			} catch (IOException | RuntimeException | Error err) {
 				try {
-					reportInternalServerErrorOverSideband(
-							JGitText.get().internalServerError);
+					errOut.writeError(JGitText.get().internalServerError);
 				} catch (IOException e) {
 					err.addSuppressed(e);
 					throw err;
@@ -2146,16 +2150,6 @@ public class UploadPack {
 		}
 	}
 
-	private void reportInternalServerErrorOverSideband(String message)
-			throws IOException {
-		@SuppressWarnings("resource" /* java 7 */)
-		SideBandOutputStream err = new SideBandOutputStream(
-				SideBandOutputStream.CH_ERROR, SideBandOutputStream.SMALL_BUF,
-				rawOut);
-		err.write(Constants.encode(message));
-		err.flush();
-	}
-
 	/**
 	 * Send the requested objects to the client.
 	 *
@@ -2411,4 +2405,28 @@ public class UploadPack {
 			}
 		}
 	}
+
+	private interface ErrorWriter {
+		void writeError(String message) throws IOException;
+	}
+
+	private class SideBandErrorWriter implements ErrorWriter {
+		@Override
+		public void writeError(String message) throws IOException {
+			@SuppressWarnings("resource" /* java 7 */)
+			SideBandOutputStream err = new SideBandOutputStream(
+					SideBandOutputStream.CH_ERROR,
+					SideBandOutputStream.SMALL_BUF, requireNonNull(rawOut));
+			err.write(Constants.encode(message));
+			err.flush();
+		}
+	}
+
+	private class PackProtocolErrorWriter implements ErrorWriter {
+		@Override
+		public void writeError(String message) throws IOException {
+			new PacketLineOut(requireNonNull(rawOut))
+					.writeString("ERR " + message + '\n'); //$NON-NLS-1$
+		}
+	}
 }