Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Yii2: Logs are not flushed on shutdown #650

Open
timkelty opened this issue Jun 15, 2022 · 5 comments
Open

Yii2: Logs are not flushed on shutdown #650

timkelty opened this issue Jun 15, 2022 · 5 comments
Labels
backlog We hope to fix this feature/bug in the future feature request Request for a new feature

Comments

@timkelty
Copy link

timkelty commented Jun 15, 2022

Describe the bug

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

  • Exceptions seem to work, just not logged errors
  • If I disable batching ($bugsnagClient->setBatchSending(false);

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
Copy link
Author

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
Copy link

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

@yousif-bugsnag yousif-bugsnag added the awaiting feedback Awaiting a response from a customer. Will be automatically closed after approximately 2 weeks. label Jun 16, 2022
@timkelty
Copy link
Author

@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
Copy link
Member

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.

@johnkiely1 johnkiely1 added feature request Request for a new feature backlog We hope to fix this feature/bug in the future and removed awaiting feedback Awaiting a response from a customer. Will be automatically closed after approximately 2 weeks. labels Jun 17, 2022
@timkelty
Copy link
Author

timkelty commented Jun 17, 2022

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog We hope to fix this feature/bug in the future feature request Request for a new feature
Projects
None yet
Development

No branches or pull requests

3 participants