ExceptionHandler - Code Metrics - Inspection of "Some minor bugfixes for newinternal" - enwikipedia-acc/waca - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — newinternal (#285)

by Simon

created 2017-02-18 21:51 UTC

ExceptionHandler A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	112
Duplicated Lines	0 %

Coupling/Cohesion

Components	0
Dependencies	0

Test Coverage

Coverage

Importance

Changes

Metric	Value
dl	0
loc	112
ccs	0
cts	65
cp	0
rs	10
c	0
b	0
f	0
wmc	7
lcom	0
cbo	0

3 Methods

Rating	Name	Size	Complexity
B	exceptionHandler()	66	4
A	errorHandler()	5	1
A	getExceptionData()	13	2

<?php
/******************************************************************************
 * Wikipedia Account Creation Assistance tool                                 *
 *                                                                            *
 * All code in this file is released into the public domain by the ACC        *
 * Development Team. Please see team.json for a list of contributors.         *
 ******************************************************************************/

namespace Waca;

use ErrorException;
use Exception;

class ExceptionHandler
{
    /**
     * Global exception handler
     *
     * Smarty would be nice to use, but it COULD BE smarty that throws the errors.
     * Let's build something ourselves, and hope it works.
     *
     * @param $exception
     *
     * @category Security-Critical - has the potential to leak data when exception is thrown.
     */
    public static function exceptionHandler(Exception $exception)
// Bad
class Router
{
    public function generate($path)
    {
        return $_SERVER['HOST'].$path;
    }
}

// Better
class Router
{
    private $host;

    public function __construct($host)
    {
        $this->host = $host;
    }

    public function generate($path)
    {
        return $this->host.$path;
    }
}

class Controller
{
    public function myAction(Request $request)
    {
        // Instead of
        $page = isset($_GET['page']) ? intval($_GET['page']) : 1;

        // Better (assuming you use the Symfony2 request)
        $page = $request->query->get('page', 1);
    }
}
    {
        /** @global $siteConfiguration SiteConfiguration */
        global $siteConfiguration;
function myFunction($a, $b) {
    // Do something
}

        $errorDocument = <<<HTML
<!DOCTYPE html>
<html lang="en"><head>
<meta charset="utf-8">
<title>Oops! Something went wrong!</title>
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<link href="{$siteConfiguration->getBaseUrl()}/lib/bootstrap/css/bootstrap.min.css" rel="stylesheet">
<style>
  body {
    padding-top: 60px;
  }
</style>
<link href="{$siteConfiguration->getBaseUrl()}/lib/bootstrap/css/bootstrap-responsive.min.css" rel="stylesheet">
</head><body><div class="container">
<h1>Oops! Something went wrong!</h1>
<p>We'll work on fixing this for you, so why not come back later?</p>
<p class="muted">If our trained monkeys ask, tell them this error ID: <code>$1$</code></p>
$2$
</div></body></html>
HTML;

        $errorData = self::getExceptionData($exception);
        $errorData['server'] = $_SERVER;
        $errorData['get'] = $_GET;
        $errorData['post'] = $_POST;

        $state = serialize($errorData);
        $errorId = sha1($state);

        // Save the error for later analysis
        file_put_contents($siteConfiguration->getErrorLog() . '/' . $errorId . '.log', $state);

        // clear and discard any content that's been saved to the output buffer
        if (ob_get_level() > 0) {
            ob_end_clean();
        }

        // push error ID into the document.
        $message = str_replace('$1$', $errorId, $errorDocument);

        if ($siteConfiguration->getDebuggingTraceEnabled()) {
            ob_start();
            var_dump($errorData);

            $textErrorData = ob_get_contents();
            ob_end_clean();

            $message = str_replace('$2$', $textErrorData, $message);
        }
        else {
            $message = str_replace('$2$', "", $message);
        }

        // While we *shouldn't* have sent headers by now due to the output buffering, PHPUnit does weird things.
        // This is "only" needed for the tests, but it's a good idea to wrap this anyway.
        if (!headers_sent()) {
            header('HTTP/1.1 500 Internal Server Error');
        }

        // output the document
        print $message;
    }

    /**
     * @param int    $errorSeverity The severity level of the exception.
     * @param string $errorMessage  The Exception message to throw.
     * @param string $errorFile     The filename where the exception is thrown.
     * @param int    $errorLine     The line number where the exception is thrown.
     *
     * @throws ErrorException
     */
    public static function errorHandler($errorSeverity, $errorMessage, $errorFile, $errorLine)
    {
        // call into the main exception handler above
        throw new ErrorException($errorMessage, 0, $errorSeverity, $errorFile, $errorLine);
    }

    /**
     * @param Exception $exception
     *
     * @return null|array
     */
    private static function getExceptionData($exception)
    {
        if ($exception == null) {
            return null;
        }

        return array(
            'exception' => get_class($exception),
            'message'   => $exception->getMessage(),
            'stack'     => $exception->getTraceAsString(),
            'previous'  => self::getExceptionData($exception->getPrevious()),
        );
    }
}

Pull Request — newinternal (#285)

ExceptionHandler A

Complexity

Size/Duplication

Coupling/Cohesion

Test Coverage

Importance

3 Methods

1. Pass all data via parameters

2. Create a class that maintains your state

1			<?php
2			/******************************************************************************
3			* Wikipedia Account Creation Assistance tool *
4			* *
5			* All code in this file is released into the public domain by the ACC *
6			* Development Team. Please see team.json for a list of contributors. *
7			******************************************************************************/
8
9			namespace Waca;
10
11			use ErrorException;
12			use Exception;
13
14			class ExceptionHandler
15			{
16			/**
17			* Global exception handler
18			*
19			* Smarty would be nice to use, but it COULD BE smarty that throws the errors.
20			* Let's build something ourselves, and hope it works.
21			*
22			* @param $exception
23			*
24			* @category Security-Critical - has the potential to leak data when exception is thrown.
25			*/
26			public static function exceptionHandler(Exception $exception)
			0 ignored issues – show Coding Style introduced 2016-03-26 16:48 UTC by Report Bug Copy Issue Report `exceptionHandler` uses the super-global variable `$_SERVER` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history... Coding Style introduced 2016-03-26 16:48 UTC by Report Bug Copy Issue Report `exceptionHandler` uses the super-global variable `$_GET` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history... Coding Style introduced 2016-03-26 16:48 UTC by Report Bug Copy Issue Report `exceptionHandler` uses the super-global variable `$_POST` which is generally not recommended. Instead of super-globals, we recommend to explicitly inject the dependencies of your class. This makes your code less dependent on global state and it becomes generally more testable: // Bad class Router { public function generate($path) { return $_SERVER['HOST'].$path; } } // Better class Router { private $host; public function __construct($host) { $this->host = $host; } public function generate($path) { return $this->host.$path; } } class Controller { public function myAction(Request $request) { // Instead of $page = isset($_GET['page']) ? intval($_GET['page']) : 1; // Better (assuming you use the Symfony2 request) $page = $request->query->get('page', 1); } } Loading history... Coding Style Best Practice introduced 2016-03-26 16:48 UTC by Report Bug Copy Issue Report Please use `__construct()` instead of a PHP4-style constructor that is named after the class. Loading history...
27			{
28			/** @global $siteConfiguration SiteConfiguration */
29			global $siteConfiguration;
			0 ignored issues – show Compatibility Best Practice introduced 2016-03-26 16:48 UTC by Report Bug Copy Issue Report Use of `global` functionality is not recommended; it makes your code harder to test, and less reusable. Instead of relying on `global` state, we recommend one of these alternatives: 1. Pass all data via parameters function myFunction($a, $b) { // Do something } 2. Create a class that maintains your state class MyClass { private $a; private $b; public function __construct($a, $b) { $this->a = $a; $this->b = $b; } public function myFunction() { // Do something } } Loading history...
30
31			$errorDocument = <<<HTML
32			<!DOCTYPE html>
33			<html lang="en"><head>
34			<meta charset="utf-8">
35			<title>Oops! Something went wrong!</title>
36			<meta name="viewport" content="width=device-width, initial-scale=1.0">
37			<link href="{$siteConfiguration->getBaseUrl()}/lib/bootstrap/css/bootstrap.min.css" rel="stylesheet">
38			<style>
39			body {
40			padding-top: 60px;
41			}
42			</style>
43			<link href="{$siteConfiguration->getBaseUrl()}/lib/bootstrap/css/bootstrap-responsive.min.css" rel="stylesheet">
44			</head><body><div class="container">
45			<h1>Oops! Something went wrong!</h1>
46			<p>We'll work on fixing this for you, so why not come back later?</p>
47			<p class="muted">If our trained monkeys ask, tell them this error ID: <code>$1$</code></p>
48			$2$
49			</div></body></html>
50			HTML;
51
52			$errorData = self::getExceptionData($exception);
53			$errorData['server'] = $_SERVER;
54			$errorData['get'] = $_GET;
55			$errorData['post'] = $_POST;
56
57			$state = serialize($errorData);
58			$errorId = sha1($state);
59
60			// Save the error for later analysis
61			file_put_contents($siteConfiguration->getErrorLog() . '/' . $errorId . '.log', $state);
62
63			// clear and discard any content that's been saved to the output buffer
64			if (ob_get_level() > 0) {
65			ob_end_clean();
66			}
67
68			// push error ID into the document.
69			$message = str_replace('$1$', $errorId, $errorDocument);
70
71			if ($siteConfiguration->getDebuggingTraceEnabled()) {
72			ob_start();
73			var_dump($errorData);
			0 ignored issues – show Security Debugging Code introduced 2016-03-26 16:48 UTC by Report Bug Copy Issue Report `var_dump($errorData);` looks like debug code. Are you sure you do not want to remove it? This might expose sensitive data. Loading history...
74			$textErrorData = ob_get_contents();
75			ob_end_clean();
76
77			$message = str_replace('$2$', $textErrorData, $message);
78			}
79			else {
80			$message = str_replace('$2$', "", $message);
81			}
82
83			// While we shouldn't have sent headers by now due to the output buffering, PHPUnit does weird things.
84			// This is "only" needed for the tests, but it's a good idea to wrap this anyway.
85			if (!headers_sent()) {
86			header('HTTP/1.1 500 Internal Server Error');
87			}
88
89			// output the document
90			print $message;
91			}
92
93			/**
94			* @param int $errorSeverity The severity level of the exception.
95			* @param string $errorMessage The Exception message to throw.
96			* @param string $errorFile The filename where the exception is thrown.
97			* @param int $errorLine The line number where the exception is thrown.
98			*
99			* @throws ErrorException
100			*/
101			public static function errorHandler($errorSeverity, $errorMessage, $errorFile, $errorLine)
102			{
103			// call into the main exception handler above
104			throw new ErrorException($errorMessage, 0, $errorSeverity, $errorFile, $errorLine);
105			}
106
107			/**
108			* @param Exception $exception
109			*
110			* @return null\|array
111			*/
112			private static function getExceptionData($exception)
113			{
114			if ($exception == null) {
115			return null;
116			}
117
118			return array(
119			'exception' => get_class($exception),
120			'message' => $exception->getMessage(),
121			'stack' => $exception->getTraceAsString(),
122			'previous' => self::getExceptionData($exception->getPrevious()),
123			);
124			}
125			}

enwikipedia-acc / waca

Pull Request — newinternal (#285)

ExceptionHandler A

Complexity

Size/Duplication

Coupling/Cohesion

Test Coverage

Importance

3 Methods

1. Pass all data via parameters

2. Create a class that maintains your state

Duplication Side-by-Side

Filter issues like