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

RESOLVED FIXED in Bugzilla 2.18

Status

()

Bugzilla
Installation & Upgrading
RESOLVED FIXED
16 years ago
5 years ago

People

(Reporter: Joel Peshkin, Assigned: Joel Peshkin)

Tracking

2.16
Bugzilla 2.18
x86
Other

Details

(Whiteboard: [fixed for 2.16.1])

Attachments

(1 attachment)

(Assignee)

Description

16 years ago
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.
(Assignee)

Updated

16 years ago
Blocks: 165133
Status: NEW → ASSIGNED
Target Milestone: --- → Bugzilla 2.16
(Assignee)

Comment 1

16 years ago
Created attachment 97446 [details] [diff] [review]
Replace DisplayError with die


I could not duplicate the problem that complaining users have
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.
Attachment #97446 - Flags: review+
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
gerv: Yes, but:

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

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

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

Gerv

(Assignee)

Comment 6

16 years ago

Checking in globals.pl;
/cvsroot/mozilla/webtools/bugzilla/globals.pl,v  <--  globals.pl
new revision: 1.199; previous revision: 1.198
done             
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
Whiteboard: [want for 2.16.1]
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18

Comment 7

16 years ago
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?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 8

16 years ago
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                                                   
Status: REOPENED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED

Updated

16 years ago
Whiteboard: [want for 2.16.1] → [fixed for 2.16.1]
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.