bugsnag / bugsnag-php

BugSnag error monitoring and crash reporting tool for PHP apps
https://docs.bugsnag.com/platforms/php
MIT License
553 stars 110 forks source link

Yii2: Logs are not flushed on shutdown #650

Open timkelty opened 2 years ago

timkelty commented 2 years ago

Describe the bug

Using Bugsnag with https://github.com/yiisoft/yii2, logged errors are not flushed to Bugsnag with batching enabled.

By settings some breakpoints, I can see that the order of operations seems off:

  1. \Bugsnag\Shutdown\PhpShutdownStrategy::registerShutdownStrategy
  2. \Bugsnag\Client::flush (queue is empty)
  3. \Bugsnag\Client::notify (after which, queue has report)

As you can see, flush seems to be getting called before notify, so the reports never get flushed to Bugsnag.

This seems to be because Yii's logger has its own register_shutdown_function to flush its logs, but is always called last. In this case Bugsnag's shutdown needs to be called after, or no logs exist.

Environment

timkelty commented 2 years ago

Not sure if this is the best solution, but I've fixed this by registering my own shutdown function:

<?php

namespace craftnet\logs;

use Bugsnag\Client;
use Bugsnag\Shutdown\ShutdownStrategyInterface;
use Craft;

/**
 * Class PhpShutdownStrategy.
 *
 * Use the built-in PHP shutdown function
 */
class PhpShutdownStrategy implements ShutdownStrategyInterface
{
    /**
     * @param Client $client
     *
     * @return void
     */
    public function registerShutdownStrategy(Client $client): void
    {
        register_shutdown_function(static function() use($client) {
            Craft::$app->getLog()->logger->flush(true);
            $client->flush();
        });
    }
}

Sidenote: it would be nice if Bugsnag\Client::make accepted an argument to pass the shutdownStrategy.

yousif-bugsnag commented 2 years ago

Hi @timkelty, just to confirm, does this issue only occur when batch sending is enabled?

timkelty commented 2 years ago

@yousif-bugsnag correct – if I disable batch sending, it works, because it sends them immediately and doesn't need to wait for the shutdown handler

johnkiely1 commented 2 years ago

HI @timkelty,

The Yii2 framework is not something we officially support as there are some more significant differences to other PHP frameworks. I suspect the issue you are reporting could be due to these differences. Full support for Yii2 is on our roadmap but I don't have a timeframe at the moment. I have noted your interest and use case and will let you know here as soon as we have updates.

For now your workaround would seem perfectly reasonable if it is suiting your needs.

As a slight aside, there is a third party Yii2 Bugsnag notifier library that someone has created, so that might be of interest to you: https://github.com/pinfirestudios/yii2-bugsnag. However its worth noting Bugsnag has no connection to this so we cannot support it nor verify if it works correctly.

timkelty commented 2 years ago

@johnkiely1 thanks!

FWIW, this is for Craft CMS@4 (Yii2 under the hood), which uses Monolog for logging, so we're currently getting Bugsnag integration through https://packagist.org/packages/mead-steve/mono-snag