Wedge

Public area => The Pub => Topic started by: Arantor on October 31st, 2011, 09:52 AM

Title: Error reporting with app-issued errors
Post by: Arantor on October 31st, 2011, 09:52 AM
This bit me in the arse today.

Errors that are genuine errors and tracked through PHP's error handler will have file and line numbers attached because they're passed from PHP itself. But errors issued through fatal_lang_error and fatal_error, that when passed to log_error... they will not have file and/or line numbers.

There's a certain logic to that, actually, in that there's not really a need to pack out the DB with file/line numbers when it's issued by the code itself in response to a user event/other failure condition, rather than an 'error' in the classical sense.

Trouble is, this also means plugins that call fatal_lang_error also won't get logged as being under their plugin unless they specifically pass their plugin id to the error handler, which is kludgy at best.


There's two courses. Either way I can grab the backtrace and get the file/line it came from. The question, really, is whether for general errors (rather than undefined variables etc) that the file and line should be logged. From a debugging perspective it'd be useful, but I'm not sure the user needs to be presented with the file/line every time.

What I'm going to do for now is to keep it mostly as it is, but if no file is found, grab the filename, identify if it's a plugin or not, and then bar logging the file/line number. But the option's open if people want it.

(I'm also debating providing the full backtrace in the error log for debugging purposes, which would be very useful sometimes.)
Title: Re: Error reporting with app-issued errors
Post by: Arantor on October 31st, 2011, 10:10 AM
On a separate note, do we really need the session id in the error log?

I cringe every time someone posts that on sm.org, because if we have their URL, we can trivially steal their session if the id's fresh.
Title: Re: Error reporting with app-issued errors
Post by: Nao on October 31st, 2011, 03:29 PM
I'd personally go for calling backtrace any time. It's a neat little function anyway... :P

As for session ID -- I never understood that. I asked myself the question (not for security but simplicity), when I fixed the layout for error logs a few weeks ago.
Title: Re: Error reporting with app-issued errors
Post by: Arantor on October 31st, 2011, 03:34 PM
Putting the backtrace actually into the error log by default will bulk out the error log considerably. While the idea is that admins will know about it sooner, I'd still rather keep the log lean if possible.

Though why the session is in there, I have no idea. I have never used it for debugging for any reason, so I guess that can go?
Title: Re: Error reporting with app-issued errors
Post by: Nao on October 31st, 2011, 03:35 PM
backtrace: well, I'll let you make a decision on this then.

session: hell yeah.
Title: Re: Error reporting with app-issued errors
Post by: Arantor on October 31st, 2011, 03:37 PM
I'll leave the backtrace alone for now (it's not pulled but it can be turned into a debug option readily enough, though just got to be careful about not catching it and storing it in the last-error cache and more importantly to be careful about not getting recursive calls :/

As for session, I can remove that in a bit :)
Title: Re: Error reporting with app-issued errors
Post by: Nao on October 31st, 2011, 03:38 PM
I know how you treasure removing things :lol:
Title: Re: Error reporting with app-issued errors
Post by: Arantor on October 31st, 2011, 03:44 PM
I love removing things that are unnecessary, unneeded and undesired. I would have said unuseful but that's not a real word :P

What point is there in keeping something that just adds a weight to every error, that you don't ever use?
Title: Re: Error reporting with app-issued errors
Post by: Nao on October 31st, 2011, 04:53 PM
There's a delicate balance between feature set and usability, for sure.
Title: Re: Error reporting with app-issued errors
Post by: Arantor on October 31st, 2011, 04:57 PM
True - but on the other hand, a feature that benefits no-one to the best of our knowledge is not good usability. As for some of the other stuff we've removed, almost every time it was a feature that was little or never used, and as such, removing it has a usability increase, not a decrease. Less complexity = easier to use.