tuminoid / kisakone

Finnish Disc Golf Associations retired disc golf tournament manager. 2019-> FDGA uses Disc Golf Metrix.
https://kisakone.frisbeegolfliitto.fi/
GNU General Public License v3.0
9 stars 9 forks source link

Event error log spam #273

Open tuminoid opened 8 years ago

tuminoid commented 8 years ago
nCatchable fatal error: Object of class Error could not be converted to string in /var/www/kisakone/data/event.php on line 89
janimattiellonen commented 8 years ago

New to this system so I may be wrong, but anyway...

In this case, the possible culprit is the call to data_CreateSortOrder in function data_GetEvents, which may return an Error item but in the code no return value check is made.

In other parts of the same file, the following check is made:

if (is_a($sortOrder, 'Error'))
    return $sortOrder;

First quick assumption is that this should be added to data_GetEvents as well, before $sort is actually used.

However, this is not enough. This merely trnasfers the problem to another location, for example in editemail.php:102 (inside pdr_GetDemoEvent()). If the result of data_GetEvents() is a error item, then the foreach() will fail here too (and possibly any other place, where data_GetEvents() is called).

As I understand it, the possible Error item should be bubbled to the level, where error handling is been made (seems to happen in index.php:228).

Would it make sense to make these error item creations bubble up the error just by using exceptions? We could then just use a try-catch in index.php for easier error management (logging, directing the user to an error page etc)?

tuminoid commented 8 years ago

It might be easier said than done, since most of the code is not object-oriented. Error class could extend Exception class and be thrown that way, but there is an awful lot of code to convert. It would be a good change though, error cases are PitA right now.

janimattiellonen commented 8 years ago

Do you have a full stack trace available for this error? Might be easier to track down the origin of the error as the function, which contains the error is called in multiple locations. Or is it known, on which page(s) the error occurs?

tuminoid commented 8 years ago

That is all what is found in the production system log.

tuminoid commented 8 years ago

HHVM is not really helpful with traces or urls, I've even opened issues about those.

janimattiellonen commented 8 years ago

Ok, haven't really used HHVM.

tuminoid commented 8 years ago

HHVM is at least 4 times more performant than PHP 5.6, and we're still spiking out the servers during registration storms. PHP 7 would be close to same performance, but old code is incompatible AFAIK, and switching PHP version would also be more invasive to system. Bad side is that HHVM is much more undebuggable, even with xdebug on, than PHP would be.

This is not high priority by any means, so perhaps debug it in dev VM + PHP, if you reproduce it while doing some other development work.