Error reporting with app-issued errors

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Error reporting with app-issued errors
« 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.)
Re: Error reporting with app-issued errors
« Reply #1, 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.
When we unite against a common enemy that attacks our ethos, it nurtures group solidarity. Trolls are sensational, yes, but we keep everyone honest. | Game Memorial

Nao

  • Dadman with a boy
  • Posts: 16,061
Re: Error reporting with app-issued errors
« Reply #2, 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.

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Error reporting with app-issued errors
« Reply #3, 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?

Nao

  • Dadman with a boy
  • Posts: 16,061

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Error reporting with app-issued errors
« Reply #5, 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 :)

Nao

  • Dadman with a boy
  • Posts: 16,061

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Error reporting with app-issued errors
« Reply #7, 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?

Nao

  • Dadman with a boy
  • Posts: 16,061

Arantor

  • As powerful as possible, as complex as necessary.
  • Posts: 14,278
Re: Error reporting with app-issued errors
« Reply #9, 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.