Smarty Forum Index Smarty
WARNING: All discussion is moving to https://reddit.com/r/smarty, please go there! This forum will be closing soon.

Watchout: Smarty disables E_NOTICE reporting while rendering
Goto page 1, 2  Next
 
This forum is locked: you cannot post, reply to, or edit topics.   This topic is locked: you cannot edit posts or make replies.    Smarty Forum Index -> Smarty 3
View previous topic :: View next topic  
Author Message
carloslage
Smarty n00b


Joined: 18 Oct 2010
Posts: 4

PostPosted: Mon Oct 18, 2010 6:26 pm    Post subject: Watchout: Smarty disables E_NOTICE reporting while rendering Reply with quote

Smarty has this great "feature", in which it disables the E_NOTICE reporting while rendering templates, also being disabled in all the smarty template function calls.

My team discovered this while developing under a "Zero Tolerance" policy for notices, and now we have to fix ALL the god damned notices that we missed because of this and our deadline is already short.

Our solution for this, after reading the awesome Smarty.class.php code, is:
Code:
$smarty->error_reporting = error_reporting();


This must be done right after you instantiate your smarty object, and then it all works as expected.

Good luck and godspeed.
Back to top
View user's profile Send private message
mohrt
Administrator


Joined: 16 Apr 2003
Posts: 7368
Location: Lincoln Nebraska, USA

PostPosted: Mon Oct 18, 2010 7:55 pm    Post subject: Reply with quote

The default error handling is set this way so accessing unset template variables won't emit PHP notices. Within the scope of the template language this is not considered and error. However, PHP will emit E_NOTICE errors causing more problems and questions that its worth, so disabling E_NOTICE during template rendering is the default. You however want the notices, and have control of the template error_handling, so adjust as you see fit.

Sorry it wasn't set perfectly for your specific needs out of the box, perhaps we can issue you a full refund. /s
Back to top
View user's profile Send private message Visit poster's website
carloslage
Smarty n00b


Joined: 18 Oct 2010
Posts: 4

PostPosted: Tue Oct 19, 2010 10:49 am    Post subject: Reply with quote

So, instead of encouraging good coding practices, by making sure you verify if the vars you're accessing are actually defined, you encourage the backwards PHP ancient coding practices where anything goes, as long as it kinda works.

Makes sense, I guess.
Back to top
View user's profile Send private message
douglassdavis
Smarty Junkie


Joined: 21 Jan 2008
Posts: 541

PostPosted: Tue Oct 19, 2010 1:27 pm    Post subject: Reply with quote

is that... a tantrum? Shocked

