diff --git a/MANIFEST b/MANIFEST index 28b38eb1a..a62312754 100644 --- a/MANIFEST +++ b/MANIFEST @@ -29,6 +29,7 @@ lib/Zonemaster/Backend/DB.pm lib/Zonemaster/Backend/DB/MySQL.pm lib/Zonemaster/Backend/DB/PostgreSQL.pm lib/Zonemaster/Backend/DB/SQLite.pm +lib/Zonemaster/Backend/Errors.pm lib/Zonemaster/Backend/RPCAPI.pm lib/Zonemaster/Backend/TestAgent.pm lib/Zonemaster/Backend/Translator.pm diff --git a/lib/Zonemaster/Backend/DB.pm b/lib/Zonemaster/Backend/DB.pm index 36f5e5984..09875a82c 100644 --- a/lib/Zonemaster/Backend/DB.pm +++ b/lib/Zonemaster/Backend/DB.pm @@ -51,7 +51,8 @@ sub get_db_class { sub user_exists { my ( $self, $user ) = @_; - die "username not provided to the method user_exists\n" unless ( $user ); + die Zonemaster::Backend::Error::Internal->new( reason => "username not provided to the method user_exists") + unless ( $user ); return $self->user_exists_in_db( $user ); } @@ -59,14 +60,16 @@ sub user_exists { sub add_api_user { my ( $self, $username, $api_key ) = @_; - die "username or api_key not provided to the method add_api_user\n" - unless ( $username && $api_key ); + die Zonemaster::Backend::Error::Internal->new( reason => "username or api_key not provided to the method add_api_user") + unless ( $username && $api_key ); - die "User already exists\n" if ( $self->user_exists( $username ) ); + die Zonemaster::Backend::Error::Conflict->new( message => 'User already exists', data => { username => $username } ) + if ( $self->user_exists( $username ) ); my $result = $self->add_api_user_to_db( $username, $api_key ); - die "add_api_user_to_db not successful\n" unless ( $result ); + die Zonemaster::Backend::Error::Internal->new( reason => "add_api_user_to_db not successful") + unless ( $result ); return $result; } diff --git a/lib/Zonemaster/Backend/DB/MySQL.pm b/lib/Zonemaster/Backend/DB/MySQL.pm index ef607b51f..d95070993 100644 --- a/lib/Zonemaster/Backend/DB/MySQL.pm +++ b/lib/Zonemaster/Backend/DB/MySQL.pm @@ -10,6 +10,7 @@ use DBI qw(:utils); use JSON::PP; use Zonemaster::Backend::Validator qw( untaint_ipv6_address ); +use Zonemaster::Backend::Errors; with 'Zonemaster::Backend::DB'; @@ -128,7 +129,7 @@ sub user_authorized { sub create_new_batch_job { my ( $self, $username ) = @_; - my ( $batch_id, $creaton_time ) = $self->dbh->selectrow_array( " + my ( $batch_id, $creation_time ) = $self->dbh->selectrow_array( " SELECT batch_id, batch_jobs.creation_time AS batch_creation_time @@ -142,7 +143,8 @@ sub create_new_batch_job { LIMIT 1 ", undef, $username ); - die "You can't create a new batch job, job:[$batch_id] started on:[$creaton_time] still running \n" if ( $batch_id ); + die Zonemaster::Backend::Error::Conflict->new( message => 'Batch job still running', data => { batch_id => $batch_id, creation_time => $creation_time } ) + if ( $batch_id ); $self->dbh->do( "INSERT INTO batch_jobs (username) VALUES(?)", undef, $username ); my ( $new_batch_id ) = $self->dbh->{mysql_insertid}; @@ -228,12 +230,16 @@ sub get_test_params { my ( $params_json ) = $self->dbh->selectrow_array( "SELECT params FROM test_results WHERE hash_id=?", undef, $test_id ); + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) + unless defined $params_json; + my $result; eval { $result = decode_json( $params_json ); }; - warn "decoding of params_json failed (testi_id: [$test_id]):".Dumper($params_json) if $@; + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) + if $@; return decode_json( $params_json ); } @@ -249,8 +255,22 @@ sub test_results { my $result; my ( $hrefs ) = $self->dbh->selectall_hashref( "SELECT id, hash_id, CONVERT_TZ(`creation_time`, \@\@session.time_zone, '+00:00') AS creation_time, params, results FROM test_results WHERE hash_id=?", 'hash_id', undef, $test_id ); $result = $hrefs->{$test_id}; - $result->{params} = decode_json( $result->{params} ); - $result->{results} = decode_json( $result->{results} ); + + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) + unless defined $result; + + eval { + $result->{params} = decode_json( $result->{params} ); + + if (defined $result->{results}) { + $result->{results} = decode_json( $result->{results} ); + } else { + $result->{results} = []; + } + }; + + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) + if $@; return $result; } @@ -370,7 +390,7 @@ sub add_batch_job { $dbh->{AutoCommit} = 1; } else { - die "User $params->{username} not authorized to use batch mode\n"; + die Zonemaster::Backend::Error::PermissionDenied->new( message => 'User not authorized to use batch mode', data => { username => $params->{username}} ); } return $batch_id; diff --git a/lib/Zonemaster/Backend/DB/PostgreSQL.pm b/lib/Zonemaster/Backend/DB/PostgreSQL.pm index 908e2aa3c..1177066a7 100644 --- a/lib/Zonemaster/Backend/DB/PostgreSQL.pm +++ b/lib/Zonemaster/Backend/DB/PostgreSQL.pm @@ -10,6 +10,7 @@ use Encode; use JSON::PP; use Zonemaster::Backend::DB; +use Zonemaster::Backend::Errors; with 'Zonemaster::Backend::DB'; @@ -153,8 +154,8 @@ sub create_new_batch_job { test_results.progress<>100 LIMIT 1 ", undef, $username ); - - die "You can't create a new batch job, job:[$batch_id] started on:[$creation_time] still running \n" if ( $batch_id ); + die Zonemaster::Backend::Error::Conflict->new( message => 'Batch job still running', data => { batch_id => $batch_id, creation_time => $creation_time } ) + if ( $batch_id ); my ( $new_batch_id ) = $dbh->selectrow_array( "INSERT INTO batch_jobs (username) VALUES (?) RETURNING id", undef, $username ); @@ -207,8 +208,14 @@ sub get_test_params { my $dbh = $self->dbh; my ( $params_json ) = $dbh->selectrow_array( "SELECT params FROM test_results WHERE hash_id=?", undef, $test_id ); + + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) + unless defined $params_json; + eval { $result = decode_json( encode_utf8( $params_json ) ); }; - die "$@ \n" if $@; + + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) + if $@; return $result; } @@ -222,27 +229,36 @@ sub test_results { if ( $results ); my $result; - eval { - my ( $hrefs ) = $dbh->selectall_hashref( "SELECT id, hash_id, creation_time at time zone current_setting('TIMEZONE') at time zone 'UTC' as creation_time, params, results FROM test_results WHERE hash_id=?", 'hash_id', undef, $test_id ); - $result = $hrefs->{$test_id}; + my ( $hrefs ) = $dbh->selectall_hashref( "SELECT id, hash_id, creation_time at time zone current_setting('TIMEZONE') at time zone 'UTC' as creation_time, params, results FROM test_results WHERE hash_id=?", 'hash_id', undef, $test_id ); + $result = $hrefs->{$test_id}; + + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) + unless defined $result; + eval { # This workaround is needed to properly handle all versions of perl and the DBD::Pg module # More details in the zonemaster backend issue #570 if (utf8::is_utf8($result->{params}) ) { - $result->{params} = decode_json( encode_utf8($result->{params}) ); + $result->{params} = decode_json( encode_utf8($result->{params}) ); } else { - $result->{params} = decode_json( $result->{params} ); + $result->{params} = decode_json( $result->{params} ); } - if (utf8::is_utf8($result->{results} ) ) { + if (defined $result->{results} ) { + if (utf8::is_utf8($result->{results} ) ) { $result->{results} = decode_json( encode_utf8($result->{results}) ); - } - else { + } + else { $result->{results} = decode_json( $result->{results} ); + } + } else { + $result->{results} = []; } }; - die "$@ \n" if $@; + + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id }) + if $@; return $result; } @@ -342,7 +358,7 @@ sub add_batch_job { $dbh->commit(); } else { - die "User $params->{username} not authorized to use batch mode\n"; + die Zonemaster::Backend::Error::PermissionDenied->new( message => 'User not authorized to use batch mode', data => { username => $params->{username}} ); } return $batch_id; diff --git a/lib/Zonemaster/Backend/DB/SQLite.pm b/lib/Zonemaster/Backend/DB/SQLite.pm index 7dc784e3c..72919151e 100644 --- a/lib/Zonemaster/Backend/DB/SQLite.pm +++ b/lib/Zonemaster/Backend/DB/SQLite.pm @@ -11,6 +11,8 @@ use Digest::MD5 qw(md5_hex); use JSON::PP; use Log::Any qw( $log ); +use Zonemaster::Backend::Errors; + with 'Zonemaster::Backend::DB'; has 'dbh' => ( @@ -145,7 +147,7 @@ sub user_authorized { sub create_new_batch_job { my ( $self, $username ) = @_; - my ( $batch_id, $creaton_time ) = $self->dbh->selectrow_array( " + my ( $batch_id, $creation_time ) = $self->dbh->selectrow_array( " SELECT batch_id, batch_jobs.creation_time AS batch_creation_time @@ -158,7 +160,8 @@ sub create_new_batch_job { LIMIT 1 " ); - die "You can't create a new batch job, job:[$batch_id] started on:[$creaton_time] still running \n" if ( $batch_id ); + die Zonemaster::Backend::Error::Conflict->new( message => 'Batch job still running', data => { batch_id => $batch_id, creation_time => $creation_time } ) + if ( $batch_id ); $self->dbh->do("INSERT INTO batch_jobs (username) VALUES(" . $self->dbh->quote( $username ) . ")" ); my ( $new_batch_id ) = $self->dbh->sqlite_last_insert_rowid; @@ -244,12 +247,16 @@ sub get_test_params { my ( $params_json ) = $self->dbh->selectrow_array( "SELECT params FROM test_results WHERE hash_id=?", undef, $test_id ); + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) + unless defined $params_json; + my $result; eval { $result = decode_json( $params_json ); }; - $log->warn( "decoding of params_json failed (test_id: [$test_id]):".Dumper($params_json) ) if $@; + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) + if $@; return $result; } @@ -265,8 +272,22 @@ sub test_results { my $result; my ( $hrefs ) = $self->dbh->selectall_hashref( "SELECT id, hash_id, creation_time, params, results FROM test_results WHERE hash_id=?", 'hash_id', undef, $test_id ); $result = $hrefs->{$test_id}; - $result->{params} = decode_json( $result->{params} ); - $result->{results} = decode_json( $result->{results} ); + + die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) + unless defined $result; + + eval { + $result->{params} = decode_json( $result->{params} ); + + if (defined $result->{results}) { + $result->{results} = decode_json( $result->{results} ); + } else { + $result->{results} = []; + } + }; + + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) + if $@; return $result; } @@ -363,7 +384,7 @@ sub add_batch_job { $dbh->{AutoCommit} = 1; } else { - die "User $params->{username} not authorized to use batch mode\n"; + die Zonemaster::Backend::Error::PermissionDenied->new( message => 'User not authorized to use batch mode', data => { username => $params->{username}} ); } return $batch_id; diff --git a/lib/Zonemaster/Backend/Errors.pm b/lib/Zonemaster/Backend/Errors.pm new file mode 100644 index 000000000..cc25cf877 --- /dev/null +++ b/lib/Zonemaster/Backend/Errors.pm @@ -0,0 +1,157 @@ +package Zonemaster::Backend::Error; +use Moose; +use Data::Dumper; + + +has 'message' => ( + is => 'ro', + isa => 'Str', + required => 1, +); + +has 'code' => ( + is => 'ro', + isa => 'Int', + required => 1, +); + +has 'data' => ( + is => 'ro', + isa => 'Any', + default => undef, +); + +sub as_hash { + my $self = shift; + my $error = { + code => $self->code, + message => $self->message, + error => ref($self), + }; + $error->{data} = $self->data if defined $self->data; + return $error; +} + +sub as_string { + my $self = shift; + my $str = sprintf "%s (code %d).", $self->message, $self->code; + if (defined $self->data) { + $str .= sprintf " Context: %s", $self->_data_dump; + } + return $str; +} + +sub _data_dump { + my $self = shift; + local $Data::Dumper::Indent = 0; + local $Data::Dumper::Terse = 1; + my $data = Dumper($self->data); + $data =~ s/[\n\r]/ /g; + return $data ; +} + +package Zonemaster::Backend::Error::Internal; +use Moose; + +extends 'Zonemaster::Backend::Error'; + +has '+message' => ( + default => 'Internal server error' +); + +has '+code' => ( + default => -32603 +); + +has 'reason' => ( + isa => 'Str', + is => 'ro' +); + +has 'method' => ( + is => 'ro', + isa => 'Str', + builder => '_build_method' +); + +sub _build_method { + my $s = 0; + while (my @c = caller($s)) { + $s ++; + last if $c[3] eq 'Moose::Object::new'; + } + my @c = caller($s); + if ($c[3] =~ /^(.*)::handle_exception$/ ) { + @c = caller(++$s); + } + + return $c[3]; +} + +around 'BUILDARGS', sub { + my ($orig, $class, %args) = @_; + + if(exists $args{reason}) { + # trim new lines + $args{reason} =~ s/\n/ /g; + $args{reason} =~ s/^\s+|\s+$//g; + } + + $class->$orig(%args); +}; + +sub as_string { + my $self = shift; + my $str = sprintf "Caught %s in the `%s` method: %s", ref($self), $self->method, $self->reason; + if (defined $self->data) { + $str .= sprintf " Context: %s", $self->_data_dump; + } + return $str; +} + + +package Zonemaster::Backend::Error::ResourceNotFound; +use Moose; + +extends 'Zonemaster::Backend::Error'; + +has '+message' => ( + default => 'Resource not found' +); + +has '+code' => ( + default => -32000 +); + +package Zonemaster::Backend::Error::PermissionDenied; +use Moose; + +extends 'Zonemaster::Backend::Error'; + +has '+message' => ( + default => 'Permission denied' +); + +has '+code' => ( + default => -32001 +); + +package Zonemaster::Backend::Error::Conflict; +use Moose; + +extends 'Zonemaster::Backend::Error'; + +has '+message' => ( + default => 'Conflicting resource' +); + +has '+code' => ( + default => -32002 +); + +package Zonemaster::Backend::Error::JsonError; +use Moose; + +extends 'Zonemaster::Backend::Error::Internal'; + +1; diff --git a/lib/Zonemaster/Backend/RPCAPI.pm b/lib/Zonemaster/Backend/RPCAPI.pm index fae0d9b31..b36a05813 100644 --- a/lib/Zonemaster/Backend/RPCAPI.pm +++ b/lib/Zonemaster/Backend/RPCAPI.pm @@ -26,6 +26,7 @@ use Zonemaster::Backend; use Zonemaster::Backend::Config; use Zonemaster::Backend::Translator; use Zonemaster::Backend::Validator; +use Zonemaster::Backend::Errors; my $zm_validator = Zonemaster::Backend::Validator->new; my %json_schemas; @@ -43,7 +44,7 @@ sub new { bless( $self, $type ); if ( ! $params || ! $params->{config} ) { - handle_exception('new', "Missing 'config' parameter", '001'); + handle_exception("Missing 'config' parameter"); } $self->{config} = $params->{config}; @@ -67,18 +68,27 @@ sub _init_db { my $dbclass = Zonemaster::Backend::DB->get_db_class( $dbtype ); $self->{db} = $dbclass->from_config( $self->{config} ); }; + if ($@) { - handle_exception('_init_db', "Failed to initialize the [$dbtype] database backend module: [$@]", '002'); + handle_exception("Failed to initialize the [$dbtype] database backend module: [$@]"); } } sub handle_exception { - my ( $method, $exception, $exception_id ) = @_; + my ( $exception ) = @_; + + if ( !$exception->isa('Zonemaster::Backend::Error') ) { + my $reason = $exception; + $exception = Zonemaster::Backend::Error::Internal->new( reason => $reason ); + } + + if ( $exception->isa('Zonemaster::Backend::Error::Internal') ) { + $log->error($exception->as_string); + } else { + $log->info($exception->as_string); + } - $exception =~ s/\n/ /g; - $exception =~ s/^\s+|\s+$//g; - $log->error("Internal error $exception_id: Unexpected error in the $method API call: [$exception]"); - die "Internal error $exception_id \n"; + die $exception->as_hash; } $json_schemas{version_info} = joi->object->strict; @@ -92,7 +102,7 @@ sub version_info { }; if ($@) { - handle_exception('version_info', $@, '003'); + handle_exception( $@ ); } return \%ver; @@ -105,7 +115,7 @@ sub profile_names { my %profiles; eval { %profiles = $self->{config}->PUBLIC_PROFILES }; if ( $@ ) { - handle_exception( 'profile_names', $@, '004' ); + handle_exception( $@ ); } return [ keys %profiles ]; @@ -146,7 +156,7 @@ sub get_host_by_name { }; if ($@) { - handle_exception('get_host_by_name', $@, '005'); + handle_exception( $@ ); } return \@adresses; @@ -196,7 +206,7 @@ sub get_data_from_parent_zone { return \%result; }; if ($@) { - handle_exception('get_data_from_parent_zone', $@, '006'); + handle_exception( $@ ); } elsif ($result) { return $result; @@ -290,7 +300,7 @@ $extra_validators{start_domain_test} = sub { return @errors; }; if ($@) { - handle_exception('start_domain_test_validate_syntax', $@, '008'); + handle_exception( $@ ); } else { return @errors; @@ -330,7 +340,7 @@ sub start_domain_test { $result = $self->{db}->create_new_test( $params->{domain}, $params, $self->{config}->ZONEMASTER_age_reuse_previous_test ); }; if ($@) { - handle_exception('start_domain_test', $@, '009'); + handle_exception( $@ ); } return $result; @@ -348,7 +358,7 @@ sub test_progress { $result = $self->{db}->test_progress( $test_id ); }; if ($@) { - handle_exception('test_progress', $@, '010'); + handle_exception( $@ ); } return $result; @@ -368,7 +378,7 @@ sub get_test_params { $result = $self->{db}->get_test_params( $test_id ); }; if ($@) { - handle_exception('get_test_params', $@, '011'); + handle_exception( $@ ); } return $result; @@ -409,7 +419,7 @@ sub get_test_results { my $previous_locale = $translator->locale; if ( !$translator->locale( $locale ) ) { - handle_exception( 'get_test_results', "Failed to set locale: $locale", '017' ); + handle_exception( "Failed to set locale: $locale" ); } eval { $translator->data } if $translator; # Provoke lazy loading of translation data @@ -468,7 +478,7 @@ sub get_test_results { $result->{results} = \@zm_results; }; if ($@) { - handle_exception('get_test_results', $@, '012'); + handle_exception( $@ ); } $translator->locale( $previous_locale ); @@ -500,7 +510,7 @@ sub get_test_history { $results = $self->{db}->get_test_history( $params ); }; if ($@) { - handle_exception('get_test_history', $@, '013'); + handle_exception( $@ ); } return $results; @@ -529,7 +539,7 @@ sub add_api_user { } }; if ($@) { - handle_exception('add_api_user', $@, '014'); + handle_exception( $@ ); } return $result; @@ -569,7 +579,7 @@ sub add_batch_job { $results = $self->{db}->add_batch_job( $params ); }; if ($@) { - handle_exception('add_batch_job', $@, '015'); + handle_exception( $@ ); } return $results; @@ -589,7 +599,7 @@ sub get_batch_job_result { $result = $self->{db}->get_batch_job_result($batch_id); }; if ($@) { - handle_exception('get_batch_job_result', $@, '016'); + handle_exception( $@ ); } return $result;