Re: Connected Callback Errors Ignored

On Nov 15, 2013, at 3:12 AM, Tim Bunce <[email protected]> wrote:

> You'd need to write the callback code in the way you'd naturally write
> it when not using RaiseError. Which typically means something like:
> 
>    $dbh->do('SET search_path = ?', undef, 'foo')
>        or return;

That will prevent any further code from executing, but does not give me a chance to handle the error.

>> I’m also finding it means I can’t do error handling in the callback.
> 
> I'm not sure what you mean there.

I *always* use RaiseError => 1 (or HandleError). Never ever check return values.

>> I have to do this to get it to ignore errors:
>> 
>>                        $dbh->do('SET search_path = ?', undef, 'foo');
>>                        $dbh->set_err(undef) if $dbh->err;
>> 
>> I feel a little dirty.
> 
> Ignoring errors isn't a common action.

Oh, I agree, this is an edge case in that sense. The issue for me is more the inconsistency. I have RaiseError set, but I cannot actually catch the error and handle it where it happens, even though I have RaiseError set to true. I have to know that callbacks are different, and either check return values to handle errors (and call set_err(undef) if I have handled them in the callback), or check return values and return (on 1.630 or later) to let an external error handler catch the error. IOW, It’s a userland function but I have to write it like an internal DBI function. It’s inconsistent UE.

> Probably. There are three main down-sides to consider:
> 1. compatibility - the risk of breaking some existing callback code

Seems unlikely, I think, since it’s not documented that it’s not the user’s handle that is passed to callbacks. I think I have been using callbacks longer than anyone and never noticed. Frankly I’m amazed at how few people know they’re there (I regularly get requests to add them to DBIx::Connector).

> 2. performance - the cost of getting the outer handle
> 3. pure-perl compatibility
> 
> There is an undocumented DBI::_handles function that returns the inner
> and outer handles for a given (inner or outer) handle. Perl code within
> DBI.pm, such as the connect_cached callback logic, would need to call
> that (or something like it) to get the outer handle to pass to the callback.

Yeah. That’s not a ton over overhead, is it? Is an additional method call really likely to have much of a performance impact?

> The DBI::_handles implementation in lib/DBI/PurePerl.pm doesn't support
> getting the outer handle from an inner one. That's probably fixable by
> adding a weakref.
> 
> I don't have a strong opinion on this.

I am strongly in favor of providing a consistent interface to users. Currently, error-handling in callbacks is inconsistent when using RaiseError or HandleError.

I can carve out a little time today to write some test cases if it’ll help.

Thanks,

David