Skip to content
This repository has been archived by the owner on Feb 11, 2023. It is now read-only.

unremoved handlers #2

Open
jeroenbourgois opened this issue Jan 31, 2013 · 15 comments
Open

unremoved handlers #2

jeroenbourgois opened this issue Jan 31, 2013 · 15 comments

Comments

@jeroenbourgois
Copy link

If the _init function is called more than once, previous handlers are not cleaned. I understand that the _init should not be called twice, but apparently somewhere during the autoloading process by Fuel, it does happen.

I fixed this by adding the following lines to the _init() function:

if(!is_null(static::$monolog)) {
  return;
}

I understand this is not ideal, but without this fix, my Sentry handler was not called anymore...

@jeroenbourgois
Copy link
Author

I doublechecked this again, and whenever I call \Log::instance() from within my custom package, the _init() function is called and the monolog instance recreated. The code above will help checking if the instance already exists (in that case you can exit).

I do not fully understand why _init is called though... ?

@WanWizard
Copy link
Member

instance() calls

static::$monolog or static::_init();

So that can explain the call to _init. But I don't understand why it would do that, that should only happen if static:$monolog is null...

@WanWizard
Copy link
Member

This line shouldn't even be there, since _init is called on class load (and the issue with that was fixed in the autoloader in the 1.5 and 1.6 develop branches). So instance() should always have a valid Monolog instance.

@jeroenbourgois
Copy link
Author

Yes. I added a pullrequest to the Log repo here on github with my proposition for a 'fix'. It is indeed a strange behaviour, but it explains why my own handler got lost in the void, since it was ditched upon creating the new instance. Fuel catches the error and calls the Log class after the packages are loaded, so my custom package couldn't not attach it's handler long enough so it seems.

EDIT: just saw you closed my pullrequest :P

@jeroenbourgois
Copy link
Author

Yes the bug with the autoloader class is what started my quest, I reported it and they fixed it rather quick. Also, should my custom package's _init method be called also upon loading? Because that is not the case, even with the bugfix in the autoloader. Inside the bootstrap of my package I have:

Autoloader::add_core_namespace('Sentry');
Autoloader::add_classes(array(
    'Sentry\\Sentry' => __DIR__.'/classes/sentry.php',
));

\Sentry::instance();

If I lose the last line, the class is not init.

@WanWizard
Copy link
Member

I just closed that pull request. It's a workaround, like this line is a workaround. We need to find the real cause of this, and at the moment I haven't got a clue why that line would call _init a second time, and why if that happens, your workaround wil fix it. Since both run the same test on the same class property.

@WanWizard
Copy link
Member

There is only one possible reason I can think of.

If an error occurs in Log::_init(), it will abort before the Monolog instance is created. The error handler will want to log the error, so it will call log again, which will NOT call _init (since the class is already loaded), and that will fail with an empy property.

If you are using the Log class from the 1.5 release package, that doesn't have the fixes yet. 1.5/develop branch does, so perhaps you need to get that too.

@jeroenbourgois
Copy link
Author

I will try that today and give you feedback

Sent from somewhere in the universe, in a galaxy far far away

On 07 Feb 2013, at 23:59, Harro Verton [email protected] wrote:

There is only one possible reason I can think of.

If an error occurs in Log::_init(), it will abort before the Monolog instance is created. The error handler will want to log the error, so it will call log again, which will NOT call _init (since the class is already loaded), and that will fail with an empy property.

If you are using the Log class from the 1.5 release package, that doesn't have the fixes yet. 1.5/develop branch does, so perhaps you need to get that too.


Reply to this email directly or view it on GitHub.

@jeroenbourgois
Copy link
Author

Sorry, no luck. I updated my fuel/core branch to the 1.5/master when I saw the autoloader fix was merged there. Then I tried both 1.5/develop as 1.6/develop for the log package but with no luck. I still have to add my 'hack' to check the instance in order to get it to work...

@jeroenbourgois
Copy link
Author

Any updates here?

@WanWizard
Copy link
Member

You're the only one with this problem, so it's difficult to see from here what is causing it.

If it's (still) about the autoloader not calling _init() after load, see if you can debug that.

@jeroenbourgois
Copy link
Author

I posted the issue also in the core repo with a new test on the 1.5.3. Maybe this can help... fuel/core#1414

I found that first inside my package the \Log instance is created and then the core\base.php kicks in and init's the log again!

@WanWizard
Copy link
Member

Try 1.6 (or wait a few days for it's release), a large chunk of the log handling is rewritten.

@WanWizard
Copy link
Member

It might be caused by an exception (for example in the shutdown handler) leaving the autoloader in a race condition, which can either forget to call _init(), or call it twice(). This is all adressed in 1.6.

@jeroenbourgois
Copy link
Author

I will wait a few days then :) and we can move our project to 1.6

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants