RavenHandler: check if exception capturing succeeded, and send a normal message if it has not

[zerkms]

At the moment the RavenHandler capturing messages is implemented like this:

        if (isset($record['context']['exception']) && ($record['context']['exception'] instanceof \Exception || (PHP_VERSION_ID >= 70000 && $record['context']['exception'] instanceof \Throwable))) {
            $options['extra']['message'] = $record['formatted'];
            $this->ravenClient->captureException($record['context']['exception'], $options);
        } else {
            $this->ravenClient->captureMessage($record['formatted'], array(), $options);
        }

basically, if it's an exception - try to capture it, otherwise capture it as a message.

So far so good.

Sentry API accepts data json-encoded, so sentry client JSON-encodes everything that is to be sent.

A serialised exception contains the complete stack trace including arguments.

The problem is that those arguments sometimes are strings with binary data, which is more often than not is not valid UTF8, hence cannot be json-encoded.

If you open the official sentry client you'd find this line somewhere near Client.php:1047

$message = $this->encode($data);

If $data is exception details, then it would simply return false and set $this->_lasterror as something like malformed utf-8.

Then the data is attempted to be sent with $this->send_remote($this->server, $message, $headers); few lines below it.

There is unfortunately no way to detect if the SentryClient::send_remote completed unsuccessfully due to how they designed their API.

But in the monolog RavenHandler we could at least try our best, and here is how:

If right before $this->ravenClient->captureException($record['context']['exception'], $options); we

$lastSentryError = $this->ravenClient->getLastSentryError();

and then after $this->ravenClient->captureException(...) check

if ($lastSentryError !== $this->ravenClient->getLastSentryError()) { ... }

we then are 100% confident the data for some reason was not successfully sent.

What I propose is that - in case if it was not sent, we would run

$this->ravenClient->captureMessage($record['formatted'], array(), $options);

as a fallback.

Incomplete error (just a message won't contain the stack trace) is still better than no error, since at least you're informed. Additionally we could add something like [extras][monolog] = Could not serialise exception, an incomplete error.

But at least it would be something.

Thoughts?

[Seldaek]

Sounds good to me. More reliability is definitely good. Missing exceptions entirely sucks.

[zerkms]

Awesome, I will allocate some time to prototype it after I return from a parental leave

[zerkms]

I have found that there might be a better solution, using a concept of raven message processors:

1. Define a processor that normalises and fixes the broken utf8:

final class Utf8NormalizerProcessor extends \Raven_Processor
{
    /**
     * @var \Raven_Serializer
     */
    private $serializer;

    public function __construct(\Raven_Client $client)
    {
        parent::__construct($client);

        $this->serializer = new \Raven_Serializer($client->mb_detect_order, $client->message_limit);
    }

    public function process(&$data)
    {
        foreach ($data as $key => $value) {
            if (is_string($value)) {
                $data[$key] = $this->serializer->serialize($value);
            } elseif (is_array($value)) {
                $data[$key] = $this->process($value);
            }
        }

        return $data;
    }
}

2. Enable it together with default (and possible other processors, if any):

$ravenClient = new \Raven_Client($dsn, [
    // ... other options omitted
    'processors' => array_merge(
        \Raven_Client::getDefaultProcessors(),
        [Utf8NormalizerProcessor::class]
    ),
]);

I will send a suggestion to the raven-php project a bit later, but at the moment I think this solution is much more clear than what I initially suggested here above.