[OJS] Article without Section causes fatal error about pkp-lib HOT 9 CLOSED

That's a data issue -- if you know how the article came to be sectionless, I'd prefer to track down the cause. (It's possible that it came from an earlier version of OJS.)

Generally speaking I'd actually prefer this kind of data situation to cause an error rather than continuing silently -- we don't make any guarantees about the system's operation when the data is non-conformant, so allowing it to continue silently will mask the problem.

We've talked on a few occasions about writing a data check tool that tests for database integrity, but haven't gotten to it.

from pkp-lib.

This is almost certainly from an earlier version: specifically, data entered under 2.3.3.1, then upgraded to 2.3.8, then to 2.4.5.

Having it throw an error sounds reasonable, but throwing an uncaught fatal error does not. This is being called from the PDF display (.../article/view/...) so it is a very high-profile error. There's no reason we can't throw an error to the log or to the admin and still continue to display the PDF for a sectionless article to the end user.

from pkp-lib.

Agreed, and the error message itself isn't very useful.

Currently we're using a mix of approaches, none of them perfect:

• Assertions. Good for coding, and arguably integrity sanity checks are what they're for, but often disabled in production environments.
• fatalError() function calls. (This is our wrapper around die(), which can provide stack traces when the config.inc.php option is enabled). Looks awfully programmer-oriented to the end user in either configuration. (I've never liked this wrapper.)
• die() function calls. I don't think there are many of these.
• Object existence checks, like you've proposed above; these are OK, but risk hiding error conditions.

How does this sound...

• Continue to use assertions for integrity checks
• Handle errors so that end users are presented with a "nice" error page when the site is not configured as a dev environment.

If we can simultaneously remove the collected kludges (fatalError comes to mind), so much the better.

One very viable option would be to make comprehensive changes on the master branch only; there's already been a fair amount of cleanup there (e.g. removal of the old error handler kludge that was necessary for PHP4 support).

from pkp-lib.

The assertions for integrity checks and friendly error handling make sense to me.

For the comprehensive changes on master to handle the "nice" error page, are you thinking something like a register_shutdown_function()?

In this instance for 2.4.5, I checked our journals. Of 60-some journals in 30 installations, we have one with 3 data errors, one with 6 data errors, and one with 1,200 data errors. I presented this to our Scholarly Communications folks, and we think this would benefit from the near-term and limited-focus fix of adjusting this code block to avoid the fatal error. If this was a prior bug which is now fixed but has left data issues, we can't be the only ones affected. If others are affected on the order that our one large journal was, they won't be able to circumvent the issue as easily as we can.

It will take quite some time to cleanup the 1200 missing section ids, and the software should avoid the fatal error in the meantime. Their suggestion was to grab the getDisableComments() setting from the first existent section.

from pkp-lib.

Clinton, I was thinking about set_error_handler (http://php.net/manual/en/function.set-error-handler.php) rather than register_shutdown_function. That's well-supported, including PHP 4.x, where things like exception handling don't exist yet.

What's likely happened to cause those data errors is that the section they were in was deleted when using a prior version of OJS. I'd suggest looking in the table of contents to see how those articles are presented. It might also be possible that those articles are archived and were never published.

from pkp-lib.

Currently, the orphaned articles float to the top of the TOC, and the only error report we've received is in viewing the PDF. We have our admins and the journal managers reviewing these particular cases. I'll pass that comment along, and I should have more data on whether they think they "should" have been published soon.

The set_error_handler should be good. I didn't realize it could handle fatals. But.... let PHP4 die. It has been over 6 years since PHP4 reached end of life. We're headed the right way by moving master to PHP5 only functionality.

from pkp-lib.

Don't worry, I'm not losing sleep over PHP4 support. That horse is dead and we're not particuarly invested in kicking it further. Just that set_error_handler plays nicely with the current OJS2 codebase, whereas exceptions would introduce a pattern that's new to the package.

from pkp-lib.

(BTW, I think a more helpful thing for OJS to have done is to have errored out immediately on the missing data rather than having those articles quietly float to the top in the reader interface. The data error hasn't been fixed because the reader interface looked OK and didn't throw any errors; meanwhile, who knows what import/exports, OAI, etc. are doing? It certainly won't be consistent and likely won't be compliant in the case of XML exports.)

from pkp-lib.

Confession time: I just ran into this recently and was irritated by it myself. I'm still in favour of some kind of data checker, but I doubt we're going to get to it before the 3.0 line. I've gone with one of your proposals in the original post.

from pkp-lib.