Do I need to trap errors in my calls to Win32::OLE->LastError?

Question

[EDIT] - with the benefit of hindsight, this question was misdirected. I have not deleted it because it is a good example of the incorrect use of eval and correct criticism by Perl::Critic.

Perl Critic raises the following criticism for the code below:

Return value of eval not tested. You can't depend upon the value of $@/$EVAL_ERROR to tell whether an eval failed

my $Jet = Win32::OLE->CreateObject('DAO.DBEngine.36')
    or croak "Can't create Jet database engine.";
my $DB = $Jet->OpenDatabase($DBFile)

# code omitted for the sake of brevity
# perl script writes results to Access db via an append query
$DB->Execute( $SQLquery, 128 );                       #128=DBFailOnError

eval {$err = Win32::OLE->LastError()} ; #<<<< PROBLEM LINE SEE FEEDBACK BELOW  
if ( $err){
     print $ERROR "WIN32::OLE raised an exception: $err\n";
     Win32::OLE->LastError(0);  # this clears your error
}

My thinking is that I am using eval to detect the existence of the error object and on the Win32:OLE module to detects the error and reports it.

Am I safe to ignore the criticism?

Answer 1

Leaving aside the perl-critic issuse, your code does not make much sense.

The Win32::OLE docs explain when exceptions will be thrown (and how you can automatically catch them).

LastError just gives you information about an error after it has occurred assuming your program has not died. Wrapping it in eval is pointless.

Update: I would have written something along the following lines (untested because I am on Linux with no access to Windows right now):

use strict;
use warnings;

use Carp;

use Win32;
use Win32::OLE;

$Win32::OLE::Warn = 3;

# No need for this eval if you are OK with the default error message    
my $Jet = eval {
    Win32::OLE->CreateObject('DAO.DBEngine.36')
} or croak sprintf(
    "Can't create Jet database engine: %s", 
    win32_error_message(Win32::OLE->LastError)
);

# No need for this eval if you are OK with the default error message
my $DB = eval {
    $Jet->OpenDatabase($DBFile)
} or croak sprintf(
    "Can't open database '$DBFile': %s",
    win32_error_message(Win32::OLE->LastError)
);

my $result = eval {
    $DB->Execute( $SQLquery, 128 )
};

unless (defined $result) {
    print $ERROR win32_error_message(Win32::OLE->LastError);
    Win32::OLE->LastError(0);
}

Answer 2

The meaning of that message is detailed in the documentation. In short, it tells you to not rely on $@ alone after an eval, but to also check the return value of eval.

However, in your case, the problem is that you aren't checking either the return value of eval nor are you checking $@ and moreover it seems that your use of eval is completely superfluous because the method you are calling shouldn't be throwing any exceptions.