From: Gergő Tisza <tgr.huwiki@gmail.com>
Date: Mon, 23 Oct 2017 08:29:20 +0000 (-0700)
Subject: MediaWiki\Shell: log stderr
X-Git-Tag: 1.31.0-rc.0~1659^2
X-Git-Url: http://git.heureux-cyclage.org/?a=commitdiff_plain;h=7d9dbc0040034e1dbe97c959d37d96c8ca400aa5;p=lhc%2Fweb%2Fwiklou.git

MediaWiki\Shell: log stderr

Change-Id: I1495fe2aba10102d7e36c3a3e5fdabf97f14546b
---

diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index 75ce8eca94..0496b67fc7 100644
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -442,6 +442,7 @@ return [
 
 		$factory = new CommandFactory( $limits, $cgroup );
 		$factory->setLogger( LoggerFactory::getInstance( 'exec' ) );
+		$factory->logStderr();
 
 		return $factory;
 	},
diff --git a/includes/shell/Command.php b/includes/shell/Command.php
index 1816c5aed1..9f080d5141 100644
--- a/includes/shell/Command.php
+++ b/includes/shell/Command.php
@@ -57,7 +57,10 @@ class Command {
 	private $method;
 
 	/** @var bool */
-	private $useStderr = false;
+	private $doIncludeStderr = false;
+
+	/** @var bool */
+	private $doLogStderr = false;
 
 	/** @var bool */
 	private $everExecuted = false;
@@ -180,7 +183,19 @@ class Command {
 	 * @return $this
 	 */
 	public function includeStderr( $yesno = true ) {
-		$this->useStderr = $yesno;
+		$this->doIncludeStderr = $yesno;
+
+		return $this;
+	}
+
+	/**
+	 * When enabled, text sent to stderr will be logged with a level of 'error'.
+	 *
+	 * @param bool $yesno
+	 * @return $this
+	 */
+	public function logStderr( $yesno = true ) {
+		$this->doLogStderr = $yesno;
 
 		return $this;
 	}
@@ -235,7 +250,7 @@ class Command {
 				$cmd = '/bin/bash ' . escapeshellarg( __DIR__ . '/limit.sh' ) . ' ' .
 					escapeshellarg( $cmd ) . ' ' .
 					escapeshellarg(
-						"MW_INCLUDE_STDERR=" . ( $this->useStderr ? '1' : '' ) . ';' .
+						"MW_INCLUDE_STDERR=" . ( $this->doIncludeStderr ? '1' : '' ) . ';' .
 						"MW_CPU_LIMIT=$time; " .
 						'MW_CGROUP=' . escapeshellarg( $this->cgroup ) . '; ' .
 						"MW_MEM_LIMIT=$mem; " .
@@ -246,7 +261,7 @@ class Command {
 				$useLogPipe = true;
 			}
 		}
-		if ( !$useLogPipe && $this->useStderr ) {
+		if ( !$useLogPipe && $this->doIncludeStderr ) {
 			$cmd .= ' 2>&1';
 		}
 
@@ -424,6 +439,15 @@ class Command {
 			$this->logger->warning( "$logMsg: {command}", [ 'command' => $cmd ] );
 		}
 
+		if ( $errBuffer && $this->doLogStderr ) {
+			$this->logger->error( "Error running {command}: {error}", [
+				'command' => $cmd,
+				'error' => $errBuffer,
+				'exitcode' => $retval,
+				'exception' => new Exception( 'Shell error' ),
+			] );
+		}
+
 		return new Result( $retval, $outBuffer, $errBuffer );
 	}
 }
diff --git a/includes/shell/CommandFactory.php b/includes/shell/CommandFactory.php
index c0b8f899ed..84dd50f71f 100644
--- a/includes/shell/CommandFactory.php
+++ b/includes/shell/CommandFactory.php
@@ -37,6 +37,9 @@ class CommandFactory {
 	/** @var string|bool */
 	private $cgroup;
 
+	/** @var bool */
+	private $doLogStderr = false;
+
 	/**
 	 * Constructor
 	 *
@@ -49,6 +52,16 @@ class CommandFactory {
 		$this->setLogger( new NullLogger() );
 	}
 
+	/**
+	 * When enabled, text sent to stderr will be logged with a level of 'error'.
+	 *
+	 * @param bool $yesno
+	 * @see Command::logStderr
+	 */
+	public function logStderr( $yesno = true ) {
+		$this->doLogStderr = $yesno;
+	}
+
 	/**
 	 * Instantiates a new Command
 	 *
@@ -60,6 +73,7 @@ class CommandFactory {
 
 		return $command
 			->limits( $this->limits )
-			->cgroup( $this->cgroup );
+			->cgroup( $this->cgroup )
+			->logStderr( $this->doLogStderr );
 	}
 }
diff --git a/tests/phpunit/includes/shell/CommandFactoryTest.php b/tests/phpunit/includes/shell/CommandFactoryTest.php
index aacfd43c16..f90e8377ca 100644
--- a/tests/phpunit/includes/shell/CommandFactoryTest.php
+++ b/tests/phpunit/includes/shell/CommandFactoryTest.php
@@ -23,11 +23,13 @@ class CommandFactoryTest extends PHPUnit_Framework_TestCase {
 
 		$factory = new CommandFactory( $limits, $cgroup );
 		$factory->setLogger( $logger );
+		$factory->logStderr();
 		$command = $factory->create();
 
 		$wrapper = TestingAccessWrapper::newFromObject( $command );
 		$this->assertSame( $logger, $wrapper->logger );
 		$this->assertSame( $cgroup, $wrapper->cgroup );
 		$this->assertSame( $limits, $wrapper->limits );
+		$this->assertTrue( $wrapper->doLogStderr );
 	}
 }
diff --git a/tests/phpunit/includes/shell/CommandTest.php b/tests/phpunit/includes/shell/CommandTest.php
index 32d855ea39..81fae33c84 100644
--- a/tests/phpunit/includes/shell/CommandTest.php
+++ b/tests/phpunit/includes/shell/CommandTest.php
@@ -118,4 +118,25 @@ class CommandTest extends PHPUnit_Framework_TestCase {
 			$this->assertEquals( 333333, strlen( $output ) );
 		}
 	}
+
+	public function testLogStderr() {
+		$this->requirePosix();
+
+		$logger = new TestLogger( true, function ( $message, $level, $context ) {
+			return $level === Psr\Log\LogLevel::ERROR ? '1' : null;
+		}, true );
+		$command = new Command();
+		$command->setLogger( $logger );
+		$command->params( 'bash', '-c', 'echo ThisIsStderr 1>&2' );
+		$command->execute();
+		$this->assertEmpty( $logger->getBuffer() );
+
+		$command = new Command();
+		$command->setLogger( $logger );
+		$command->logStderr();
+		$command->params( 'bash', '-c', 'echo ThisIsStderr 1>&2' );
+		$command->execute();
+		$this->assertSame( 1, count( $logger->getBuffer() ) );
+		$this->assertSame( trim( $logger->getBuffer()[0][2]['error'] ), 'ThisIsStderr' );
+	}
 }