140460 - ThrowTemplateError doesn't print content-type when necessary

Status: RESOLVED WONTFIX

(Bugzilla :: Bugzilla-General, defect, P3)

Description

[Myk Melez [:myk] [@mykmelez]]

ThrowTemplateError is designed with the assumption that the content type will
always already have been written to the client, since templates are most
commonly used to generate and return output simultaneously.  There is at least
one case, however (in the patch for bug 138904), in which a template is
processed before the content type is returned, which causes an ugly web server
configuration error to be displayed instead of the actual error that occured if
there is an error processing the template.

ThrowTemplateError should take an optional parameter specifying whether or not
the content type was already returned.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Instead of adding a parameter ThrowTemplateError, we should abstract out HTTP
header generation (a bug for this already exists) and set a global flag when
generation is complete.  Then we can check that flag in this function like we
currently check $vars->{'header_done'} in the other error-throwing functions
(replacing the code in those functions in the process).

Comment 2

[Gervase Markham [:gerv]]

Myk: are you suggesting we do that for 2.16? The other option is to
ThrowCodeError in this one particular case. 

Also, are we not trying to move away from communicating via global variables,
for mod_perl reasons?

Gerv

Comment 3

[Matthew Tuck [:CodeMachine]]

I don't think ThrowTemplateError is appropriate here.  ThrowTemplateError is
really only appropriate for HTML output to the browser, right?

Comment 4

[Gervase Markham [:gerv]]

Originally, the difference between ThrowTemplateError and ThrowCodeError was
that ThrowTemplateError a) didn't use a template and b) didn't print a
Content-Type. However, half of that's going to change (bug 140006) so that it
tries a template first; this makes it much more like ThrowCodeError. 

So, I suggest we use ThrowCodeError for template error arms in the few cases
where we want, or might want, a Content Type (it checks $vars->{'header_done'}).
This is a bit inconsistent, but not really - in the single case we've had of
this so far, it's because we are using the TT in passing as a useful formatting
mechanism, rather than its usual use. So it's not too bad.

Gerv

Comment 5

[Myk Melez [:myk] [@mykmelez]]

>in the single case we've had of
>this so far, it's because we are using the TT in passing as a useful formatting
>mechanism, rather than its usual use. So it's not too bad.

This is going to become more common, so it won't be a bit inconsistent for long.
 We should fix ThrowTemplateError the way I suggest in comment 1, but the 2.16
fix is to use ThrowCodeError.

Comment 6

[Matthew Tuck [:CodeMachine]]

It is OK I suppose, but please make it have a default value or separate sub so
we don't have to enter an extra parameter just for the uncommon case.

And to throw a spanner in the works, with email templates (at least, with
process_bug) there will be a content type AND content already output.

How does ThrowTemplateError know what the content type is has to display is anyway?

Comment 7

[Joel Peshkin]

These unloved bugs have been sitting untouched since June 2002 or longer.  If
nobody does anything else to them, they certainly won't make 2.18
Retargetting to 2.20.  If you really plan to push them right now, you might pull
them back in.

Comment 8

[Max Kanat-Alexander]

This bug has not been touched by its owner in over six months, even though it is
targeted to 2.20, for which the freeze is 10 days away. Unsetting the target
milestone, on the assumption that nobody is actually working on it or has any
plans to soon.

If you are the owner, and you plan to work on the bug, please give it a real
target milestone. If you are the owner, and you do *not* plan to work on it,
please reassign it to [e-mail] or a .bugs component owner. If you are
*anybody*, and you get this comment, and *you* plan to work on the bug, please
reassign it to yourself if you have the ability.

Comment 9

[Dave Miller [:justdave]]

Reassigning bugs that I'm not actively working on to the default component owner
in order to try to make some sanity out of my personal buglist.  This doesn't
mean the bug isn't being dealt with, just that I'm not the one doing it.  If you
are dealing with this bug, please assign it to yourself.

Comment 10

[Frédéric Buclin]

I don't remember having seen this problem in the last 10 years. So this fix is probably not necessary.