Fixed missing messages on exceptions during hook

scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/hooks/DefaultHookHandler.java

@@ -40,11 +40,13 @@ import sonia.scm.repository.spi.HookEventFacade;
 import sonia.scm.security.BearerToken;
 import sonia.scm.security.CipherUtil;
 
+import javax.annotation.Nonnull;
 import javax.inject.Inject;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.Socket;
+import java.util.ArrayList;
 import java.util.List;
 
 import static java.util.Collections.singletonList;
@@ -87,6 +89,9 @@ class DefaultHookHandler implements HookHandler {
 
   private Response handleHookRequest(Request request) {
     LOG.trace("process {} hook for node {}", request.getType(), request.getNode());
+
+    HgHookContextProvider context = hookContextProviderFactory.create(request.getRepositoryId(), request.getNode());
+
     try {
       if (!environment.isAcceptAble(request.getChallenge())) {
         LOG.warn("received hook with invalid challenge: {}", request.getChallenge());
@@ -96,7 +101,6 @@ class DefaultHookHandler implements HookHandler {
       authenticate(request);
       environment.setPending(request.getType() == RepositoryHookType.PRE_RECEIVE);
 
-      HgHookContextProvider context = hookContextProviderFactory.create(request.getRepositoryId(), request.getNode());
       hookEventFacade.handle(request.getRepositoryId()).fireHookEvent(request.getType(), context);
 
       return new Response(context.getHgMessageProvider().getMessages(), false);
@@ -108,7 +112,7 @@ class DefaultHookHandler implements HookHandler {
       return error("repository not found");
     } catch (Exception ex) {
       LOG.warn("unknown error on hook occurred", ex);
-      return error("unknown error");
+      return error(context, ex);
     } finally {
       environment.clearPendingState();
     }
@@ -122,13 +126,24 @@ class DefaultHookHandler implements HookHandler {
     subject.login(bearer);
   }
 
+  private Response error(HgHookContextProvider context, Exception ex) {
+    List<HgHookMessage> messages = new ArrayList<>(context.getHgMessageProvider().getMessages());
+    messages.add(createErrorMessage(ex.getMessage()));
+    return new Response(messages, true);
+  }
+
   private Response error(String message) {
     return new Response(
-      singletonList(new HgHookMessage(HgHookMessage.Severity.ERROR, message)),
+      singletonList(createErrorMessage(message)),
       true
     );
   }
 
+  @Nonnull
+  private HgHookMessage createErrorMessage(String message) {
+    return new HgHookMessage(HgHookMessage.Severity.ERROR, message);
+  }
+
   private void close() {
     try {
       socket.close();

scm-plugins/scm-hg-plugin/src/main/resources/sonia/scm/python/scmhooks.py

@@ -39,7 +39,10 @@ repositoryId = os.environ['SCM_REPOSITORY_ID']
 
 def print_messages(ui, messages):
   for message in messages:
-    msg = "%s: %s\n" % (message['severity'], message['message'])
+    msg = "[SCM]"
+    if message['severity'] == "ERROR":
+      msg += " Error"
+    msg += ": " + message['message'] + "\n"
     ui.warn(msg.encode('utf-8'))
 
 def fire_hook(ui, repo, hooktype, node):

scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/hooks/DefaultHookHandlerTest.java

@@ -47,7 +47,9 @@ import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.net.Socket;
+import java.util.List;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Collectors;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
@@ -89,8 +91,12 @@ class DefaultHookHandlerTest {
   }
 
   private void mockMessageProvider() {
+    mockMessageProvider(new HgHookMessageProvider());
+  }
+
+  private void mockMessageProvider(HgHookMessageProvider messageProvider) {
     when(hookContextProviderFactory.create("42", "abc")).thenReturn(contextProvider);
-    when(contextProvider.getHgMessageProvider()).thenReturn(new HgHookMessageProvider());
+    when(contextProvider.getHgMessageProvider()).thenReturn(messageProvider);
   }
 
   @AfterEach
@@ -131,19 +137,43 @@ class DefaultHookHandlerTest {
   }
 
   @Test
-  void shouldHandleAuthenticationFailure() throws IOException {
-    doThrow(IllegalStateException.class)
+  void shouldHandleUnknownFailure() throws IOException {
+    mockMessageProvider();
+
+    doThrow(new IllegalStateException("Something went wrong"))
       .when(hookEventFacade)
       .handle("42");
 
     DefaultHookHandler.Request request = createRequest(RepositoryHookType.POST_RECEIVE);
     DefaultHookHandler.Response response = send(request);
 
-    assertError(response, "unknown");
+    assertError(response, "Something went wrong");
   }
 
   @Test
-  void shouldHandleUnknownFailure() throws IOException {
+  void shouldSendMessagesOnException() throws IOException {
+    HgHookMessageProvider messageProvider = new HgHookMessageProvider();
+    messageProvider.sendMessage("Some note");
+    messageProvider.sendMessage("Some error");
+    mockMessageProvider(messageProvider);
+
+    doThrow(new IllegalStateException("Abort it"))
+      .when(hookEventFacade)
+      .handle("42");
+
+    DefaultHookHandler.Request request = createRequest(RepositoryHookType.POST_RECEIVE);
+    DefaultHookHandler.Response response = send(request);
+
+    List<String> received = response.getMessages()
+      .stream()
+      .map(HgHookMessage::getMessage)
+      .collect(Collectors.toList());
+
+    assertThat(received).containsExactly("Some note", "Some error", "Abort it");
+  }
+
+  @Test
+  void shouldHandleAuthenticationFailure() throws IOException {
     doThrow(AuthenticationException.class)
       .when(subject)
       .login(any(AuthenticationToken.class));