Replace eval with try/catch

https://progress.opensuse.org/issues/176862

Author: b10n1k

lib/OpenQA/Assets.pm

@@ -11,6 +11,7 @@ use Mojo::File qw(path);
 use Mojo::Home;
 use Mojolicious::Plugin::AssetPack;
 use YAML::PP qw(LoadFile);
+use Feature::Compat::Try;
 
 sub setup ($server) {
     # setup asset pack, note that the config file is shared with tools/generate-packed-assets
@@ -21,14 +22,14 @@ sub setup ($server) {
 
     # -> read assets/assetpack.def
     local $SIG{CHLD};
-    eval { $server->asset->process };
-    if (my $assetpack_error = $@) {    # uncoverable statement
-        $assetpack_error    # uncoverable statement
+    try { $server->asset->process }
+    catch ($e) {
+        $e    # uncoverable statement
           .= 'If you invoked this service for development (from a Git checkout) you probably just need to'
           . ' invoke "make node_modules" before running this service. If you invoked this service via a packaged binary/service'
           . " then there is probably a problem with the packaging.\n"
-          if $assetpack_error =~ qr/could not find input asset.*node_modules/i;    # uncoverable statement
-        die $assetpack_error;    # uncoverable statement
+          if $e =~ qr/could not find input asset.*node_modules/i;    # uncoverable statement
+        die $e;    # uncoverable statement
     }
 }
 

lib/OpenQA/CacheService/Model/Cache.pm

@@ -3,6 +3,7 @@
 
 package OpenQA::CacheService::Model::Cache;
 use Mojo::Base -base, -signatures;
+use Feature::Compat::Try;
 
 use Carp 'croak';
 use Capture::Tiny 'capture_merged';
@@ -13,6 +14,7 @@ use OpenQA::Downloader;
 use Mojo::File 'path';
 use Mojo::Util 'scope_guard';
 use Time::HiRes qw(gettimeofday);
+use Feature::Compat::Try;
 
 # Only consider files larger than 250 MB for metrics (rates for smaller files are unrealistic)
 use constant METRICS_DOWNLOAD_SIZE => $ENV{OPENQA_METRICS_DOWNLOAD_SIZE} // 262144000;
@@ -46,14 +48,14 @@ sub repair_database ($self, $db_file = $self->_locate_db_file) {
     # perform integrity check and test migration; try to provoke an error
     my $log = $self->log;
     $log->debug("Testing sqlite database ($db_file)");
-    eval {
+    try {
         die "database integrity check failed\n" if $self->_check_database_integrity;
         $self->sqlite->migrations->migrate;
-    };
+    }
 
     # remove broken database
-    if (my $err = $@) {
-        $log->error("Database has been corrupted: $err");
+    catch ($e) {
+        $log->error("Database has been corrupted: $e");
         $log->error('Killing processes accessing the database file handles and removing database');
         $self->_kill_db_accessing_processes("'$db_file'*");
     }
@@ -158,15 +160,15 @@ sub asset ($self, $asset) {
 }
 
 sub track_asset ($self, $asset) {
-    eval {
+    try {
         my $db = $self->sqlite->db;
         my $tx = $db->begin('exclusive');
         my $sql = "INSERT INTO assets (filename, size, last_use) VALUES (?, 0, strftime('%s','now'))"
           . 'ON CONFLICT (filename) DO UPDATE SET pending=1';
         $db->query($sql, $asset);
         $tx->commit;
-    };
-    if (my $err = $@) { $self->log->error("Tracking asset failed: $err") }
+    }
+    catch ($e) { $self->log->error("Tracking asset failed: $e") }
 }
 
 sub metrics ($self) {
@@ -270,8 +272,9 @@ sub _exceeds_limit ($self, $needed) {
         return 1 if $self->{cache_real_size} + $needed > $limit;
     }
     if (my $min_free_percentage = $self->min_free_percentage) {
-        my ($available_bytes, $total_bytes) = eval { check_df $self->location };
-        if (my $error = $@) { chomp $error; $self->log->error($error); return 0 }
+        my ($available_bytes, $total_bytes);
+        try { ($available_bytes, $total_bytes) = check_df $self->location }
+        catch ($e) { chomp $e; $self->log->error($e); return 0 }
         return 1 if ($available_bytes - $needed) / $total_bytes * 100 < $min_free_percentage;
     }
     return 0;
@@ -287,7 +290,7 @@ sub _check_limits ($self, $needed, $to_preserve = undef) {
         my $limit_size = human_readable_size($limit);
         $log->info(
             "Cache size $cache_size + needed $needed_size exceeds limit of $limit_size, purging least used assets");
-        eval {
+        try {
             my $results
               = $self->sqlite->db->select('assets', [qw(filename size last_use)], {pending => '0'},
                 {-asc => 'last_use'});
@@ -300,22 +303,22 @@ sub _check_limits ($self, $needed, $to_preserve = undef) {
                 $self->_decrease($asset_size) if $self->purge_asset($filename);
                 last if !$self->_exceeds_limit($needed);
             }
-        };
-        if (my $err = $@) { $log->error("Checking cache limit failed: $err") }
+        }
+        catch ($e) { $log->error("Checking cache limit failed: $e") }
     }
 }
 
 sub _delete_pending_assets ($self) {
     my $log = $self->log;
-    eval {
+    try {
         my $results = $self->sqlite->db->select('assets', [qw(filename pending)], {pending => '1'});
         for my $asset ($results->hashes->each) {
             my $filename = $asset->{filename};
             $log->info(qq{Purging "$filename" because it appears pending after service startup});
             $self->purge_asset($filename);
         }
-    };
-    if (my $err = $@) { $log->error("Checking for pending leftovers failed: $err") }
+    }
+    catch ($e) { $log->error("Checking for pending leftovers failed: $e") }
 }
 
 1;

lib/OpenQA/CacheService/Model/Downloads.pm

@@ -3,7 +3,7 @@
 
 package OpenQA::CacheService::Model::Downloads;
 use Mojo::Base -base, -signatures;
-
+use Feature::Compat::Try;
 use Carp 'croak';
 
 # Two days
@@ -12,7 +12,7 @@ use constant CLEANUP_AFTER => 172800;
 has 'cache';
 
 sub add ($self, $lock, $job_id) {
-    eval {
+    try {
         my $db = $self->cache->sqlite->db;
         my $tx = $db->begin('exclusive');
 
@@ -21,8 +21,8 @@ sub add ($self, $lock, $job_id) {
         $db->insert('downloads', {lock => $lock, job_id => $job_id});
 
         $tx->commit;
-    };
-    if (my $err = $@) { croak "Couldn't add download: $err" }
+    }
+    catch ($e) { croak "Couldn't add download: $e" }
 }
 
 sub find ($self, $lock) {

lib/OpenQA/Client/Upload.pm

@@ -8,6 +8,7 @@ use OpenQA::File;
 use Carp qw(croak);
 use Mojo::Asset::Memory;
 use Mojo::File qw(path);
+use Feature::Compat::Try;
 
 sub _upload_asset_fail ($self, $uri, $form) {
     $form->{state} = 'fail';
@@ -57,7 +58,7 @@ sub asset ($self, $job_id, $opts) {
         do {
             $retries-- if $retries > 0;
             my $tx;
-            eval {
+            try {
                 my $form = {
                     file => {filename => $file_name, file => Mojo::Asset::Memory->new->add_chunk($part->serialize)},
                     asset => $opts->{asset},
@@ -69,10 +70,10 @@ sub asset ($self, $job_id, $opts) {
                     my $json = $tx->res->json;
                     $done = 1 if $json && $json->{status} && $json->{status} eq 'ok';
                 }
-            };
-            if (my $error = $@) {
-                $self->emit('upload_chunk.request_err', $tx, $error);
-                $final_error = $error;
+            }
+            catch ($e) {
+                $self->emit('upload_chunk.request_err', $tx, $e);
+                $final_error = $e;
             }
 
             unless ($done) {

lib/OpenQA/Downloader.pm

@@ -10,6 +10,7 @@ use Mojo::URL;
 use OpenQA::UserAgent;
 use OpenQA::Utils 'human_readable_size';
 use Time::HiRes 'sleep';
+use Feature::Compat::Try;
 
 has attempts => 5;
 has [qw(log tmpdir)];
@@ -48,7 +49,8 @@ sub _extract_asset ($self, $to_extract, $target) {
     my $cmd;
     if ($to_extract =~ qr/\.tar(\..*)?/) {
         # invoke bsdtar to extract (compressed) tar archives
-        eval { $target->make_path } or return $@;
+        try { $target->make_path }
+        catch ($e) { return $e }
         $cmd = "bsdtar -x --directory '$target' -f '$to_extract' 2>&1";
     }
     else {
@@ -113,7 +115,8 @@ sub _get ($self, $url, $target, $options) {
             if ($err) {
                 $ret = $code;
                 $log->error(qq{Extracting "$tempfile" failed: $err});
-                eval { $target->remove_tree } or $log->error("Unable to remove leftovers after failed extraction: $@");
+                try { $target->remove_tree }
+                catch ($e) { $log->error("Unable to remove leftovers after failed extraction: $e") }
             }
         }
         else { $asset->move_to($target) }

lib/OpenQA/Git.pm

@@ -9,6 +9,7 @@ use Cwd 'abs_path';
 use Mojo::File 'path';
 use OpenQA::Utils qw(run_cmd_with_log_return_error run_cmd_with_log);
 use OpenQA::App;
+use Feature::Compat::Try;
 
 has 'app';
 has 'dir';
@@ -180,8 +181,11 @@ sub cache_ref ($self, $ref, $url, $relative_path, $output_file, $allow_arbitrary
     # checkout git versioned file <$relative_path> of from repo at version <$ref> and write it to <$output_file>
     # returns undef on success - else the error message
     if (-f $output_file) {
-        eval { path($output_file)->touch };
-        return $@ ? $@ : undef;
+        try {
+            path($output_file)->touch;
+            return undef;
+        }
+        catch ($e) { return $e }
     }
 
     $self->app->log->debug(

lib/OpenQA/JobSettings.pm

@@ -10,6 +10,7 @@ use Mojo::URL;
 use Mojo::Util 'url_unescape';
 use OpenQA::Log 'log_debug';
 use OpenQA::Utils qw(asset_type_from_setting get_url_short);
+use Feature::Compat::Try;
 
 sub generate_settings ($params) {
     my $settings = $params->{settings};
@@ -56,10 +57,10 @@ sub expand_placeholders ($settings, $on_web_ui = 1) {
     for my $value (values %$settings) {
         next unless defined $value;
         my %visited_placeholders;
-        eval {
+        try {
             $value =~ s/(%+)(\w+)(%+)/_expand_placeholder($settings, $2, $1, $3, \%visited_placeholders, $on_web_ui)/eg;
-        };
-        return "Error: $@" if $@;
+        }
+        catch ($e) { return "Error: $e" }
     }
     return undef;
 }

lib/OpenQA/Log.pm

@@ -15,6 +15,7 @@ use OpenQA::App;
 use Time::Moment;
 use File::Spec::Functions 'catfile';
 use Sys::Hostname;
+use Feature::Compat::Try;
 
 our $VERSION = '0.0.1';
 our @EXPORT_OK = qw(
@@ -116,8 +117,11 @@ sub _log_via_mojo_app ($level, $msg) {
 }
 
 sub _try_logging_to_channel ($level, $msg, $channel) {
-    eval { $channel->$level($msg); };
-    return ($@ ? 0 : 1);
+    try {
+        $channel->$level($msg);
+        return 1
+    }
+    catch ($e) { return 0 }
 }
 
 sub _log_to_stderr_or_stdout ($level, $msg) {

lib/OpenQA/Parser.pm

@@ -19,6 +19,7 @@ use constant DATA_FIELD => '__data__';
 use constant TYPE_FIELD => '__type__';
 use OpenQA::Utils 'walker';
 use Exporter 'import';
+use Feature::Compat::Try;
 
 our @EXPORT_OK = qw(parser p);
 
@@ -40,8 +41,8 @@ sub _build_parser {
         if (my $e = load_class $parser_name) {
             croak ref $e ? "Exception: $e" : 'Parser not found!';
         }
-        eval { $p_instance = $parser_name->new(@args); };
-        croak "Invalid parser supplied: $@" if $@;
+        try { $p_instance = $parser_name->new(@args); }
+        catch ($e) { croak "Invalid parser supplied: $e" }
     }
     $p_instance;
 }
@@ -141,7 +142,7 @@ sub restore_el {
 
 sub restore_tree_section {
     my $ref = shift;
-    eval {
+    try {
         walker $ref => sub {
             my ($key, $value, $keys) = @_;
             my $hash = $ref;
@@ -155,8 +156,8 @@ sub restore_tree_section {
             $hash->{$key} = restore_el($value) if reftype $hash eq 'HASH';
             $hash->[$key] = restore_el($value) if reftype $hash eq 'ARRAY';
         };
-    };
-    confess $@ if $@;
+    }
+    catch ($e) { confess $e }
 }
 
 sub _load_tree {
@@ -165,21 +166,17 @@ sub _load_tree {
     my $tree = shift;
     my @coll = sort keys %{$tree};
 
-    {
-        local $@;
-        eval {
-            foreach my $collection (@coll) {
-                if (ref $tree->{$collection} eq 'ARRAY') {
-                    $self->$collection->add(restore_el($_)) for @{$tree->{$collection}};
-                }
-                else {
-                    $self->{$collection} = restore_el($tree->{$collection});
-                }
+    try {
+        foreach my $collection (@coll) {
+            if (ref $tree->{$collection} eq 'ARRAY') {
+                $self->$collection->add(restore_el($_)) for @{$tree->{$collection}};
             }
-        };
-        confess "Failed parsing tree: $@" if $@;
+            else {
+                $self->{$collection} = restore_el($tree->{$collection});
+            }
+        }
     }
-
+    catch ($e) { confess "Failed parsing tree: $e" }
     return $self;
 }
 

lib/OpenQA/Resource/Locks.pm

@@ -6,6 +6,7 @@ use Mojo::Base -strict, -signatures;
 
 use OpenQA::Jobs::Constants;
 use OpenQA::Schema;
+use Feature::Compat::Try;
 
 my %final_states = map { $_ => 1 } OpenQA::Jobs::Constants::NOT_OK_RESULTS();
 
@@ -94,8 +95,8 @@ sub barrier_create ($name = undef, $jobid = undef, $expected_jobs = undef) {
     return 0 if $barriers && $barriers->single;
     my $dbh = OpenQA::Schema->singleton->storage->dbh;
     my $sth = $dbh->prepare('INSERT INTO job_locks (name, owner, count) VALUES (?, ?, ?) ON CONFLICT DO NOTHING');
-    eval { $sth->execute($name, $jobid, $expected_jobs) };
-    die "Unable to create barrier for job $jobid with name '$name': $@" if $@;
+    try { $sth->execute($name, $jobid, $expected_jobs) }
+    catch ($e) { die "Unable to create barrier for job $jobid with name '$name': $e" }
     return $sth->rows > 0;
 }