Suitecrm: A blank screen is displayed instead of an message.

[jobvector]

Issue

If the URL of a deleted CRM Record is entered into the browser a blank screen is displayed instead of an message. In the previous versions a message was displayed, something like
that the user either has no access or the record is deleted

Expected Behavior

Displaying of an message

Actual Behavior

A blank screen is displayed instead of an message.

Steps to Reproduce

1. Create an Test-Call
2. Copy URL of Test-Call
   

1. Delete Test-Call record
   

2. Insert the copied URL of Test-Call in browser

3. It will display a blank page instead of error message as in earlier versions
   

Your Environment

SuiteCRM Version used: 7.7.8
Browser name and version: Firefox 50.1.0 (64-bit)
Environment name and version: MySQL, PHP 7.0
Operating System and version: Ubuntu 16.04 (64-bit)

[ChangezKhan]

Hello @jobvector, I have seen in include\utils.php file, in function named sugar_die, this line is commented echo $error_message; and because of that it is showing blank screen rather than an error message. For time being, we can uncomment it.

Hello @shogunpol, I would like to know, why it is commented, because it was uncommented in SugarCRM. Kindly let me know.

[shogunpol]

@ChangezKhan , It is part of commit done by: @adamjakab, lines:

1621                   //echo $error_message;
1622                   //die($exit_code);
1623                     throw new \Exception($error_message, $exit_code);

has been modified on 24/03/16 , here commit: commit

[ChangezKhan]

@shogunpol , from his commit I am not able to understand the need of commenting out these lines. I have left him a message on his commit.

[adamjakab]

Hi all[@shogunpol] - the fact is that the function sugar_die() has no reason to exist.
But it does, so, for the time being we have to live with it. What is to be done here is to kill all sugar_die calls in code and replace them with throwing exceptions. Exactly what I have done in sugar_die(because of lack of time). The fact is that if the message thrown by the exception is not displaying then somewhere else in the code the exception is caught and silenced (v. naughty). Find that point and fix it instead of echoing stuff on each sugar_die call.

[ChangezKhan]

Hi @adamjakab , I just found out that actually this exception call is logging the error, but not echoing out. So, is it behaving correctly or it should echo out too?

[adamjakab]

hi @ChangezKhan - an exception does not do anything. If uncaught by caller stack it will bubble up to top and interrupt all further execution of code. If php is configured to do so it will display the error message if not it will not.

When an exception is thrown, code following the statement will not be executed, and PHP will attempt to find the first matching catch block. If an exception is not caught, a PHP Fatal Error will be issued with an "Uncaught Exception ..." message, unless a handler has been defined with set_exception_handler().

and again, if your log_errors for php is set to 1 then yes, it will log the fatal error. logging and displaying are separate things.

i'd suggest, if you want to resolve this: enable displaying of errors, install and activate xdebug for php (so you can see the call stack, find 'who' is calling sugar_die, catch the exception and act on it - that is - display the error message (and do whatever else is necessary)

[adamjakab]

Sorry, I have just read the issue.
This is view level issue. The view is calling sugar_die in include/MVC/View/views/view.detail.php:98 when bean loaded with the id you supply with "record" parameter is not found.
This is bad stuff.
that's where you want to display the error message.

 /**
     * @see SugarView::display()
     */
    public function display()
    {
        if(empty($this->bean->id)){
            print('<h1 class="error">' . $GLOBALS['app_strings']['ERROR_NO_RECORD'] . '</h1>');
            return;
        }

        $this->dv->process();
        echo $this->dv->display();
    }

this is just to illustrate how you could do it - sorry i do not have time at the moment to think about this actively.

[ChangezKhan]

Hello @adamjakab , glad you understood the issue. But I think, we will need to add the code that you mentioned above in every view.detail.php files available in modules\<module_name>\views\ folder, apart from include/MVC/View/views/view.detail.php file. Please correct me, if I am wrong.
@shogunpol, need your opinion on this solution or we'll have to look for a better solution.

[adamjakab]

hi @shogunpol - no need to do that - if a module does not have its own view.detail.php file, the generic (include/MVC/View/views/view.detail.php) file will be used.

[shogunpol]

@ChangezKhan , you can create Pull Request, and can be great starting point to handle this issue, we do proper testing, including code analyse.

[adamjakab]

sorry guys wrong mention above - I ment @ChangezKhan ;)

[ChangezKhan]

@adamjakab , yeah that's why I mentioned that, need modification in files available in modules\<module_name>\views\ folder.

[adamjakab]

yeah, clearly! If a module has its own views/view.detail.php then the display function must be revised.
Is this an issue for all modules?

[ChangezKhan]

@adamjakab , yeah I have checked with lead and contact modules. Basically, this issue occurs where one tries to access a records which is not available in CRM.

[adamjakab]

Then yes, roll up your sleves and fix 'em all!

[ChangezKhan]

@adamjakab yeah, I have started

[chris001]

It would be probably better to use the PHP function set_error_handler, and let the framework display the errors in a standard way. This way, you can avoid immediately forcing all module developers to release new versions of their modules, which catch the exception.

[samus-aran]

Hi Everyone!

Just thinking, there is 'alot' of sugar_die calls throughout the system and until we have a proper error handling mechanism I don't think we should be altering the views in the PR provided (https://github.com/salesagility/SuiteCRM/pull/3036)

As a middle ground should we not just include the echo prior to the exception to provide the user feedback resolving this issue until a better plan is drawn up.
function sugar_die($error_message, $exit_code = 1) { global $focus; sugar_cleanup(); echo $error_message; throw new \Exception($error_message, $exit_code); }

[chris001]

@samus-aran Yes. That's exactly what I meant!

1. Change sugar_die() to throw the \Exception, rather than itself call die() or exit() as was done in 2005!
2. Nowhere else in the core code is allowed to be be calling die() or exit() either, _including the entrypoint checker boilerplate, which really ought to be changed now to be used at the top of all php files_, not copy-pasted which is terrible and violates coding standard practice when need to update it and there are so many source code files.
3. The few remaining places in the core code now calling exit() or die() must now be edited to call sugar_die() just like the others.
4. Rejoice - SuiteCRM users will finally have no more mysterious and frustrating White Screens Of Death when a web app crashes with no information in the browser. WSOD should truly never happen, in this day and age, and worthy of a shameful massive group facepalm if ever it does.