Do not capture E_ERROR type as fatal error

[stayallive]

This should fix #408, can you guys please test it and and let me know if this is the way we should handle this or an alternative.

IMO this gist is that we should ignore E_ERROR exceptions as fatal in PHP 7.0+ since they can be captured by userland and are thus going through the regular exception handler before entering the fatal error handler thus creating duplicated exceptions.

This PR as a side effect also unifies the way the PHP version is checked.

[Jean85]

I'll test this ASAP!

Maybe we can add a thing to this PR: update the PHPDoc of the Client::captureException() method to accept \Throwable|\Exception $exception here:

sentry-php/lib/Raven/Client.php

Lines 561 to 570 in 0baca06

	/**
	* Log an exception to sentry
	*
	* @param Exception $exception The Exception object.
	* @param array $data Additional attributes to pass with this event (see Sentry docs).
	* @param mixed $logger
	* @param mixed $vars
	* @return string|null
	*/
	public function captureException($exception, $data = null, $logger = null, $vars = null)

What do you think?

[Jean85]

Also: maybe we should add a (unit) test?

[stayallive]

I think that's a good idea.

For reference: why not remove \Throwable (this was my first thought)? This is because \Throwable was introduced in PHP 7.0 so \Exception remains for PHP < 7.0.

[stayallive]

@Jean85 I changed the test to check if the output of the shouldCaptureFatalError is correct. But not actually checking if this works correctly, any idea on how to test this? Cannot just call an undefined method since that does not seem to trigger this (tried adding a failing test before).

[Jean85]

Apart from my single comment, I tested this and it works! Great! 👍

[Jean85]

Maybe if (PHP_VERSION_ID >= 70000) is faster? That constant is available since PHP 5.2, so that's not an issue to use it.

[stayallive]

Looks like my PhpStorm inspections thought the same way. I changed it everywhere to stay consistent (updating the tests as we speak).

[Jean85]

Sorry, I missed the changed test, I think it's enough for now!

[mfb]

What are you all doing to capture fatal PHP errors, such as allowed memory size exhausted? To capture this fatal error successfully, I had to comment out the return FALSE; in shouldCaptureFatalError():

if (PHP_VERSION_ID >= 70000 && $type === E_ERROR) {
  // return false;
}

sample code:

<?php
include '../vendor/autoload.php';
$client = new Raven_Client('...');
$client->install();
while (TRUE) {
  $a[] = 11111111111;
}