--- a/sat/core/exceptions.py	Tue Jan 22 18:52:16 2019 +0100
+++ b/sat/core/exceptions.py	Fri Jan 25 09:06:29 2019 +0100
@@ -126,3 +126,15 @@
 class InvalidCertificate(Exception):
     """A TLS certificate is not valid"""
     pass
+
+
+class CommandException(RuntimeError):
+    """An external command failed
+
+    stdout and stderr will be attached to the Exception
+    """
+
+    def __init__(self, msg, stdout, stderr):
+        super(CommandException, self).__init__(msg)
+        self.stdout = stdout
+        self.stderr = stderr

--- a/sat/plugins/plugin_merge_req_mercurial.py	Tue Jan 22 18:52:16 2019 +0100
+++ b/sat/plugins/plugin_merge_req_mercurial.py	Fri Jan 25 09:06:29 2019 +0100
@@ -17,13 +17,13 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import re
+from twisted.python.procutils import which
+from sat.tools.common import async_process
+from sat.tools import utils
 from sat.core.i18n import _, D_
 from sat.core.constants import Const as C
 from sat.core import exceptions
-from twisted.internet import reactor, defer, protocol
-from twisted.python.failure import Failure
-from twisted.python.procutils import which
-import re
 from sat.core.log import getLogger
 log = getLogger(__name__)
 
@@ -42,66 +42,25 @@
 CLEAN_RE = re.compile(ur'[^\w -._]', flags=re.UNICODE)
 
 
-class MercurialProtocol(protocol.ProcessProtocol):
+class MercurialProtocol(async_process.CommandProtocol):
     """handle hg commands"""
-    hg = None
-
-    def __init__(self, deferred, stdin=None):
-        """
-        @param deferred(defer.Deferred): will be called when command is completed
-        @param stdin(str, None): if not None, will be push to standard input
-        """
-        self._stdin = stdin
-        self._deferred = deferred
-        self.data = []
-
-    def connectionMade(self):
-        if self._stdin is not None:
-            self.transport.write(self._stdin)
-            self.transport.closeStdin()
-
-    def outReceived(self, data):
-        self.data.append(data)
-
-    def errReceived(self, data):
-        self.data.append(data)
-
-    def processEnded(self, reason):
-        data = u''.join([d.decode('utf-8') for d in self.data])
-        if (reason.value.exitCode == 0):
-            log.debug(_('Mercurial command succeed'))
-            self._deferred.callback(data)
-        else:
-            msg = (_(u"Can't complete Mercurial command (error code: {code}): {message}")
-                   .format(code = reason.value.exitCode, message = data))
-            log.warning(msg)
-            self._deferred.errback(Failure(RuntimeError(msg)))
+    name = u"Mercurial"
+    command = None
 
     @classmethod
     def run(cls, path, command, *args, **kwargs):
         """Create a new MercurialRegisterProtocol and execute the given mercurial command.
 
         @param path(unicode): path to the repository
-        @param command(unicode): command to run
-        @param *args(unicode): command arguments
-        @param **kwargs: used because Python2 doesn't handle normal kw args after *args
-            can only be:
-            - stdin(unicode, None): data to push to standard input
-        @return ((D)):
+        @param command(unicode): hg command to run
         """
-        stdin = kwargs.pop('stdin', None)
-        if kwargs:
-            raise exceptions.InternalError(u'only stdin is allowed as keyword argument')
-        if stdin is not None:
-            stdin = stdin.encode('utf-8')
-        d = defer.Deferred()
-        mercurial_prot = MercurialProtocol(d, stdin=stdin)
-        cmd_args = [cls.hg, command.encode('utf-8')]
-        cmd_args.extend([a.encode('utf-8') for a in args])
-        reactor.spawnProcess(mercurial_prot,
-                             cls.hg,
-                             cmd_args,
-                             path=path.encode('utf-8'))
+        assert u"path" not in kwargs
+        kwargs["path"] = path
+        # FIXME: we have to use this workaround because Twisted's protocol.ProcessProtocol
+        #        is not using new style classes. This can be removed once moved to
+        #        Python 3 (super can be used normally then).
+        d = async_process.CommandProtocol.run.__func__(cls, command, *args, **kwargs)
+        d.addErrback(utils.logError)
         return d
 
 
@@ -111,7 +70,7 @@
     def __init__(self, host):
         log.info(_(u"Mercurial merge request handler initialization"))
         try:
-            MercurialProtocol.hg = which('hg')[0]
+            MercurialProtocol.command = which('hg')[0]
         except IndexError:
             raise exceptions.NotFound(_(u"Mercurial executable (hg) not found, "
                                         u"can't use Mercurial handler"))
@@ -119,6 +78,7 @@
         self._m = host.plugins['MERGE_REQUESTS']
         self._m.register('mercurial', self, self.data_types, SHORT_DESC)
 
+
     def check(self, repository):
         d = MercurialProtocol.run(repository, 'identify')
         d.addCallback(lambda __: True)

