[#9293] - mod_feed - catch all Exceptions (Solves #9291)
- Fixed in Code Base
- 17 May 2016
- Medium
- Build: staging
- # 9293
- Diff
- EmilMassey:rss
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success JTracker/HumanTestResults Human Test Results: 3 Successful 0 Failed. Details
User tests: Successful: Unsuccessful:
PR to avoid displaying an error in mod_feed as issued in #9291.
The module should now render a message: Feed not found
Testing Instructions
- Add a Feed Display module
- Add any RSS Feed which has uncommon date formats
- See if the module throws error in Front-End.
Votes
| Status | New | ⇒ | Pending |
| Labels |
Added:
?
|
||
This changes nothing. If the feed module is only catching RuntimeExceptions then it's better off catching all Exceptions instead. You're catching the thrown Exception and rethrowing a new Exception which makes debugging more difficult because now the error trace leads to that rethrown Exception versus the one that actually caused the error.
This PR has received new commits.
CC: @1apweb
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9293.
Yeah, it was silly sollution... I guess catching all Exceptions in mod_feed helper is the best solution for now.
| Title |
|
||||||
This PR has received new commits.
CC: @1apweb
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9293.
I have tested this item 
unsuccessfully on 0fd45bf
Tested on latest staging + this patch.
The fatal error described in #9291 appears with and without this patch applied (tested with the feed given in this issue).
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9293.
If it's a fatal error that would be expected then as that isn't a thrown Exception that can be caught. Maybe if that error is a Throwable in PHP 7 it can be caught in the feed module but not much else that can be done for PHP 5 without validating the format before passing it into the JDate instance.
Semantically this patch is valid in the fact that it will catch all Exception objects thrown during processing of the feed.
According to http://php.net/manual/en/datetime.construct.php, since 5.3.0, if date/time format passed to the constructor is invalid, then Exception is thrown, so the patch should work as 5.3.10 is minimum for Joomla 3.x. I've tested this on PHP 5.5.9, and the Exception has been caught in the module. @grhcj can you provide PHP version you tested this on?
Date/time format validation may be implemented as proposed here: http://php.net/manual/en/function.checkdate.php#113205, but is it really necessary?
brianteeman
- | Category | ⇒ | Modules |
@EmilMassey My php version is 5.6.15.
I have tested this item 
successfully on 0fd45bf
As @mbabker said - error might not be related to this PR - all fine with code review.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9293.
I have tested this item successfully on 0fd45bf
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9293.
I have tested this item successfully on 0fd45bf
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9293.
| Status | Pending | ⇒ | Ready to Commit |
RTC based on multible tests
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9293.
| Labels |
Added:
?
|
||
| Milestone |
Added: |
||
rdeutz
- | Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-05-17 18:02:34 |
| Closed_By | ⇒ | rdeutz |
| Labels |
Removed:
?
|
||
I have tested this item
successfully on 7d810d7
It's Ok, the module render a message: Feed not found with the patch.
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9293.