From b6c5a253cbbe851bccde16ceffd16910e0537d52 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Thu, 19 Nov 2020 09:26:36 +0100 Subject: [PATCH] Fixed missing messages on exceptions during hook --- .../repository/hooks/DefaultHookHandler.java | 21 ++++++++-- .../resources/sonia/scm/python/scmhooks.py | 5 ++- .../hooks/DefaultHookHandlerTest.java | 40 ++++++++++++++++--- 3 files changed, 57 insertions(+), 9 deletions(-) diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/hooks/DefaultHookHandler.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/hooks/DefaultHookHandler.java index b7ebefca3b..2b2e4132c2 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/hooks/DefaultHookHandler.java +++ b/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 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(); diff --git a/scm-plugins/scm-hg-plugin/src/main/resources/sonia/scm/python/scmhooks.py b/scm-plugins/scm-hg-plugin/src/main/resources/sonia/scm/python/scmhooks.py index 8e89bb1f06..72c5f7b528 100644 --- a/scm-plugins/scm-hg-plugin/src/main/resources/sonia/scm/python/scmhooks.py +++ b/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): diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/hooks/DefaultHookHandlerTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/hooks/DefaultHookHandlerTest.java index 45a9bc5aa5..0a430727d6 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/hooks/DefaultHookHandlerTest.java +++ b/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 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));