just kidding... seriously though... turning off PHP notices can often lead to less cluttered, more readable code, depending on your application (because you aren't having to use isset() all over the place to test if a var is set).

Some PHP coders make decisions that aren't based on rigid rules that some one handed down to them, but based on what's best for the specific project. But, I guess every body is different... So no big deal.


Last edited by douglassdavis on Tue Oct 19, 2010 1:51 pm; edited 1 time in total
Back to top
View user's profile Send private message
mohrt
Administrator


Joined: 16 Apr 2003
Posts: 7368
Location: Lincoln Nebraska, USA

PostPosted: Tue Oct 19, 2010 1:48 pm    Post subject: Reply with quote

This isn't about coding practices, its about templates spitting PHP errors that are not really errors. templates != PHP. PHP does not supply an elegant solution, you can only be in one error handling "mode" at any given time. If you are paranoid about your plugins emitting notices, switch the PHP error handling to your liking.

That said, we are looking at some possibilities for Smarty 3 that might get rid of the default mask.
Back to top
View user's profile Send private message Visit poster's website
carloslage
Smarty n00b


Joined: 18 Oct 2010
Posts: 4

PostPosted: Tue Oct 19, 2010 2:09 pm    Post subject: Reply with quote

It is about coding practices, and how Smarty silently switched off our NOTICES reporting, that is set in php.ini, in a very sneaky way.

Turning off notices in PHP is always a bad idea. Our code is no more cluttered with notices turned off (calling "isset" is not clutter), and it helped us find many "silent" bugs (like typos in var names, and such).

But this is up to the developer. What shouldn't be up to the developer is silently changing the error reporting level inside a lib, used by other projects, and affecting the said projects error reporting (we have many smarty template functions, which are called from within the templates, and the notices were obviously turned off in all of those).
Back to top
View user's profile Send private message
douglassdavis
Smarty Junkie


Joined: 21 Jan 2008
Posts: 541

PostPosted: Tue Oct 19, 2010 2:44 pm    Post subject: Reply with quote

And I forgot another annoying notice:

Notice: Undefined index

When trying to echo or compare an item from an array where the index is not set, when substituting null or comparable value when an index is undefined works just fine and is actually the intended behavior.

Oftentimes you can't even know in advance what those indexes are going to be.
Back to top
View user's profile Send private message
mohrt
Administrator


Joined: 16 Apr 2003
Posts: 7368
Location: Lincoln Nebraska, USA

PostPosted: Tue Oct 19, 2010 2:47 pm    Post subject: Reply with quote

I completely understand. The reality is that Smarty is used by hundreds of thousands, and accessing unset vars in the templates throwing notices brings on the same questions over and over. And over and over. The answer should be obvious, no? We cannot know at compile time if a var is always set, and testing with isset() on every variable is a slight performance penalty that adds up.

At some point you have to take the path of least resistance. So we have a choice: leave error reporting alone and constantly answer FAQs (which they answer is to mask E_NOTICE or test isset() themselves), or take away E_NOTICE inside template rendering by default, and just deal with the occasional PHP purist (which there is an answer for, set your own error reporting.)

But as I mentioned earlier, Smarty 3 takes a different approach to variable rendering than Smarty 2, and we may be able to get rid of the E_NOTICE mask. We haven't focused on that yet.

Similar argument: PHP has loosely typed variables, oh no! How can you sleep not knowing your variable types? Smile
Back to top
View user's profile Send private message Visit poster's website
U.Tews
Administrator


Joined: 22 Nov 2006
Posts: 5068
Location: Hamburg / Germany

PostPosted: Tue Oct 19, 2010 4:11 pm    Post subject: Reply with quote

The SVN has been updated.

Smarty3 does no longer change the error reporting level (masking out E_NOTICE) during template processing, unless you specify something in the
$smarty->error_reporting property
Back to top
View user's profile Send private message
mohrt
Administrator


Joined: 16 Apr 2003
Posts: 7368
Location: Lincoln Nebraska, USA

PostPosted: Tue Oct 19, 2010 4:12 pm    Post subject: Reply with quote

well that was easy Wink

assigned vars were already addressed, we tested and addressed special vars such as {$smarty.session.foo} and some other edge cases. However, properties within your own assigned objects will throw notices, but that should be expected.
Back to top
View user's profile Send private message Visit poster's website
carloslage
Smarty n00b


Joined: 18 Oct 2010
Posts: 4

PostPosted: Tue Oct 19, 2010 6:21 pm    Post subject: Reply with quote

Thanks, that's a very sensible solution.

We appreciate that you listened and made a change you could've easily dismissed as "useless".
Back to top
View user's profile Send private message
U.Tews
Administrator


Joined: 22 Nov 2006
Posts: 5068
Location: Hamburg / Germany

PostPosted: Wed Oct 20, 2010 6:35 pm    Post subject: Reply with quote

My first update did not work for all cases when processing arrays in the template.

There is another update in the SVN.
Back to top
View user's profile Send private message
gtraxx
Smarty Regular


Joined: 08 Jan 2008
Posts: 56

PostPosted: Fri Nov 12, 2010 9:34 am    Post subject: Reply with quote

I download the update from svn and set the error reporting in the configuration but it does not work since the stable version: (
Code:
$this->error_reporting = error_reporting() &~E_NOTICE;


My error with new config :
Quote:
E_NOTICE: Undefined index: getLanguage
Back to top
View user's profile Send private message
mohrt
Administrator


Joined: 16 Apr 2003
Posts: 7368
Location: Lincoln Nebraska, USA

PostPosted: Fri Nov 12, 2010 2:44 pm    Post subject: Reply with quote

gtraxx wrote:
I download the update from svn and set the error reporting in the configuration but it does not work since the stable version: (
Code:
$this->error_reporting = error_reporting() &~E_NOTICE;


My error with new config :
Quote:
E_NOTICE: Undefined index: getLanguage


This will be fixed in 3.0.1
Back to top
View user's profile Send private message Visit poster's website
gtraxx
Smarty Regular


Joined: 08 Jan 2008
Posts: 56

PostPosted: Fri Nov 12, 2010 5:03 pm    Post subject: Reply with quote

Great,
I also send a fix to the google group for smarty_internal_compile_continue:
http://groups.google.com/group/smarty-developers/browse_thread/thread/2857df55361d894e?hl=en
Back to top
View user's profile Send private message
Display posts from previous:   
This forum is locked: you cannot post, reply to, or edit topics.   This topic is locked: you cannot edit posts or make replies.    Smarty Forum Index -> Smarty 3 All times are GMT
Goto page 1, 2  Next
Page 1 of 2

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group
Protected by Anti-Spam ACP