Stas Bekman wrote:
Yes, this $@ save/restore logic could be added to filters only. But, still,
in the case of a failing response handler & filter handler, you'd lose the
error from the filter.
At least, here is a patch that does the same thing, but only for filters.
The problem with filters is that Apache has no provision for error handling. So may be we should at least Perl_warn it, before restoring ERRSV?
Attached patch moves $@ save/restoring to filter logic only, and does warn in the case of a filter error detected while there was already another known error before entering the filter
Maybe we need to be smarter about this a bit and somehow concatenate the errors ?
Sounds like a good idea. But may be later.
Something like APR::Error supporting aggregated errors maybe ?
-- -------------------------------------------------------------------------------- Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5 http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5
Index: Changes
===================================================================
RCS file: /home/cvs/modperl-2.0/Changes,v
retrieving revision 1.431
diff -u -I$Id -r1.431 Changes
--- Changes 9 Aug 2004 21:42:34 -0000 1.431
+++ Changes 9 Aug 2004 23:02:44 -0000
@@ -12,6 +12,9 @@
=item 1.99_15-dev
+Filters should not reset $@ if it was already set before
+invocation [Gozer]
+
Apache::compat server_root_relative now correctly handles absolute
paths like ap_server_root_relative does [Gozer]
Index: src/modules/perl/modperl_filter.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_filter.c,v
retrieving revision 1.93
diff -u -I$Id -r1.93 modperl_filter.c
--- src/modules/perl/modperl_filter.c 2 Jun 2004 21:35:58 -0000 1.93
+++ src/modules/perl/modperl_filter.c 9 Aug 2004 23:02:45 -0000
@@ -432,6 +432,7 @@
int modperl_run_filter(modperl_filter_t *filter)
{
AV *args = Nullav;
+ SV *errsv = Nullsv;
int status;
modperl_handler_t *handler =
((modperl_filter_ctx_t *)filter->f->ctx)->handler;
@@ -443,6 +444,15 @@
MP_dINTERP_SELECT(r, c, s);
+ if (SvTRUE(ERRSV)) { /* Save current value of $@ if set */
+ errsv = newSVsv(ERRSV);
+ MP_TRACE_f(MP_FUNC, "Saving [EMAIL PROTECTED]'%s' for %s filter",
+ SvPVX(errsv),
+ modperl_handler_name(handler)
+ );
+ }
+
+
modperl_handler_make_args(aTHX_ &args,
"Apache::Filter", filter->f,
"APR::Brigade",
@@ -503,6 +513,23 @@
MP_TRACE_f(MP_FUNC, MP_FILTER_NAME_FORMAT
"return: %d\n", modperl_handler_name(handler), status);
+ if (errsv) {
+ if (SvTRUE(ERRSV)) { /* Existing error + filter error */
+ /* Make sure we at least log a filter error */
+ Perl_warn(aTHX_ "%s", SvPVX(ERRSV));
+ MP_TRACE_f(MP_FUNC, "Filter error '%s' for %s",
+ SvPVX(ERRSV),
+ modperl_handler_name(handler)
+ );
+ }
+
+ sv_setsv(ERRSV, errsv);
+ MP_TRACE_f(MP_FUNC, "Restoring [EMAIL PROTECTED]'%s' for %s filter",
+ SvPVX(errsv),
+ modperl_handler_name(handler)
+ );
+ }
+
return status;
}
Index: t/filter/TestFilter/out_str_eval.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/t/filter/TestFilter/out_str_eval.pm,v
retrieving revision 1.3
diff -u -I$Id -r1.3 out_str_eval.pm
--- t/filter/TestFilter/out_str_eval.pm 3 Jul 2004 18:45:46 -0000 1.3
+++ t/filter/TestFilter/out_str_eval.pm 9 Aug 2004 23:02:45 -0000
@@ -25,9 +25,8 @@
sub response {
my $r = shift;
- plan $r, tests => 1, todo => [1];
- # XXX: see if we can fix filter handlers to restore the original
- # $@ when the callback completes
+ plan $r, tests => 1;
+ #Filters do not reset $@
eval { i_do_not_exist_really_i_do_not() };
# trigger the filter invocation, before using $@
$r->print("# whatever");
Index: todo/release
===================================================================
RCS file: /home/cvs/modperl-2.0/todo/release,v
retrieving revision 1.41
diff -u -I$Id -r1.41 release
--- todo/release 9 Aug 2004 21:42:35 -0000 1.41
+++ todo/release 9 Aug 2004 23:02:45 -0000
@@ -8,11 +8,6 @@
http://marc.theaimsgroup.com/?l=apache-modperl-dev&m=108967266419527&w=2
owner: gozer
-* filters reset $@ generated by eval, see if we can fix that. The TODO
- test: TestFilter::out_str_eval presents the case
- The description is here:
- http://marc.theaimsgroup.com/?l=apache-modperl-dev&m=108639632031457&w=2
-
* on windows $pool->clean, followed by $pool->destroy breaks other tests
See test TestAPR::pool
http://marc.theaimsgroup.com/?l=apache-modperl-dev&m=108547894817083&w=2
signature.asc
Description: OpenPGP digital signature
