[#17669] - [4.0] Remove JError
- Fixed in Code Base
- 2 Feb 2018
- Medium
- Build: 4.0-dev
- # 17669
- Diff
- Digital-Peak:j4/remove-jerror
- Pending continuous-integration/drone/pr this build is pending Details
User tests: Successful: Unsuccessful:
joomla-cms-bot
- | Category | ⇒ | Administration com_banners com_finder com_menus com_messages com_modules com_users Language & Strings Front End External Library Libraries Plugins Unit Tests |
| Status | New | ⇒ | Pending |
| Labels |
Added:
?
?
?
|
||
Conflicts fixed
| Category | Administration com_banners com_finder com_menus com_messages com_modules com_users Language & Strings Front End External Library Libraries Plugins Unit Tests | ⇒ | Administration com_banners com_finder com_menus com_messages com_modules com_users Language & Strings Front End Libraries Plugins Unit Tests |
Conflicts fixed
What is the status of this one?
Are we positive that the use of getError and setError in JObject subclasses (or any place doing this still really) are OK with proper Exceptions and have no internal couplings to JError or JException mechanisms? If we can drop this side of the functionality without completely breaking those two methods which still have heavy use I think it's worth re-syncing this PR and doing a heavy testing round to make sure we're OK.
JObject's setError method definitely just adds (what's doc-block documneted as) strings from an internal array https://github.com/joomla/joomla-cms/blob/staging/libraries/src/Object/CMSObject.php#L229-L243 . In the getters we also deal with that string actually being an exception https://github.com/joomla/joomla-cms/blob/staging/libraries/src/Object/CMSObject.php#L131-L182
So we're definitely safe for JObject classes. I guess I need to go back through this again and just check what classes are actually using JError directly - but most of this stuff is outdated docblocks iirc
As I did this pr, I removed every trace of JError.
Was a rather painless merge, so we are in sync again.
On review this looks fine to me. Let's get some heavy testing on this
| Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-02-02 00:00:08 |
| Closed_By | ⇒ | wilsonge |
OK Well I'm sure we'll break some things - but clearly we need this and it's not getting tests
In my component migration to namespaces in preparation of J4 I am finding JError in src/Mail/Mail.php so it looks like not all traces of JError are gone.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/17669.
In the current joomla git you can find JError at lines 118 & 154 in Mail/Mail.ph. You can also find it in Application/CMSApplication.php, Cache/Storage/RedisStorage.php, Client/ClientWrapper.php, Installer/Adapter/ComponentAdapter.php, Menu/SiteMenu.php, MVC/View/CategoryFeedView.php, MVC/Model/AdminModel.php & MVC/View/CategoryView.php. These are all in libraries/src
Are you looking at the right version of those files? It’s expected to still be there for 3.x versions, but should be gone in 4.0 (just looking at the Mail.php file it isn’t used in the 4.0 file).
Current github and my installed 3.9 version. Problem I am having is that I am running a namespaced WebApplication which I have loaded the framework (included defines.php & framework.php), created a class of WebApplication. If an error happens in Mail.php it is calling \JError which is not found.
Perhaps there is some other autoloader I need to call beforehand so that JError is found?
In J3 it is expected JError exists. We're only removing it in J4. You should be covered by the framework.php include which in turn loads the autoloaders in libraries/import.legacy.php which should autoload everything in libraries/legacy
Hmmm, but it doesn't seem to autoload JError. Where should I look for answers? I assume this area is solely for J4.
This Pull Request was to the J4 branch (hence why we assumed you were referring to J4 in your comment here). If you create a fresh issue and drop in a stack trace of the issue your getting we should be able to help more :)
OK. I fixed the reason JError was being called so will have to revert to the state which caused the error and then will post a fresh issue. Thanks.
can you fix conflicts here please?