--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/sat/tools/common/async_process.py	Fri Jan 25 09:06:29 2019 +0100
@@ -0,0 +1,141 @@
+#!/usr/bin/env python2
+# -*- coding: utf-8 -*-
+
+# SAT: a jabber client
+# Copyright (C) 2009-2019 Jérôme Poisson (goffi@goffi.org)
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""tools to launch process in a async way (using Twisted)"""
+
+import os.path
+from twisted.internet import defer, reactor, protocol
+from twisted.python.failure import Failure
+from sat.core.i18n import _
+from sat.core import exceptions
+from sat.core.log import getLogger
+log = getLogger(__name__)
+
+
+class CommandProtocol(protocol.ProcessProtocol):
+    """handle an external command"""
+    # name of the command (unicode)
+    name = None
+    # full path to the command (bytes)
+    command = None
+    # True to activate logging of command outputs (bool)
+    log = False
+
+    def __init__(self, deferred, stdin=None):
+        """
+        @param deferred(defer.Deferred): will be called when command is completed
+        @param stdin(str, None): if not None, will be push to standard input
+        """
+        self._stdin = stdin
+        self._deferred = deferred
+        self.data = []
+        self.err_data = []
+
+    @property
+    def command_name(self):
+        """returns command name or empty string if it can't be guessed"""
+        if self.name is not None:
+            return self.name
+        elif self.command is not None:
+            return os.path.splitext(os.path.basename(self.command))[0].decode('utf-8',
+                                                                              'ignore')
+        else:
+            return u''
+
+    def connectionMade(self):
+        if self._stdin is not None:
+            self.transport.write(self._stdin)
+            self.transport.closeStdin()
+
+    def outReceived(self, data):
+        if self.log:
+            log.info(data.decode('utf-8', 'replace'))
+        self.data.append(data)
+
+    def errReceived(self, data):
+        if self.log:
+            log.warning(data.decode('utf-8', 'replace'))
+        self.err_data.append(data)
+
+    def processEnded(self, reason):
+        data = ''.join(self.data)
+        if (reason.value.exitCode == 0):
+            log.debug(_(u'{name} command succeed').format(name=self.command_name))
+            # we don't use "replace" on purpose, we want an exception if decoding
+            # is not working properly
+            self._deferred.callback(data.encode('utf-8'))
+        else:
+            err_data = u''.join(self.err_data)
+
+            msg = (_(u"Can't complete {name} command (error code: {code}):\n"
+                     u"stderr:\n{stderr}\n{stdout}\n")
+                   .format(name = self.command_name,
+                           code = reason.value.exitCode,
+                           stderr= err_data.encode('utf-8', 'replace'),
+                           stdout = "stdout: " + data.encode('utf-8', 'replace')
+                                    if data else u'',
+                           ))
+            self._deferred.errback(Failure(exceptions.CommandException(
+                msg, data, err_data)))
+
+    @classmethod
+    def run(cls, *args, **kwargs):
+        """Create a new CommandProtocol and execute the given command.
+
+        @param *args(unicode): command arguments
+            if cls.command is specified, it will be the path to the command to execture
+            otherwise, first argument must be the path
+        @param **kwargs: can be:
+            - stdin(unicode, None): data to push to standard input
+            - verbose(bool): if True stdout and stderr will be logged
+            other keyword arguments will be used in reactor.spawnProcess
+        @return ((D)): stdout in case of success
+        @raise RuntimeError: command returned a non zero status
+            stdin and stdout will be given as arguments
+
+        """
+        stdin = kwargs.pop('stdin', None)
+        if stdin is not None:
+            stdin = stdin.encode('utf-8')
+        verbose = kwargs.pop('verbose', False)
+        if u'path' in kwargs:
+            kwargs[u'path'] = kwargs[u'path'].encode('utf-8')
+        args = [a.encode('utf-8') for a in args]
+        kwargs = {k:v.encode('utf-8') for k,v in kwargs.items()}
+        d = defer.Deferred()
+        prot = cls(d, stdin=stdin)
+        if verbose:
+            prot.log = True
+        if cls.command is None:
+            if not args:
+                raise ValueError(
+                    u"You must either specify cls.command or use a full path to command "
+                    u"to execute as first argument")
+            command = args.pop(0)
+            if prot.name is None:
+                name = os.path.splitext(os.path.basename(command))[0]
+                prot.name = name.encode(u'utf-8', u'ignore')
+        else:
+            command = cls.command
+        cmd_args = [os.path.basename(command)] + args
+        reactor.spawnProcess(prot,
+                             command,
+                             cmd_args,
+                             **kwargs)
+        return d

--- a/sat/tools/utils.py	Tue Jan 22 18:52:16 2019 +0100
+++ b/sat/tools/utils.py	Fri Jan 25 09:06:29 2019 +0100
@@ -21,12 +21,7 @@
 
 import unicodedata
 import os.path
-from sat.core.constants import Const as C
-from sat.core.log import getLogger
-
-log = getLogger(__name__)
 import datetime
-from twisted.python import procutils
 import subprocess
 import time
 import sys
@@ -34,6 +29,11 @@
 import inspect
 import textwrap
 import functools
+from twisted.python import procutils
+from sat.core.constants import Const as C
+from sat.core.log import getLogger
+
+log = getLogger(__name__)
 
 
 NO_REPOS_DATA = u"repository data unknown"
@@ -56,6 +56,12 @@
     return "".join(valid_chars(ustr))
 
 
+def logError(failure_):
+    """Genertic errback which log the error as a warning, and re-raise it"""
+    log.warning(failure_.value)
+    raise failure_
+
+
 def partial(func, *fixed_args, **fixed_kwargs):
     # FIXME: temporary hack to workaround the fact that inspect.getargspec is not working with functools.partial
     #        making partial unusable with current D-bus module (in addMethod).