You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4php-dev@logging.apache.org by businessdad <di...@pathtoenlightenment.net> on 2012/11/23 16:38:12 UTC

LoggerAutoloader - Why "include" and not "require"?

I had some issues with a client where Log4php is installed. He got the error
*class LoggerHierarchy not found*, and I found out that, for some reasons,
the Autoloader could not load the file.

Unfortunately, as it often happens in production environments, PHP warning
were suppressed, therefore nobody realised that the file could not be loaded
until I noticed it. I'm therefore asking why the Autoloader uses *include*
to load the classes, which simply raises a warning and carries on in case of
failure, rather than *require*, which would stop the execution and informing
that the file could not be loaded.

Personally, I think that *require* should be used, as, if I ask for a class,
I need that class and, therefore, the file *must* be loaded for the caller
to work properly. Failing silently is not an option, as it simply causes
issues later on, making debugging much more difficult.




--
View this message in context: http://apache-logging.6191.n7.nabble.com/LoggerAutoloader-Why-include-and-not-require-tp34939.html
Sent from the Log4php - Dev mailing list archive at Nabble.com.

Re: LoggerAutoloader - Why "include" and not "require"?

Posted by businessdad <di...@pathtoenlightenment.net>.
On Fri, Nov 23, 2012 at 4:41 PM, Christian grobmeier [via Apache Logging] <
ml-node+s6191n34943h67@n7.nabble.com> wrote:

> On Fri, Nov 23, 2012 at 5:36 PM, businessdad
> <[hidden email] <http://user/SendEmail.jtp?type=node&node=34943&i=0>>
> wrote:
>
> Fair enough. Let's hear what Ivan says (the guy who does coding mainly
> these days)
>
> > About changing the Autoloader: I've done it already, together with
> several
> > other minor changes, but thanks for the suggestion. :)
>
> Hey, what about donating a patch to your favorite logging project? :-)
> We are always eager to get improvements into our code base. If you
> need help with that, let us know.
>

I would love to, but those changes were made to tailor Log4php for a
specific use inside a framework, they would be of no use to general public.
Apart from changing "include" to "require", that is, but that doesn't take
much anyway. :)




--
View this message in context: http://apache-logging.6191.n7.nabble.com/LoggerAutoloader-Why-include-and-not-require-tp34939p34944.html
Sent from the Log4php - Dev mailing list archive at Nabble.com.

Re: LoggerAutoloader - Why "include" and not "require"?

Posted by Christian Grobmeier <gr...@gmail.com>.
On Fri, Nov 23, 2012 at 5:36 PM, businessdad
<di...@pathtoenlightenment.net> wrote:
>
>
> On Fri, Nov 23, 2012 at 4:26 PM, Christian grobmeier [via Apache Logging]
> <[hidden email]> wrote:
>>
>> On Fri, Nov 23, 2012 at 4:38 PM, businessdad
>> <[hidden email]> wrote:
>>
>> in my opinion, a logging framework should never cause an application
>> to fail. If you do that, it might happen that you reconfigure logging
>> in production and - due to an error - you production app stops jus
>> because of your logging framework. This is not nice too.
>>
>> That said, it is actually very easy if you still prefer require
>> instead of included.
>>
>> You could exchange this file:
>>
>> http://svn.apache.org/repos/asf/logging/log4php/trunk/src/main/php/LoggerAutoloader.php
>>
>> Another option is to patch log4php in such a way, the user can decide
>> between require and include with some kind of ini parameter.
>>
>> What do you think?
>
>
> The issue is precisely that, due to the include, Logger carries on also when
> required classes are not loaded. Logger cannot work without LoggerHierarcy,
> and it fires a fatal error when it tries to instantiate it. That means
> backtracing the issue, just to find out that the file was not loaded.
>
> A Logger is a fundamental part of a system, not a fancy optional one like an
> animated menu. If the Logger doesn't work, it should raise an error. Having
> a website that keeps going on without anyone noticing that something is
> broken is worse than having it stopped.
>
> Besides, any call to the Logger would fail anyway, triggering an error, like
> in my case. Very simply, it would be easier to be informed of the root cause
> (file can't be loaded) straight away, rather than having to figure it out
> because some class doesn't exist.

Fair enough. Let's hear what Ivan says (the guy who does coding mainly
these days)

