GatewayLog module improvements

* Switch hash tests from ref() to UNIVSERAL::isa() to keep blessed hashes from producing false negatives. * Move major failure log writes from debug to global error. * Modified DESTROY to call log_it() via eval {} to quiet any impact of a failed call on parent code and ensure that any unexpected failure is logged to the global error log.
interchange · Nov 4, 2017 · c5f8ad0 · c5f8ad0
1 parent 871c475
commit c5f8ad0
Showing 1 changed file with 20 additions and 9 deletions.
diff --git a/lib/Vend/Payment/GatewayLog.pm b/lib/Vend/Payment/GatewayLog.pm
@@ -25,7 +25,7 @@ sub init {
         my $host = `hostname -s`;
         chomp ($self->{_source} = $host);
     }
-    
+
     return 1;
 }
 
@@ -73,7 +73,7 @@ sub request {
     my $request = shift;
     return $self->{__request}
         unless $request;
-    unless (ref ($request) eq 'HASH') {
+    unless (UNIVERSAL::isa($request, 'HASH')) {
         ::logDebug(
             'Skipping non-HASH request set: received %s (unevals to %s)',
             $request,
@@ -91,7 +91,7 @@ sub response {
     my $response = shift;
     return $self->{__response}
         unless $response;
-    unless (ref ($response) eq 'HASH') {
+    unless (UNIVERSAL::isa($response, 'HASH')) {
         ::logDebug(
             'Skipping non-HASH response set: received %s (unevals to %s)',
             $response,
@@ -135,7 +135,7 @@ sub write {
 
     if ($@) {
         my $err = $@;
-        ::logDebug(
+        ::logGlobal(
             q{Couldn't write to %s: %s -- request: %s -- response: %s},
             $self->table,
             $err,
@@ -152,22 +152,33 @@ sub write {
 }
 
 sub table {
-    return shift()->{_log_table};
+    return shift->{_log_table};
 }
 
 sub _enabled {
-    return shift()->{_enabled};
+    return shift->{_enabled};
 }
 
 sub source {
-    return shift()->{_source};
+    return shift->{_source};
 }
 
 sub DESTROY {
     my $self = shift;
+
     return 1 unless $self->_enabled;
-#::logDebug('Logging request to database in destructor');
-    $self->log_it;
+
+    # Unexpected order of operations causes any evals
+    # within DESTROY to execute after any eval that may
+    # contain the DESTROYed object. Effect is that $@ upon
+    # exit of the containing eval is *not* controlled by
+    # that eval. Localization contains the effect.
+    local $@;
+
+    eval { $self->log_it };
+    ::logGlobal("log_it eval died: $@")
+        if $@;
+
     1;
 }