Exception / trace member

php.internals

Stephane Drouard

22 years ago
The current implementation of class Exception has a private "trace" member, which is used to store the backtrace. But if you throw an exception of a derived class of Exception, print_r() displays an empty "trace:private" member, as well as a "trace" (public) member that really holds the backtrace. I deduce that the backtrace is executed/stored at the derived class level, explaining why a new public member is created. Curriously, I have also noticed that "getTrace()" returns the public one, not the private one as expected. This is not to report a bug, but to ask you not to fix it as it was intended ("trace" member as private). Indeed I use this "feature" (being able to modify the trace member) to hide an intermediate call: <? class SystemException extends Exception { function __construct($message, $code) { parent::__construct($message, $code); // Hide trace of handler() call. array_splice($this->trace, 0, 1); $this->file = $this->trace[0]['file']; $this->line = $this->trace[0]['line']; } static function handler($code, $message) { throw new SystemException($message, $code); } } set_error_handler(array('SystemException', 'handler')); ?> That way, the reported error focuses on the real file and line, not on the uninteresting intermediate handler call. In the current Exception class implementation, if both the "trace" member is private and getTrace() is final, it would be impossible to override the default behaviour. As a result, we would do our own exception base class, which goes against code reuse: need to redo all the things already done, just because the proposed class is not too open. A second consequence, is that each time you use code you've got from "outside", it could implement its own exception base class, not using the provided Exception. If so, we won't be able to write generic code that guaranties to catch all exceptions (C++ "catch(...)" or Java "finally" not implemented in PHP5, so need to specify a class for catch). The last point suggests me that PHP should guaranty that all thrown exceptions derive from Exception. It could be done in 2 ways: 1/ a fatal error when executing "throw new ...". Problem: it's not easy to simulate all error conditions, so we may deliver code that was never tested on all its "throw" branches. As a result, the script will stop on a fatal error, even though it was intented to catch and handle exceptions in a clean way. 2/ even if the class is not explicitly derived from Exception, throwing it will make it derived from Exception. Problem: an object of such a class will be an instance of Exception only after being thrown. The 2nd is my prefered one, even if I understand it could not be so easy to implement. One word about private and final: I don't think it's good programming to use them, except for security reasons. Not using them allows classes to be easily extended, in different directions, which is the goal of OOP. Moreover, all members should be protected, not public, and accessed through (virtual) methods, which guaranties that everything can be overriden if needed. Regards, Stephane

Marcus Börger

22 years ago
Hello Stephane, if this is really the case, well then i am the one to blame. I'll have a look. Thursday, January 22, 2004, 3:46:13 PM, you wrote:
> The current implementation of class Exception has a private "trace" > member, which is used to store the backtrace. But if you throw an > exception of a derived class of Exception, print_r() displays an empty > "trace:private" member, as well as a "trace" (public) member that really > holds the backtrace.
> I deduce that the backtrace is executed/stored at the derived class > level, explaining why a new public member is created. > Curriously, I have also noticed that "getTrace()" returns the public > one, not the private one as expected.
> This is not to report a bug, but to ask you not to fix it as it was > intended ("trace" member as private). Indeed I use this "feature" (being > able to modify the trace member) to hide an intermediate call:
> <? > class SystemException extends Exception { > function __construct($message, $code) { > parent::__construct($message, $code); > // Hide trace of handler() call. > array_splice($this->trace, 0, 1); > $this->file = $this->trace[0]['file']; > $this->line = $this->trace[0]['line']; > }
> static function handler($code, $message) { > throw new SystemException($message, $code); > } > }
> set_error_handler(array('SystemException', 'handler'));
?>>
> That way, the reported error focuses on the real file and line, not on > the uninteresting intermediate handler call.
> In the current Exception class implementation, if both the "trace" > member is private and getTrace() is final, it would be impossible to > override the default behaviour. As a result, we would do our own exception > base class, which goes against code reuse: need to redo all the things > already done, just because the proposed class is not too open.
> A second consequence, is that each time you use code you've got from > "outside", it could implement its own exception base class, not using the > provided Exception. If so, we won't be able to write generic code that > guaranties to catch all exceptions (C++ "catch(...)" or Java "finally" not > implemented in PHP5, so need to specify a class for catch).
> The last point suggests me that PHP should guaranty that all thrown > exceptions derive from Exception. It could be done in 2 ways:
> 1/ a fatal error when executing "throw new ...". > Problem: it's not easy to simulate all error conditions, so we may > deliver code that was never tested on all its "throw" branches. As a > result, the script will stop on a fatal error, even though it was intented > to catch and handle exceptions in a clean way.
> 2/ even if the class is not explicitly derived from Exception, throwing > it will make it derived from Exception. > Problem: an object of such a class will be an instance of Exception only after being thrown.
> The 2nd is my prefered one, even if I understand it could not be so easy to implement.
> One word about private and final: I don't think it's good programming > to use them, except for security reasons. Not using them allows classes to > be easily extended, in different directions, which is the goal of OOP. > Moreover, all members should be protected, not public, and accessed > through (virtual) methods, which guaranties that everything can be > overriden if needed.
> Regards, > Stephane
-- Best regards, Marcus mailto:helly@php.net

Marcus Börger

22 years ago
Hello Stephane,
> if this is really the case, well then i am the one to blame. I'll have a > look.
i looked into it - you were right - and i fixed it. Could you please test current HEAD to verify i have fixed all issues? That would give me a better feeling. And thanks for noticing this. marcus
> Thursday, January 22, 2004, 3:46:13 PM, you wrote:
>> The current implementation of class Exception has a private "trace" >> member, which is used to store the backtrace. But if you throw an >> exception of a derived class of Exception, print_r() displays an empty >> "trace:private" member, as well as a "trace" (public) member that really >> holds the backtrace.
>> I deduce that the backtrace is executed/stored at the derived class >> level, explaining why a new public member is created. >> Curriously, I have also noticed that "getTrace()" returns the public >> one, not the private one as expected.
>> This is not to report a bug, but to ask you not to fix it as it was >> intended ("trace" member as private). Indeed I use this "feature" (being >> able to modify the trace member) to hide an intermediate call:
>> <? >> class SystemException extends Exception { >> function __construct($message, $code) { >> parent::__construct($message, $code); >> // Hide trace of handler() call. >> array_splice($this->trace, 0, 1); >> $this->file = $this->trace[0]['file']; >> $this->line = $this->trace[0]['line']; >> }
>> static function handler($code, $message) { >> throw new SystemException($message, $code); >> } >> }
>> set_error_handler(array('SystemException', 'handler'));
?>>>
>> That way, the reported error focuses on the real file and line, not on >> the uninteresting intermediate handler call.
>> In the current Exception class implementation, if both the "trace" >> member is private and getTrace() is final, it would be impossible to >> override the default behaviour. As a result, we would do our own exception >> base class, which goes against code reuse: need to redo all the things >> already done, just because the proposed class is not too open.
>> A second consequence, is that each time you use code you've got from >> "outside", it could implement its own exception base class, not using the >> provided Exception. If so, we won't be able to write generic code that >> guaranties to catch all exceptions (C++ "catch(...)" or Java "finally" not >> implemented in PHP5, so need to specify a class for catch).
>> The last point suggests me that PHP should guaranty that all thrown >> exceptions derive from Exception. It could be done in 2 ways:
>> 1/ a fatal error when executing "throw new ...". >> Problem: it's not easy to simulate all error conditions, so we may >> deliver code that was never tested on all its "throw" branches. As a >> result, the script will stop on a fatal error, even though it was intented >> to catch and handle exceptions in a clean way.
>> 2/ even if the class is not explicitly derived from Exception, throwing >> it will make it derived from Exception. >> Problem: an object of such a class will be an instance of Exception only after being thrown.
>> The 2nd is my prefered one, even if I understand it could not be so easy to implement.
>> One word about private and final: I don't think it's good programming >> to use them, except for security reasons. Not using them allows classes to >> be easily extended, in different directions, which is the goal of OOP. >> Moreover, all members should be protected, not public, and accessed >> through (virtual) methods, which guaranties that everything can be >> overriden if needed.
>> Regards, >> Stephane
> -- > Best regards, > Marcus mailto:helly@php.net
-- Best regards, Marcus mailto:helly@php.net

Stephane Drouard

22 years ago
Hello Marcus, Hum... you fixed the issue but not really in the way I expected it. Now my code does not work... My message was not only to report a bug, but to ask *not* to fix it as expected. Particularly to put the "trace" member as protected, (or "getTrace()" as virtual and not final). Remember my code: <? class SystemException extends Exception { function __construct($message, $code) { parent::__construct($message, $code); // Hide trace of handler() call. array_splice($this->trace, 0, 1); $this->file = $this->trace[0]['file']; $this->line = $this->trace[0]['line']; } static function handler($code, $message) { throw new SystemException($message, $code); } } set_error_handler(array('SystemException', 'handler')); ?> Indeed, I'm no more able to modify the trace, so if I want to report the real file and line (I mean the interresting ones, so those in trace[1]), they won't be coherent with the back trace. Why could you accept a derived class to modify "file" and "line" members, and not the "trace" one? My original message was also to limit the use of private and final modifiers, as they go against inheritance. Regards, Stephane

Marcus Börger

22 years ago
Hello Stephane, i had reasons to make that property private. You may use getTrace() to read the contents and if you must overwrite it because you want to misuse the whole thing then you can overwrite the read access method, too. This should be enough, right? Friday, January 23, 2004, 10:54:53 AM, you wrote:
> Hello Marcus,
> Hum... you fixed the issue but not really in the way I expected it. Now my code does not work...
> My message was not only to report a bug, but to ask *not* to fix it as > expected. Particularly to put the "trace" member as protected, (or > "getTrace()" as virtual and not final).
> Remember my code:
> <? > class SystemException extends Exception { > function __construct($message, $code) { > parent::__construct($message, $code); > // Hide trace of handler() call. > array_splice($this->trace, 0, 1); > $this->file = $this->trace[0]['file']; > $this->line = $this->trace[0]['line']; > }
> static function handler($code, $message) { > throw new SystemException($message, $code); > } > }
> set_error_handler(array('SystemException', 'handler'));
?>>
> Indeed, I'm no more able to modify the trace, so if I want to report > the real file and line (I mean the interresting ones, so those in > trace[1]), they won't be coherent with the back trace.
> Why could you accept a derived class to modify "file" and "line" members, and not the "trace" one?
> My original message was also to limit the use of private and final > modifiers, as they go against inheritance.
> Regards, > Stephane
-- Best regards, Marcus mailto:helly@php.net

Stephane Drouard

22 years ago
Hello Marcus, == Quote from Marcus Boerger (helly@php.net)'s article
> i had reasons to make that property private. You may use getTrace() to > read the contents and if you must overwrite it because you want to misuse > the whole thing then you can overwrite the read access method, too. This > should be enough, right?
Except if I misunderstood your answer, but getTrace() is final, so I can't overwrite it. This was in fact the idea of my original message, by putting methods final (and most of Exception are) you can't overwrite them, limiting by this fact its use. A second idea of my original message was to force Exception to be the base class of all exceptions, to allow to write generic code that catches all exceptions (to mimic "finally" clauses). There were no reactions on that, but according to our exchanges, I assume you would not do that (you're more on proposing things rather than imposing them, right?). So what about implementing a "catch(...)" or a "finally" clause? Regards, Stephane

Marcus Börger

22 years ago
Hello Stephane, maybe i could make getTrace a protected or even public method in 5.1 but at the moment it is impossible. The whole thing of handling the exceptions is very complex and any change may result in the whole exception/catching facility being bork and trowing around SEGVs. Then we didn't add finally or catch all because we already had far too much problems. And now we are in feature freeze mode there is no chance of such features. If we would add one of them we'd need to start with another b1 because the impact is to high if you go and change compiler and/or executor. Anyway having soemone like you to play with all the things to their deepst tricks is a good thing only you currently need to live with what we have until you find real bugs. So please continue investigating and maybe you need to collect all of your ideas for developing after 5.0 is out. marcus Saturday, January 24, 2004, 4:23:54 PM, you wrote:
> Hello Marcus,
> == Quote from Marcus Boerger (helly@php.net)'s article >> i had reasons to make that property private. You may use getTrace() to >> read the contents and if you must overwrite it because you want to misuse >> the whole thing then you can overwrite the read access method, too. This >> should be enough, right?
> Except if I misunderstood your answer, but getTrace() is final, so I > can't overwrite it. This was in fact the idea of my original message, by > putting methods final (and most of Exception are) you can't overwrite > them, limiting by this fact its use.
> A second idea of my original message was to force Exception to be the > base class of all exceptions, to allow to write generic code that catches > all exceptions (to mimic "finally" clauses). > There were no reactions on that, but according to our exchanges, I > assume you would not do that (you're more on proposing things rather than > imposing them, right?). So what about implementing a "catch(...)" or a > "finally" clause?
> Regards, > Stephane
-- Best regards, Marcus mailto:helly@php.net

Andi Gutmans

22 years ago
At 03:23 PM 1/24/2004 +0000, Stephane Drouard wrote:
>Hello Marcus, > >== Quote from Marcus Boerger (helly@php.net)'s article > > i had reasons to make that property private. You may use getTrace() to > > read the contents and if you must overwrite it because you want to misuse > > the whole thing then you can overwrite the read access method, too. This > > should be enough, right? > >Except if I misunderstood your answer, but getTrace() is final, so I can't >overwrite it. This was in fact the idea of my original message, by putting >methods final (and most of Exception are) you can't overwrite them, >limiting by this fact its use. > >A second idea of my original message was to force Exception to be the base >class of all exceptions, to allow to write generic code that catches all >exceptions (to mimic "finally" clauses). >There were no reactions on that, but according to our exchanges, I assume >you would not do that (you're more on proposing things rather than >imposing them, right?). So what about implementing a "catch(...)" or a >"finally" clause?
You can create your own exception hierarchy. Anyway, as Marcus mentioned this is not the time to make changes to the exception mechanism (except for bug fixes). We will see how things go with 5.0.0 and will evaluate extending things as needed. We are at a time now where we have to stabilize things (including exceptions) and not add more problems. Andi