PrintError won"t turn off

Just finished a rather irritating debugging session trying to track
down a warning triggered by DBIx::Timeout. Turns out this code
doesn't exactly do what I mean:

$dbh->{PrintError} = 0;

Looks like it turns PrintError off, huh? Actually, as far as I can
tell it turns it on. The relevent C code is (from dbih_set_attr_k):

else if (strEQ(key, "PrintError")) {
DBIc_set(imp_xxh,DBIcf_PrintError, on);

In contrast to attributes that you can turn off, which look like:

else if (strEQ(key, "Warn")) {
(on) ? DBIc_WARN_on(imp_xxh) : DBIc_WARN_off(imp_xxh);

Is this a bug, or is there some reason for this behavior?

-sam

Re: PrintError won"t turn off

On Mon, 18 Sep 2006, Sam Tregar wrote:

> Looks like it turns PrintError off, huh? Actually, as far as I can
> tell it turns it on. The relevent C code is (from dbih_set_attr_k):
>
> else if (strEQ(key, "PrintError")) {
> DBIc_set(imp_xxh,DBIcf_PrintError, on);

Sheesh, ignore me. I see that's using "on", which should toggle it on
and off. I wonder why it doesn't seem to be working.

-sam

Re: PrintError won"t turn off

On Mon, 18 Sep 2006, Sam Tregar wrote:

> On Mon, 18 Sep 2006, Sam Tregar wrote:
>
>> Looks like it turns PrintError off, huh? Actually, as far as I can
>> tell it turns it on. The relevent C code is (from dbih_set_attr_k):
>>
>> else if (strEQ(key, "PrintError")) {
>> DBIc_set(imp_xxh,DBIcf_PrintError, on);
>
> Sheesh, ignore me. I see that's using "on", which should toggle it on
> and off. I wonder why it doesn't seem to be working.

Is it possible that connect_cached() resets PrintError if it's not
explicitely set in the options hash? That would explain what I'm
seeing. I can set PrintError off and see its effect, but as soon as
some other code gets the same handle via connect_cached() PrintError
is on again.

-sam

Re: PrintError won"t turn off

On Mon, Sep 18, 2006 at 05:05:52PM -0400, Sam Tregar wrote:
> On Mon, 18 Sep 2006, Sam Tregar wrote:
>
> >On Mon, 18 Sep 2006, Sam Tregar wrote:
> >
> >>Looks like it turns PrintError off, huh? Actually, as far as I can
> >>tell it turns it on. The relevent C code is (from dbih_set_attr_k):
> >>
> >> else if (strEQ(key, "PrintError")) {
> >> DBIc_set(imp_xxh,DBIcf_PrintError, on);
> >
> >Sheesh, ignore me. I see that's using "on", which should toggle it on
> >and off. I wonder why it doesn't seem to be working.
>
> Is it possible that connect_cached() resets PrintError if it's not
> explicitely set in the options hash? That would explain what I'm
> seeing. I can set PrintError off and see its effect, but as soon as
> some other code gets the same handle via connect_cached() PrintError
> is on again.

Yes, and the same applies to AutoCommit. Whether it's a good thing or
not is debatable but it's been that way forever. The 'workaround' is
to explicitly state the attributes you want.

(But even then some would argue that it's unreasonable for
connect/connect_cached to re-apply those attribute settings to an
already cached handle. You can argue that one either way.)

Tim.

Re: PrintError won"t turn off

On Mon, 18 Sep 2006, Tim Bunce wrote:

> Yes, and the same applies to AutoCommit. Whether it's a good thing or
> not is debatable but it's been that way forever. The 'workaround' is
> to explicitly state the attributes you want.

Hmmm, interesting. What would you think about making PrintError
default to 0 when RaiseError is on? My reasoning is that if the error
is getting croak()ed then there's no need to warn() it too. The
warn() comes without a stack-trace and can be very difficult to track
down when the croak() is being intentionally trapped (as was the case
in DBIx::Timeout). When the croak() isn't being trapped it's just
duplicate information on STDERR.

-sam

Re: PrintError won"t turn off

On Mon, Sep 18, 2006 at 10:30:49PM -0400, Sam Tregar wrote:
> On Mon, 18 Sep 2006, Tim Bunce wrote:
>
> >Yes, and the same applies to AutoCommit. Whether it's a good thing or
> >not is debatable but it's been that way forever. The 'workaround' is
> >to explicitly state the attributes you want.
>
> Hmmm, interesting. What would you think about making PrintError
> default to 0 when RaiseError is on? My reasoning is that if the error
> is getting croak()ed then there's no need to warn() it too. The
> warn() comes without a stack-trace and can be very difficult to track
> down when the croak() is being intentionally trapped (as was the case
> in DBIx::Timeout). When the croak() isn't being trapped it's just
> duplicate information on STDERR.

There's some interesting history here. For some time the DBI used to
automatically silence PrintError if RaiseError was set (I can't now
remember if it only happened if RaiseError wasn't being caught - and
that edge case is part what made be remove the 'cleverness').

Rather than try to be 'not quite so clever' again I'm tempted to say
that people should be explicit and ask for exactly what they want.

Tim.

p.s. Your "warn() comes without a stack-trace" comment sparked an idea.
The DBI tracing mechanism can already show one level of stack trace
so it's trivial to do the same for PrintError & RaiseError:

@@ -3256,6 +3256,13 @@
}
}

+ if (1) {
+ COP *cop = dbi_caller_cop();
+ if (cop) {
+ dbi_caller_string(msg, cop, " called via ", Nullch, 1, 1, 0);
+ }
+ }
+
if ( SvTRUE(err_sv)
&& DBIc_has(imp_xxh, DBIcf_HandleError)
&& (hook_svp = hv_fetch((HV*)SvRV(h),"HandleError",11,0))

Then the messages look something like this:

DBD::ExampleP::st execute failed: 2 values bound when 1 expected [for Statement "select mode from ?" with ParamValues: 1='first', 2='second'] called via 10examp.t line 123 at t/10examp.t line 456.

Worth doing?