Re: [PEPr] -1 for RFC::Error Handling Guidelines for PHP5

On Sat, 28 Aug 2004 17:44:47 -0400, Greg Beaver <greg@chiaraquartet.net> wrote: > Justin Patrin wrote: > > > If the parser can contiue, then it's not an error. Parse problems in > > this instance would be warnings, then the code calling the parser can > > decide if these warnings should be upgraded to an error. The warnings > > could be aggregated into one error which is thrown out of the upper > > level code. > > The parser itself could indeed use exceptions, and the code would still > function just as accurately. The end-user would experience astronomical > levels of annoyance over continually fixing problem after problem, but > this would fit the rigid and narrow definition of an error as "only > something that can be thrown." However, it would be really crappy design. > I agree. Since the program can go on, THOSE ARE NOT ERRORS. > It is correct to say that the error is an error because as soon as it is > detected, recovery is impossible. This doesn't mean other errors can't > be detected, but it does mean that as soon as parsing completes, an > exception must be thrown to maintain correct code. > I don't believe that the RFC states that an exception must be thrown at the exact point of an error. This would be very rigid. However, an exception must be thrown at some point to tell the user that an error happened. > Regardless of whether you call it a warning or multiple errors, it is > indeed multiple errors, and in this case it is possible to handle them > all at once, and it is necessary for the user to standardize this > possibility so that they can know how to retrieve them. It is a > technical question for the internals of PEAR_Exception, but it must be > answered before we tell our users how to use the thing. > If it's possible to get mutliple errors back to the user, more power to you. This can easily be done by passing an array into the exception and dealing with it appropriately. If what you want is a way to do this will exceptions, either get some code into PEAR_Exception, or make an extended PEAR_Exception_Multiple. I am all in favor of this. > > > You say that we're distorting the definition of an error, but if we > > use exceptions for error handling, there is *no way* that the parser > > can continue parsing if it throws an exception. Unless, of course, it > > has great re-starting capabilities, but in this case it wasn't a > > situation where the execution had to stop, so it should have been a > > warning. I thought that the point of a sax parser was to be able to > > parse an invalid document, despite possible problems. It's up to the > > code calling the parser to decide if it's worth stopping on. > > Remember, I am thinking of things from the *user's* perspective, which > in this case is the only one that matters. Users of our packages want a > simple and consistent way to retrieve all error conditions. > Generally, there is one error which stops a program's execution. If you have a situation where there are multiples, you're getting into a gray area. > Another example of (poorly constructed) multiple error conditions that I > encountered the other day was DOMDocument->validate() in php5. If there > are any errors, each one is trigger_error()ed with E_WARNING. In order > to determine the exact kind of error, I as the user of DOMDocument have > absolutely no choice but to define an error catcher with > set_error_handler() and to just dumbly display the errors to the user. > Many of the errors are completely uninformative, such as "contents of > package element do not match" where package is the top-level element. I > would like to be able to re-format some of the error messages to be more > elucidating, but can't do this because I have no other information than > the error message, and not even documentation on the array of possible > error messages to do a kludgy parsing of the message. > Yes, that is poorly constructed. Another coder's bad design has nothing to do with this discussion. > Sure, they are all E_WARNINGs and by your definition a warning, but the > fact is this was simply a design choice in DOMDocument. There is no > recovery possible from an invalid package.xml, and by the definition in > the RFC, the errors should be thrown as exceptions. The design choice > does not define the existence of an error condition, the error condition > exists regardless of whether you throw it as an exception, > trigger_error() it as a warning or error, or use PEAR_Error. > > So, yes, there is twisting of the definition of error to fit the > technical solution you feel is best and claiming that there is no error, > rather than fitting the technical solutions to the problem. > Let's back up a bit here, we're saying the same things over and over. There are really two levels to the code you're talking about. 1) Sax Parser The point of the Sax parser is to parse an XML document which has possible errors and to be able to finish the parsing, right? If this is what it's supposed to do, by our definition those are warnings, not errors. As long as the parser continues without stopping, it's a warning. These would be handled as warnings. 2) PEAR Installer (or whatever needs the package.xml) After having the sax parser finish parsing, this code checks the warning stack and sees all of them. *This* code sees parsing problems as errors, so it aggregates the warnings (in an array of some kind) and throws an exception. Let me say it again. The parser doesn't care if there are parsing problems, it can go on. There fore, those are warnings. The code which called it sees those as an error and throws an exception. You may want to call those parsing errors and you're welcome to, but according to the RFC they're warnings. Whatever, it's just semantics. Let's stop quibbling about what to call these things and add an way to handle multiple error codes / strings at once. To make this very simple, just instantiate (not throw) an exception for each one of these. Then pass them as an array into PEAR_Exception_Aggregate. It just has to be written. To make this even simpler, we can justmake $p2 in the constructor of PEAR_Exception allow an array of exceptions to be passed in. This should add 10 or so lines to the code, nothing comparatively. -- DB_DataObject_FormBuilder - The database at your fingertips http://pear.php.net/package/DB_DataObject_FormBuilder paperCrane --Justin Patrin--