Allow error, fatal error and exception handlers to be enabled separately.

[mfb]

Some apps have their own built-in error handlers that we want to use to send errors to Sentry, while we may want to use Sentry's built-in fatal error shutdown handler and/or exception handler. In these cases we want to enable/disable the various Sentry error handlers separately.

It looks like at present in 2.x branch we would have to create our own error handler integration, which should be totally doable, but also seems like a waste :)

[stayallive]

I believe this is already possible:

$client = ClientBuilder::create([
	'dsn' => 'xxx',
	'default_integrations' => false,
])->getClient()

This would disable the default integrations (which includes the error handler) and you could supply the additional default integrations in the integrations key if you do want the other ones.

For context:

sentry-php/src/ClientBuilder.php

Lines 95 to 113 in bf9184f

	public function __construct(Options $options = null)
	{
	$this->options = $options ?? new Options();
	if ($this->options->hasDefaultIntegrations()) {
	$this->options->setIntegrations(array_merge([
	new ErrorHandlerIntegration(),
	new RequestIntegration($this->options),
	], $this->options->getIntegrations()));
	}
	}
	/**
	* {@inheritdoc}
	*/
	public static function create(array $options = []): ClientBuilderInterface
	{
	return new static(new Options($options));
	}

[mfb]

I need to enable only the fatal error shutdown handler, or only the fatal error shutdown handler and the exception handler, and but the upgrade docs say:

The methods Raven_ErrorHandler::registerErrorHandler, Raven_ErrorHandler::registerExceptionHandler and Raven_ErrorHandler::registerShutdownFunction have been removed. You should use the ErrorHandler::register method instead, but note that it registers all error handlers (error, exception and fatal error) at once and there is no way anymore to only use one of them.

[stayallive]

Yep, I missed you meant that, sorry about that.

Let's see if we can possibly split them out via either configuration or something else, I actually like this use case but also understand why they were merged... might be a compromise made somehwere. Thanks for bringing this up!

[Jean85]

This would be possible with #762: in that PR I've splitted the integrations into two separated listeners, you would need to disable the default and add only the error listener, not the exceptions one.

[mfb]

@Jean85 I don't think that helps - I want to use Sentry's fatal error shutdown handler, but not the error handler. The app already has its own error handler that I need to use instead of Sentry's.

[ste93cry]

The app already has its own error handler that I need to use instead of Sentry's.

Just asking: since Sentry's error handler should be as transparent as possible it should work with other handlers too, it will catch errors and then just forward them to the handler that was registered previously. Doesn't it works for you?

[mfb]

No, because it modifies the stacktrace.

[ste93cry]

It should not edit the stacktrace as far as I remember. By "it modified the stacktrace" do you mean that you don't want the Sentry's frames to be visible in the stacktrace or that the data of the stacktrace is modified? If that's the case can you please post a little script that reproduces the scenario?

[mfb]

<?php

use function Sentry\init;
use function Sentry\captureException;

require('vendor/autoload.php');

set_error_handler(function () {
  $backtrace = debug_backtrace();
  while ($backtrace && !isset($backtrace[0]['line'])) {
    array_shift($backtrace);
  }
  // The first trace is the call itself.
  // It gives us the line and the file of the last call.
  $call = $backtrace[0];
  // The second call give us the function where the call originated.
  if (isset($backtrace[1])) {
    if (isset($backtrace[1]['class'])) {
      $call['function'] = $backtrace[1]['class'] . $backtrace[1]['type'] . $backtrace[1]['function'] . '()';
    }
    else {
      $call['function'] = $backtrace[1]['function'] . '()';
    }
  }
  else {
    $call['function'] = 'main()';
  }
  error_log("Error in {$call['function']} ({$call['file']})");
});

init(['dsn' => 'http://123@example.com/123' ]);

$foo = $bar[1];

Basically, the app figures out the last caller and prints it in the logs as well as any displayed error message. And is already deployed to literally a million websites :D

So the easiest work-around for me will be to make my own custom integration to send fatal errors to Sentry. However, I was thinking it would be nice if Sentry 2.x let me register just the fatal error handler, thus I created this issue.

[Jean85]

I've quickly tested your case, and I've seen that if you register Sentry first, and your handle later, the backtrace is not changed.

I'm not sure if we can fix the behavior in the opposite case, but at least you have a workaround.

[mfb]

Right, but that's not possible because the app's error handler is setup before it loads 3rd party libraries. So what I do on Sentry 1.x is plug into the app's error handler rather than PHP's error handler (while still using Sentry's fatal error handler).

[Jean85]

Unfortunately it seems that there's no other way around it. We already do everything that's possible by calling the previous error handler and passing the error forward.

[mfb]

Would allowing the ErrorHandler class to be extended be a possibility? That way I wouldn't have to duplicate code, I could just have a child class with a different constructor.

[Jean85]

We declared a lot of classes final to avoid maintainability issues, like in the past. If we open up such class to extension, we erode our freedom to fix stuff up in the future, to avoid BC.

[mfb]

Ok. I think it could make sense to break these out into separate classes then, at some point. Assuming I'm not the only person who wants to customize handler registration.

[ste93cry]

The linked PR should fix the issue by allowing each handler to be registered separately from the others. @mfb it would be great if you could take a look at it and give me feedback