166023 - On failure in template->new, a template is used to display error

Status: RESOLVED FIXED

(Bugzilla :: Installation & Upgrading, defect)

Description

[Joel Peshkin]

Bugs such as bug 165133 show failures in checksetup that derive from the fact
that globals.pl uses DisplayError() to show errors in Template->new  

Since DisplayError() will never work if Template->new fails, this should be a
die() instead.

Comment 1

[Joel Peshkin]

Attachment: Replace DisplayError with die

I could not duplicate the problem that complaining users have

Comment 2

[Bradley Baetz (:bbaetz)]

Comment on attachment 97446 [details] [diff] [review]
Replace DisplayError with die

r=bbaetz x2

Technically, DisplayError will display text if it can't handle the template,
but that needs a bit of work to avoid trying to call the method on an undefined
object.

That work will fall out of modulisation, anyway, so we shouldn't care about it
now.

Comment 3

[Gervase Markham [:gerv]]

If a template fails, you should be using ThrowTemplateError($template->error).
This tries to report the error using another template (fine if e.g. the first
template is missing) and then falls back on "print" statements. 

There is documentation in the code near the *Error functions on which ones to
use when.

bug 165133 is caused by the fact that we don't define the *Error functions early
enough. That needs fixing first.

Gerv

Comment 4

[Bradley Baetz (:bbaetz)]

gerv: Yes, but:

$template->process(...)|| ThrowTemplateError(...)

will crash if $template is undef.

Comment 5

[Gervase Markham [:gerv]]

> $template->process(...)|| ThrowTemplateError(...)
> 
> will crash if $template is undef.

Indeed. OK, I get it now. die seems fine in this one case. Sorry :-)

Gerv

Comment 6

[Joel Peshkin]

Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.199; previous revision: 1.198
done             

Comment 7

[J. Paul Reed [:preed]]

I'm in IRC odd hours now, so I'll just ask in the bug: what's the holdup on
checking this into the 2.16 branch?

Comment 8

[Joel Peshkin]

Didn't realize it was waiting for me...

Checked into 2.16 Branch:

Checking in globals.pl;                                     
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.169.2.11; previous revision: 1.169.2.10
done