There is inconsistency in JUser::save method.

Doc block say that method should throw exception but all save method use try and catch to catch all that exception inside and return false.

Exceptions from bind method and database query are not caught in save method.

https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/user/user.php#L703-L706

This code https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/user/helper.php#L62
does not care about any fails, but can catch an exception.

Below:
https://github.com/joomla/joomla-cms/blob/staging/libraries/joomla/user/helper.php#L124

This method is not ready to catch any exception. (doc block missing)

IMHO because of B/C save method probably won't change for now.
Can anybody confirm?

For now you can use something like:

// Save the data.
try
{
    // Only compare to false, in the future it may return null as success.
    if ($user->save() === false)
    {
        throw new Exception($user->getError(), 500);
    }
}
catch (Exception $e)
{
// ...
}

Suggest to close the issue here and refer to the appropriate channels: https://groups.google.com/forum/#!forum/joomla-dev-general or forum.joomla.org

The issue queue here is for software bugs in Joomla. See com_users for proper use of JUser.

This is the appropriate channel because it's relating to a B/C breaking change related to error handling. 4.0 is the place to make it as more error conditions are turned to proper Exception throwing and the reliance on JError is replaced so that something that's been deprecated since 2011 can finally rest peacefully at /dev/null.

Reset the priority according to the documented priority https://docs.joomla.org/Priority

@mbabker This would be the right channel to report the issue you mention. Doesn't sound like the issue being reported here.
To me it sounds like someone not understanding how to use JUser properly. As mentioned the call returns false when not succesful. In addition it also throws runtime exceptions, which have to be dealt with.

Not saying there is nothing wrong there, just saying that is not the issue reported!

Just as pointed out before there is a documentation inconsistency.
https://api.joomla.org/cms-3/classes/JUser.html#method_save

It's easy to read that and expect to catch an exception. So it should be
made clear what will throw one and what returns boolean false.

On Wednesday, September 21, 2016, sovainfo notifications@github.com wrote:

@mbabker https://github.com/mbabker This would be the right channel to
report the issue you mention. Doesn't sound like the issue being reported
here.
To me it sounds like someone not understanding how to use JUser properly.
As mentioned the call returns false when not succesful. In addition it also
throws runtime exceptions, which have to be dealt with.

Not saying there is nothing wrong there, just saying that is not the issue
reported!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#12094 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoctlaCnan01kf20EeF-nVVO7OSdlks5qsSY-gaJpZM4KBK10
.

Although I agree with improvements suggested, that is not what this issue is about. This issue is about the poor understanding about the usage of JUser. The expectations aren't met because they are false/wrong!
Again, for proper usage of JUser see current core code in administrator/components/com_users/models/user.php function save:

// Bind the data.
        if (!$user->bind($data))
        {
            $this->setError($user->getError());

            return false;
        }

        // Store the data.
        if (!$user->save())
        {
            $this->setError($user->getError());

            return false;
        }

Also already pointed out by @csthomas !
Both bind and save mention the return. Obviously, you should provide error handling for (real runtime) exceptions. Don't know about the desired functionality but consider it bad application design to ignore the return values.
As far as I know the error handling has not been implemented as exception handling only!

BTW With more that 1800 methods returning a boolean currently in the product, this shouldn't come as a surprise!

@sovainfo,

I want to get rid of using $this->setError($user->getError()); because both the functions setError and getError are marked as deprecated in Eclipse IDE.

Alternatively, I was using try catch expecting the save function will throw exception but it didn't.

I'm developing an extention from scratch and I want to get rid of all deprecated functions.

You're not going to. Some stuff marked deprecated is still used heavily
throughout Joomla, like JObject and JError style error handling.

On Thursday, September 22, 2016, akfaisel notifications@github.com wrote:

@sovainfo https://github.com/sovainfo,

I want to get rid of using $this->setError($user->getError()); because
both the functions setError and getError are marked as deprecated in
Eclipse IDE.

Alternatively, I was using try catch expecting the save function will
throw exception but it didn't.

I'm developing an extention from scratch and I want to get rid of all
deprecated functions.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#12094 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAWfoV9dya_ytritjykrIFJ9wuj-17Fuks5qseOxgaJpZM4KBK10
.

@akfaisel Appreciate what you are trying to do.
Use JFactory::getApplication()->enqueueMessage to get rid of $this->setError. See the ->bind method, using both!!

Using try/catch is not an alternative for error handling. It is used for exception handling. It should be used everywhere you want to introduce proper error handling. That doesn't mean it replaces the error handling of methods. As mentioned, the boolean return is used for that. Doubt that it will be completely replaced by exception handling. It is not meant to be used for that. Although, it is used in ->save.

Obviously, all returns false need to be replaced with throwing exceptions for that to work. When that is the situation the checking for false can be replaced by catches. Obviously, that can only work for new stuff, due to the B/C promise. See ->save : throw new RuntimeException('User not Super Administrator');
Newly added constraint that is allowed to use try/catch because it wasn't there, so it doesn't break BC.

For your own extension you can make sure you don't use deprecated stuff. Obviously, when using what is available in Joomla, you have no choice. Joomla doesn't provide clean alternatives for everything that is deprecated.