pug-php / pug

Pug template engine for PHP
https://www.phug-lang.com
MIT License
391 stars 42 forks source link

PHP notice raises ExceptionError? #187

Closed alessandro-fazzi closed 6 years ago

alessandro-fazzi commented 6 years ago

Hello,

I encountered an issue with the following code:

= $array['non-existent-key']

I expected to get:

<p></p>

But I actually get an ExceptionError.

Undefined index AFAIK will generate a notice in PHP. Why is Phug throwing an ExceptionError? Is this expected behaviour? Is this a configurable behaviour?

Thanks!

alessandro-fazzi commented 6 years ago

I'm going to update initial informations. Which, at his point of my comprehension, makes the situation even more complicated.

I have a pug file with

- $bar = new Foo()
= $bar->get_key('non-existent-key')

Given that Foo class is already in the scope, this is the definition

class Foo {
    $ary = array('foo' => 'baz')

    function get_key($key){
        return this->$ary[$key]
    }
}

It seems to me that since get_key method will raise a noice, the pug Renderer will raise an exception of type ErrorException.

W/o any more information this sounds as a bug to me. Looking for more.

Thanks in advance

kylekatarnls commented 6 years ago

Hi, my proposition is to provide some settings to set error_reporting into account or a custom settings for errors: https://github.com/phug-php/util/blob/master/src/Phug/Util/SandBox.php#L28 Right now, our sandbox catch all or any error, but it does not take into account the types of errors.

But you must understand that if we do not catch errors, we will not be able to tell where it happened in the pug code. That's why the Renderer use a sandbox to catch errors, then add infos about the matching position in the pug code and re-throw or display it.

The error handler is not a bug, we will keep it because for 99% of cases, the wrapped error is more useful than the original error. Moreover, the wrapped error contains the original error, so no information can be lost.

But I agree to provide settings to ignore notice errors, strict standard errors and so on.

alessandro-fazzi commented 6 years ago

Hi @kylekatarnls ,

I'm absolutely with you about the usefulness of the error handler. But :)

I'm writing a template for WordPress and using a do_shortcode function which calls logic from within a 3rd party plugin. So what? ATM I'm running around the office screaming half the time, and fixing tons of 3rd party code for the other half 😛

I and a colleague of mine thought about using a :php filter in the template, but the NOTICE is anyway catched by Phug.

Summarizing:

We've migrated from pug-php v2 to v3 and we think it's an awesome work, and the debug/error handling implementations are just great. But I can't be responsible for notices from vendor (or just another dev team from my company) libraries while building a template.

The interface I'd expect would be an error_reporting option to pass to the Pug constructor with similar arguments to the underlying PHP one.

Moreover: could we think about do honor the PHP error reporting settings?

A lot of thoughts, in the hope they could be somewhat useful.

Thanks for your support

kylekatarnls commented 6 years ago

Yes error_reporting is already what we use. Now we provide a full-support.

You can already try the new sandbox with composer update and so you would be able to:

- error_reporting(E_ALL ^E_NOTICE)
= $array['non-existent-key']

Obviously, you can set error_reporting outside of the template (or in your php.ini).

I will add an option soon to customize the error handling inside templates.

kylekatarnls commented 6 years ago

Hi, here is the new option you can change by using the new renderer (composer update): https://phug-lang.com/#error-reporting-callable-int

I close this issue but do not hesitate to answer on it if you get any trouble.

alessandro-fazzi commented 6 years ago

I'm really sorry to come back just now. ATM here just to thank you for this improvement. I'm going to work on it.