> About changing the Autoloader: I've done it already, together with several
> other minor changes, but thanks for the suggestion. :)

Hey, what about donating a patch to your favorite logging project? :-)
We are always eager to get improvements into our code base. If you
need help with that, let us know.

Cheers
Christian


>
> ________________________________
> View this message in context: Re: LoggerAutoloader - Why "include" and not
> "require"?
>
> Sent from the Log4php - Dev mailing list archive at Nabble.com.



--
http://www.grobmeier.de
https://www.timeandbill.de

Re: LoggerAutoloader - Why "include" and not "require"?

Posted by businessdad <di...@pathtoenlightenment.net>.
On Fri, Nov 23, 2012 at 4:26 PM, Christian grobmeier [via Apache Logging] <
ml-node+s6191n34941h5@n7.nabble.com> wrote:

> On Fri, Nov 23, 2012 at 4:38 PM, businessdad
> <[hidden email] <http://user/SendEmail.jtp?type=node&node=34941&i=0>>
> wrote:
>
> in my opinion, a logging framework should never cause an application
> to fail. If you do that, it might happen that you reconfigure logging
> in production and - due to an error - you production app stops jus
> because of your logging framework. This is not nice too.
>
> That said, it is actually very easy if you still prefer require
> instead of included.
>
> You could exchange this file:
>
> http://svn.apache.org/repos/asf/logging/log4php/trunk/src/main/php/LoggerAutoloader.php
>
> Another option is to patch log4php in such a way, the user can decide
> between require and include with some kind of ini parameter.
>
> What do you think?


The issue is precisely that, due to the include, Logger carries on also
when required classes are not loaded. Logger cannot work without
LoggerHierarcy, and it fires a fatal error when it tries to instantiate it.
That means backtracing the issue, just to find out that the file was not
loaded.

A Logger is a fundamental part of a system, not a fancy optional one like
an animated menu. If the Logger doesn't work, it *should* raise an error.
Having a website that keeps going on without anyone noticing that something
is broken is worse than having it stopped.

Besides, any call to the Logger would fail anyway, triggering an error,
like in my case. Very simply, it would be easier to be informed of the root
cause (file can't be loaded) straight away, rather than having to figure it
out because some class doesn't exist.

About changing the Autoloader: I've done it already, together with several
other minor changes, but thanks for the suggestion. :)




--
View this message in context: http://apache-logging.6191.n7.nabble.com/LoggerAutoloader-Why-include-and-not-require-tp34939p34942.html
Sent from the Log4php - Dev mailing list archive at Nabble.com.

Re: LoggerAutoloader - Why "include" and not "require"?

Posted by Christian Grobmeier <gr...@gmail.com>.
On Fri, Nov 23, 2012 at 4:38 PM, businessdad
<di...@pathtoenlightenment.net> wrote:
> I had some issues with a client where Log4php is installed. He got the error
> class LoggerHierarchy not found, and I found out that, for some reasons, the
> Autoloader could not load the file.
>
> Unfortunately, as it often happens in production environments, PHP warning
> were suppressed, therefore nobody realised that the file could not be loaded
> until I noticed it. I'm therefore asking why the Autoloader uses include to
> load the classes, which simply raises a warning and carries on in case of
> failure, rather than require, which would stop the execution and informing
> that the file could not be loaded.
>
> Personally, I think that require should be used, as, if I ask for a class, I
> need that class and, therefore, the file must be loaded for the caller to
> work properly. Failing silently is not an option, as it simply causes issues
> later on, making debugging much more difficult.

in my opinion, a logging framework should never cause an application
to fail. If you do that, it might happen that you reconfigure logging
in production and - due to an error - you production app stops jus
because of your logging framework. This is not nice too.

That said, it is actually very easy if you still prefer require
instead of included.

You could exchange this file:
http://svn.apache.org/repos/asf/logging/log4php/trunk/src/main/php/LoggerAutoloader.php

Another option is to patch log4php in such a way, the user can decide
between require and include with some kind of ini parameter.

What do you think?

Cheers
Christian