From 04222e04529515b5571315a1d7366968e6ce231d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Wed, 11 Aug 2021 11:56:31 +0200 Subject: [PATCH 1/8] use logger instead of warn --- lib/Zonemaster/Backend/RPCAPI.pm | 3 ++- script/zonemaster_backend_rpcapi.psgi | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/Zonemaster/Backend/RPCAPI.pm b/lib/Zonemaster/Backend/RPCAPI.pm index 4153cd14e..efbc70e0a 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 Log::Any qw( $log ); my $zm_validator = Zonemaster::Backend::Validator->new; my %json_schemas; @@ -77,7 +78,7 @@ sub handle_exception { $exception =~ s/\n/ /g; $exception =~ s/^\s+|\s+$//g; - warn "Internal error $exception_id: Unexpected error in the $method API call: [$exception] \n"; + $log->error("Internal error $exception_id: Unexpected error in the $method API call: [$exception]"); die "Internal error $exception_id \n"; } diff --git a/script/zonemaster_backend_rpcapi.psgi b/script/zonemaster_backend_rpcapi.psgi index 3ce397708..bae147650 100644 --- a/script/zonemaster_backend_rpcapi.psgi +++ b/script/zonemaster_backend_rpcapi.psgi @@ -45,15 +45,20 @@ Log::Any::Adapter->set( 'Screen', min_level => get_loglevel(), stderr => 1, + newline => 1, callbacks => sub { my %args = @_; - $args{message} = sprintf "%s [%d] %s - %s\n", strftime( "%FT%TZ", gmtime ), $PID, uc $args{level}, $args{message}; + $args{message} = sprintf "%s [%d] %s - %s", strftime( "%FT%TZ", gmtime ), $PID, uc $args{level}, $args{message}; }, ], ] ), ); +$SIG{__WARN__} = sub { + $log->warning(map { my $m = $_; $m =~ s/\n/ /g; $m =~ s/^\s+|\s+$//g; $m } @_); +}; + my $config = Zonemaster::Backend::Config->load_config(); builder { From 3317d809328cbc2911a19a52f312bc4c99e4143c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Wed, 11 Aug 2021 18:14:44 +0200 Subject: [PATCH 2/8] make the stdout logger match the file logger --- script/zonemaster_backend_testagent | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/script/zonemaster_backend_testagent b/script/zonemaster_backend_testagent index de8c20d30..58b75b4f6 100755 --- a/script/zonemaster_backend_testagent +++ b/script/zonemaster_backend_testagent @@ -78,9 +78,10 @@ sub log_dispatcher_dup_stdout { 'Handle', handle => $handle, min_level => $min_level, + newline => 1, callbacks => sub { my %args = @_; - $args{message} = sprintf "%s: %s\n", uc $args{level}, $args{message}; + $args{message} = sprintf "%s [%d] %s - %s", strftime( "%FT%TZ", gmtime ), $PID, uc $args{level}, $args{message}; }, ], ] @@ -99,15 +100,20 @@ sub log_dispatcher_file { filename => $log_file, mode => '>>', min_level => $min_level, + newline => 1, callbacks => sub { my %args = @_; - $args{message} = sprintf "%s [%d] %s - %s\n", strftime( "%FT%TZ", gmtime ), $PID, uc $args{level}, $args{message}; + $args{message} = sprintf "%s [%d] %s - %s", strftime( "%FT%TZ", gmtime ), $PID, uc $args{level}, $args{message}; }, ], ] ); } +$SIG{__WARN__} = sub { + $log->warning(map { my $m = $_; $m =~ s/\n/ /g; $m =~ s/^\s+|\s+$//g; $m } @_); +}; + ### ### Actual functionality ### From af3b17305d31694de7cdeacdab9e715886cd87b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Thu, 12 Aug 2021 14:48:37 +0200 Subject: [PATCH 3/8] add errors module --- lib/Zonemaster/Backend/DB/PostgreSQL.pm | 31 ++++-- lib/Zonemaster/Backend/Errors.pm | 141 ++++++++++++++++++++++++ lib/Zonemaster/Backend/RPCAPI.pm | 22 ++-- script/zonemaster_db_exporter.psgi | 35 ++++++ 4 files changed, 214 insertions(+), 15 deletions(-) create mode 100644 lib/Zonemaster/Backend/Errors.pm create mode 100644 script/zonemaster_db_exporter.psgi diff --git a/lib/Zonemaster/Backend/DB/PostgreSQL.pm b/lib/Zonemaster/Backend/DB/PostgreSQL.pm index 908e2aa3c..6cda68f3a 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'; @@ -207,8 +208,12 @@ 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::Internal->new( reason => "$@", data => { test_id => $test_id } ) if $@; return $result; } @@ -224,25 +229,35 @@ sub test_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}; + $result = $hrefs->{$test_id}; + }; + + die Zonemaster::Backend::Error::Internal->new( reason => "$@", data => { test_id => $test_id } ) if $@; + 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::Internal->new( reason => "$@", data => { test_id => $test_id }) if $@; return $result; } diff --git a/lib/Zonemaster/Backend/Errors.pm b/lib/Zonemaster/Backend/Errors.pm new file mode 100644 index 000000000..068e67046 --- /dev/null +++ b/lib/Zonemaster/Backend/Errors.pm @@ -0,0 +1,141 @@ +package Zonemaster::Backend::Error; +use Moose; +use Data::Dumper; + + +has 'message' => ( + is => 'rw', + isa => 'Str', + required => 1, +); + +has 'code' => ( + is => 'rw', + isa => 'Int', + required => 1, +); + +has 'data' => ( + is => 'rw', + isa => 'Any', + default => undef, +); + +sub as_hash { + my $self = shift; + my $error = { + code => $self->code, + message => $self->message + }; + $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 => 'rw', + initializer => 'reason', +); + +has 'method' => ( + is => 'rw', + isa => 'Str', + builder => '_build_method' +); + +has 'id' => ( + is => 'rw', + isa => 'Int', + default => 0, +); + +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 'reason' => sub { + my $orig = shift; + my $self = shift; + + my ( $value, $setter, $attr ) = @_; + + # reader + return $self->$orig if not $value; + + # trim new lines + $value =~ s/\n/ /g; + $value =~ s/^\s+|\s+$//g; + + # initializer + return $setter->($value) if $setter; + + # writer + $self->$orig($value); +}; + + +sub as_string { + my $self = shift; + my $str = sprintf "Internal error %0.3d: Unexpected error in the `%s` method: [%s]", $self->id, $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 +); + +1; diff --git a/lib/Zonemaster/Backend/RPCAPI.pm b/lib/Zonemaster/Backend/RPCAPI.pm index efbc70e0a..8245c3783 100644 --- a/lib/Zonemaster/Backend/RPCAPI.pm +++ b/lib/Zonemaster/Backend/RPCAPI.pm @@ -26,7 +26,7 @@ use Zonemaster::Backend; use Zonemaster::Backend::Config; use Zonemaster::Backend::Translator; use Zonemaster::Backend::Validator; -use Log::Any qw( $log ); +use Zonemaster::Backend::Errors; my $zm_validator = Zonemaster::Backend::Validator->new; my %json_schemas; @@ -68,18 +68,26 @@ 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'); + if ($@) { } } sub handle_exception { - my ( $method, $exception, $exception_id ) = @_; + my ( $_method, $exception, $exception_id ) = @_; + + if ( !$exception->isa('Zonemaster::Backend::Error') ) { + my $reason = $exception; + $exception = Zonemaster::Backend::Error::Internal->new(reason => $reason, id => $exception_id); + } + + if ( $exception->isa('Zonemaster::Backend::Error::Internal') ) { + $log->error($exception->as_string); + } else { + $log->notice($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; diff --git a/script/zonemaster_db_exporter.psgi b/script/zonemaster_db_exporter.psgi new file mode 100644 index 000000000..bb7144a2d --- /dev/null +++ b/script/zonemaster_db_exporter.psgi @@ -0,0 +1,35 @@ +#!/usr/bin/env perl +use strict; +use warnings; + +use Zonemaster::Backend::Config; +use Zonemaster::Backend::DB; +use Net::Prometheus; + +my $client = Net::Prometheus->new; + +my $tests_gauge = $client->new_gauge( + name => 'zonemaster_tests_total', + help => 'Total number of tests', + labels => [ 'state' ], +); + +my $config = Zonemaster::Backend::Config->load_config(); +my $dbtype = $config->DB_engine; +my $dbclass = Zonemaster::Backend::DB->get_db_class( $dbtype ); +my $db = $dbclass->from_config( $config ); + +my $prom_app = $client->psgi_app; + +sub { + my $queued = $db->dbh->selectrow_hashref('SELECT count(*) from test_results WHERE progress = 0'); + $tests_gauge->labels('queued')->set($queued->{count}); + + my $finished = $db->dbh->selectrow_hashref('SELECT count(*) from test_results WHERE progress = 100'); + $tests_gauge->labels('finished')->set($finished->{count}); + + my $running = $db->dbh->selectrow_hashref('SELECT count(*) from test_results WHERE progress > 0 and progress < 100'); + $tests_gauge->labels('running')->set($running->{count}); + + $prom_app->(@_); +}; From 14625325a94ba71c937ca531d6d651b08bae0f30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Thu, 12 Aug 2021 15:20:28 +0200 Subject: [PATCH 4/8] improve errors --- lib/Zonemaster/Backend/DB/MySQL.pm | 21 ++++++++++++++++++--- lib/Zonemaster/Backend/DB/PostgreSQL.pm | 11 ++++------- lib/Zonemaster/Backend/DB/SQLite.pm | 22 +++++++++++++++++++--- lib/Zonemaster/Backend/Errors.pm | 23 +++++++++++++++++++++-- lib/Zonemaster/Backend/RPCAPI.pm | 3 ++- 5 files changed, 64 insertions(+), 16 deletions(-) diff --git a/lib/Zonemaster/Backend/DB/MySQL.pm b/lib/Zonemaster/Backend/DB/MySQL.pm index ef607b51f..473ce396d 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'; @@ -228,12 +229,14 @@ 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 +252,20 @@ 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; } diff --git a/lib/Zonemaster/Backend/DB/PostgreSQL.pm b/lib/Zonemaster/Backend/DB/PostgreSQL.pm index 6cda68f3a..b249b26a8 100644 --- a/lib/Zonemaster/Backend/DB/PostgreSQL.pm +++ b/lib/Zonemaster/Backend/DB/PostgreSQL.pm @@ -213,7 +213,7 @@ sub get_test_params { eval { $result = decode_json( encode_utf8( $params_json ) ); }; - die Zonemaster::Backend::Error::Internal->new( reason => "$@", data => { test_id => $test_id } ) if $@; + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id } ) if $@; return $result; } @@ -227,12 +227,9 @@ 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::Internal->new( reason => "$@", data => { test_id => $test_id } ) if $@; die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $result; eval { @@ -257,7 +254,7 @@ sub test_results { } }; - die Zonemaster::Backend::Error::Internal->new( reason => "$@", data => { test_id => $test_id }) if $@; + die Zonemaster::Backend::Error::JsonError->new( reason => "$@", data => { test_id => $test_id }) if $@; return $result; } diff --git a/lib/Zonemaster/Backend/DB/SQLite.pm b/lib/Zonemaster/Backend/DB/SQLite.pm index 7dc784e3c..ecea51f89 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' => ( @@ -244,12 +246,14 @@ 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 +269,20 @@ 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; } diff --git a/lib/Zonemaster/Backend/Errors.pm b/lib/Zonemaster/Backend/Errors.pm index 068e67046..903579d2c 100644 --- a/lib/Zonemaster/Backend/Errors.pm +++ b/lib/Zonemaster/Backend/Errors.pm @@ -25,7 +25,8 @@ sub as_hash { my $self = shift; my $error = { code => $self->code, - message => $self->message + message => $self->message, + error => ref($self), }; $error->{data} = $self->data if defined $self->data; return $error; @@ -114,10 +115,23 @@ around 'reason' => sub { $self->$orig($value); }; +around 'as_hash' => sub { + my $orig = shift; + my $self = shift; + + my $href = $self->$orig; + + $href->{exception_id} = $self->id; + $href->{reason} = $self->reason; + $href->{method} = $self->method; + + return $href; +}; + sub as_string { my $self = shift; - my $str = sprintf "Internal error %0.3d: Unexpected error in the `%s` method: [%s]", $self->id, $self->method, $self->reason; + my $str = sprintf "Internal error %0.3d (%s): Unexpected error in the `%s` method: [%s]", $self->id, ref($self), $self->method, $self->reason; if (defined $self->data) { $str .= sprintf "; Context: %s", $self->_data_dump; } @@ -138,4 +152,9 @@ has '+code' => ( default => -32000 ); +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 8245c3783..9f1a32bf0 100644 --- a/lib/Zonemaster/Backend/RPCAPI.pm +++ b/lib/Zonemaster/Backend/RPCAPI.pm @@ -68,8 +68,9 @@ sub _init_db { my $dbclass = Zonemaster::Backend::DB->get_db_class( $dbtype ); $self->{db} = $dbclass->from_config( $self->{config} ); }; - handle_exception('_init_db', "Failed to initialize the [$dbtype] database backend module: [$@]", '002'); + if ($@) { + handle_exception('_init_db', "Failed to initialize the [$dbtype] database backend module: [$@]", '002'); } } From 6484d04bca52ca1b215c55e113c1000911f04929 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Thu, 12 Aug 2021 16:03:28 +0200 Subject: [PATCH 5/8] change log level --- lib/Zonemaster/Backend/DB/SQLite.pm | 2 +- lib/Zonemaster/Backend/RPCAPI.pm | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Zonemaster/Backend/DB/SQLite.pm b/lib/Zonemaster/Backend/DB/SQLite.pm index ecea51f89..3568080ab 100644 --- a/lib/Zonemaster/Backend/DB/SQLite.pm +++ b/lib/Zonemaster/Backend/DB/SQLite.pm @@ -270,7 +270,7 @@ sub test_results { 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}; - die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $result; + #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} ); diff --git a/lib/Zonemaster/Backend/RPCAPI.pm b/lib/Zonemaster/Backend/RPCAPI.pm index 9f1a32bf0..1be68d685 100644 --- a/lib/Zonemaster/Backend/RPCAPI.pm +++ b/lib/Zonemaster/Backend/RPCAPI.pm @@ -85,7 +85,7 @@ sub handle_exception { if ( $exception->isa('Zonemaster::Backend::Error::Internal') ) { $log->error($exception->as_string); } else { - $log->notice($exception->as_string); + $log->info($exception->as_string); } die $exception->as_hash; From f9d0d8d2ef43f669e07f7e4f9d3ec1c8d39591f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Thu, 12 Aug 2021 16:07:55 +0200 Subject: [PATCH 6/8] fix comment --- lib/Zonemaster/Backend/DB/SQLite.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Zonemaster/Backend/DB/SQLite.pm b/lib/Zonemaster/Backend/DB/SQLite.pm index 3568080ab..ecea51f89 100644 --- a/lib/Zonemaster/Backend/DB/SQLite.pm +++ b/lib/Zonemaster/Backend/DB/SQLite.pm @@ -270,7 +270,7 @@ sub test_results { 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}; - #die Zonemaster::Backend::Error::ResourceNotFound->new( message => "Test not found", data => { test_id => $test_id } ) unless defined $result; + 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} ); From 4db8356aecb1b68590fa079bf23ede79aa3f6940 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Thu, 12 Aug 2021 17:13:56 +0200 Subject: [PATCH 7/8] add exception handler decorator --- lib/Zonemaster/Backend/RPCAPI.pm | 49 ++++++++++++++++++++++++++------ 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/lib/Zonemaster/Backend/RPCAPI.pm b/lib/Zonemaster/Backend/RPCAPI.pm index 1be68d685..6bc1e6a92 100644 --- a/lib/Zonemaster/Backend/RPCAPI.pm +++ b/lib/Zonemaster/Backend/RPCAPI.pm @@ -75,11 +75,11 @@ sub _init_db { } sub handle_exception { - my ( $_method, $exception, $exception_id ) = @_; + my ( $method, $exception, $exception_id ) = @_; if ( !$exception->isa('Zonemaster::Backend::Error') ) { my $reason = $exception; - $exception = Zonemaster::Backend::Error::Internal->new(reason => $reason, id => $exception_id); + $exception = Zonemaster::Backend::Error::Internal->new(reason => $reason, id => $exception_id, method => $method); } if ( $exception->isa('Zonemaster::Backend::Error::Internal') ) { @@ -96,18 +96,49 @@ sub version_info { my ( $self ) = @_; my %ver; - eval { - $ver{zonemaster_engine} = Zonemaster::Engine->VERSION; - $ver{zonemaster_backend} = Zonemaster::Backend->VERSION; + $ver{zonemaster_engine} = Zonemaster::Engine->VERSION; + $ver{zonemaster_backend} = Zonemaster::Backend->VERSION; + return \%ver; +} - }; - if ($@) { - handle_exception('version_info', $@, '003'); +sub decorate { + my $subname = shift; + my @decorators = reverse @_; + $subname = __PACKAGE__ . "::$subname"; + my $sub = \&$subname; + for my $d (@decorators) { + $sub = $d->($sub, $subname); } + no strict; + * { $subname } = $sub; +} - return \%ver; +sub handler_exception_decorator { + my $id = shift; + return sub { + my $sub = shift; + my $subname = shift; + return sub { + my $ret = eval { + return $sub->(@_); + }; + handle_exception($subname, $@, $id) if ($@); + return $ret; + } + } } +sub validate_decorator { + my $sub = shift; + my $subname = shift; + return sub { + print "validating...\n"; + return $sub->(@_); + }; +} + +decorate('version_info', \&validate_decorator, handler_exception_decorator(003)); + $json_schemas{profile_names} = joi->object->strict; sub profile_names { my ( $self ) = @_; From 7b1f5b45a6b90c9e4bfcc12b6df4f41b7acc7baa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Thu, 12 Aug 2021 18:05:45 +0200 Subject: [PATCH 8/8] decorate all methods --- lib/Zonemaster/Backend/RPCAPI.pm | 412 +++++++++++++------------------ 1 file changed, 178 insertions(+), 234 deletions(-) diff --git a/lib/Zonemaster/Backend/RPCAPI.pm b/lib/Zonemaster/Backend/RPCAPI.pm index 6bc1e6a92..bc515a4ec 100644 --- a/lib/Zonemaster/Backend/RPCAPI.pm +++ b/lib/Zonemaster/Backend/RPCAPI.pm @@ -61,6 +61,67 @@ sub new { return ( $self ); } +sub __decorate { + my $subname = shift; + my @decorators = reverse @_; + $subname = __PACKAGE__ . "::$subname"; + my $sub = \&$subname; + for my $d (@decorators) { + $sub = $d->($sub, $subname); + } + no strict; + no warnings 'redefine'; + * { $subname } = $sub; +} + +sub __handler_exception_decorator { + my $id = shift; + return sub { + my $sub = shift; + my $subname = shift; + return sub { + my $ret = eval { + return $sub->(@_); + }; + handle_exception($subname, $@, $id) if ($@); + return $ret; + } + } +} + +__decorate('_init_db', __handler_exception_decorator( 2)); +# Example of chained decorators, exception handler first then validation so that error are catched. +# __decorate('version_info', __handler_exception_decorator( 3), \&validate_decorator); +__decorate('version_info', __handler_exception_decorator( 3)); +__decorate('profile_names', __handler_exception_decorator( 4)); +__decorate('get_language_tags', __handler_exception_decorator( 0)); +__decorate('get_host_by_name', __handler_exception_decorator( 5)); +__decorate('get_data_from_parent_zone', __handler_exception_decorator( 6)); +__decorate('start_domain_test_validate_syntax', __handler_exception_decorator( 8)); +__decorate('start_domain_test', __handler_exception_decorator( 9)); +__decorate('test_progress', __handler_exception_decorator( 10)); +__decorate('get_test_params', __handler_exception_decorator( 11)); +__decorate('get_test_results', __handler_exception_decorator( 12)); +__decorate('get_test_history', __handler_exception_decorator( 13)); +__decorate('add_api_user', __handler_exception_decorator( 14)); +__decorate('add_batch_job', __handler_exception_decorator( 15)); +__decorate('get_batch_job_result', __handler_exception_decorator( 16)); + + +# sub validate_decorator { +# my $sub = shift; +# my $subname = shift; +# $subname =~ /.*::(\w+)/; +# print $1 . "\n"; +# return sub { +# my $self = shift; +# my $params = shift; +# # Add validation here... +# # $self->validate_params($subname, $params); +# return $sub->($self, $params, @_); +# }; +# } + sub _init_db { my ( $self, $dbtype ) = @_; @@ -68,10 +129,7 @@ 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'); - } + die Zonemaster::Backend::Error::Internal->new( message => "Failed to initialize the [$dbtype] database backend module: [$@]") if ($@); } sub handle_exception { @@ -83,6 +141,7 @@ sub handle_exception { } if ( $exception->isa('Zonemaster::Backend::Error::Internal') ) { + $exception->id($exception_id); $log->error($exception->as_string); } else { $log->info($exception->as_string); @@ -91,6 +150,7 @@ sub handle_exception { die $exception->as_hash; } + $json_schemas{version_info} = joi->object->strict; sub version_info { my ( $self ) = @_; @@ -101,53 +161,13 @@ sub version_info { return \%ver; } -sub decorate { - my $subname = shift; - my @decorators = reverse @_; - $subname = __PACKAGE__ . "::$subname"; - my $sub = \&$subname; - for my $d (@decorators) { - $sub = $d->($sub, $subname); - } - no strict; - * { $subname } = $sub; -} - -sub handler_exception_decorator { - my $id = shift; - return sub { - my $sub = shift; - my $subname = shift; - return sub { - my $ret = eval { - return $sub->(@_); - }; - handle_exception($subname, $@, $id) if ($@); - return $ret; - } - } -} - -sub validate_decorator { - my $sub = shift; - my $subname = shift; - return sub { - print "validating...\n"; - return $sub->(@_); - }; -} - -decorate('version_info', \&validate_decorator, handler_exception_decorator(003)); $json_schemas{profile_names} = joi->object->strict; sub profile_names { my ( $self ) = @_; my %profiles; - eval { %profiles = $self->{config}->PUBLIC_PROFILES }; - if ( $@ ) { - handle_exception( 'profile_names', $@, '004' ); - } + %profiles = $self->{config}->PUBLIC_PROFILES; return [ keys %profiles ]; } @@ -179,16 +199,10 @@ sub get_host_by_name { my ( $self, $params ) = @_; my @adresses; - eval { - my $ns_name = $params->{hostname}; - - @adresses = map { {$ns_name => $_->short} } $recursor->get_addresses_for($ns_name); - @adresses = { $ns_name => '0.0.0.0' } if not @adresses; + my $ns_name = $params->{hostname}; - }; - if ($@) { - handle_exception('get_host_by_name', $@, '005'); - } + @adresses = map { {$ns_name => $_->short} } $recursor->get_addresses_for($ns_name); + @adresses = { $ns_name => '0.0.0.0' } if not @adresses; return \@adresses; } @@ -209,39 +223,31 @@ $json_schemas{get_data_from_parent_zone} = joi->object->strict->props( sub get_data_from_parent_zone { my ( $self, $params ) = @_; - my $result = eval { - my %result; - my $domain = $params->{domain}; + my %result; + my $domain = $params->{domain}; - my @ns_list; - my @ns_names; + my @ns_list; + my @ns_names; - my $zone = Zonemaster::Engine->zone( $domain ); - push @ns_list, { ns => $_->name->string, ip => $_->address->short} for @{$zone->glue}; + my $zone = Zonemaster::Engine->zone( $domain ); + push @ns_list, { ns => $_->name->string, ip => $_->address->short} for @{$zone->glue}; - my @ds_list; + my @ds_list; - $zone = Zonemaster::Engine->zone($domain); - my $ds_p = $zone->parent->query_one( $zone->name, 'DS', { dnssec => 1, cd => 1, recurse => 1 } ); - if ($ds_p) { - my @ds = $ds_p->get_records( 'DS', 'answer' ); + $zone = Zonemaster::Engine->zone($domain); + my $ds_p = $zone->parent->query_one( $zone->name, 'DS', { dnssec => 1, cd => 1, recurse => 1 } ); + if ($ds_p) { + my @ds = $ds_p->get_records( 'DS', 'answer' ); - foreach my $ds ( @ds ) { - next unless $ds->type eq 'DS'; - push(@ds_list, { keytag => $ds->keytag, algorithm => $ds->algorithm, digtype => $ds->digtype, digest => $ds->hexdigest }); - } + foreach my $ds ( @ds ) { + next unless $ds->type eq 'DS'; + push(@ds_list, { keytag => $ds->keytag, algorithm => $ds->algorithm, digtype => $ds->digtype, digest => $ds->hexdigest }); } - - $result{ns_list} = \@ns_list; - $result{ds_list} = \@ds_list; - return \%result; - }; - if ($@) { - handle_exception('get_data_from_parent_zone', $@, '006'); - } - elsif ($result) { - return $result; } + + $result{ns_list} = \@ns_list; + $result{ds_list} = \@ds_list; + return \%result; } @@ -297,46 +303,39 @@ sub _check_domain { return ( $domain, { status => 'ok', message => 'Syntax ok' } ); } -$extra_validators{start_domain_test} = sub { +$extra_validators{start_domain_test} = \&start_domain_test_validate_syntax; +sub start_domain_test_validate_syntax { my ( $self, $syntax_input ) = @_; - my @errors = eval { - my @errors; + my @errors; - if ( defined $syntax_input->{profile} ) { - $syntax_input->{profile} = lc $syntax_input->{profile}; - my %profiles = ( $self->{config}->PUBLIC_PROFILES, $self->{config}->PRIVATE_PROFILES ); - if ( !exists $profiles{ $syntax_input->{profile} } ) { - push @errors, { path => '/profile', message => 'Unknown profile' }; - } + if ( defined $syntax_input->{profile} ) { + $syntax_input->{profile} = lc $syntax_input->{profile}; + my %profiles = ( $self->{config}->PUBLIC_PROFILES, $self->{config}->PRIVATE_PROFILES ); + if ( !exists $profiles{ $syntax_input->{profile} } ) { + push @errors, { path => '/profile', message => 'Unknown profile' }; } + } - if ( defined $syntax_input->{domain} ) { - my ( undef, $dn_syntax ) = $self->_check_domain( $syntax_input->{domain} ); - push @errors, { path => "/domain", message => $dn_syntax->{message} } if ( $dn_syntax->{status} eq 'nok' ); - } + if ( defined $syntax_input->{domain} ) { + my ( undef, $dn_syntax ) = $self->_check_domain( $syntax_input->{domain} ); + push @errors, { path => "/domain", message => $dn_syntax->{message} } if ( $dn_syntax->{status} eq 'nok' ); + } - if ( defined $syntax_input->{nameservers} && ref $syntax_input->{nameservers} eq 'ARRAY' && @{ $syntax_input->{nameservers} } ) { - while (my ($index, $ns_ip) = each @{ $syntax_input->{nameservers} }) { - my ( $ns, $ns_syntax ) = $self->_check_domain( $ns_ip->{ns} ); - push @errors, { path => "/nameservers/$index/ns", message => $ns_syntax->{message} } if ( $ns_syntax->{status} eq 'nok' ); + if ( defined $syntax_input->{nameservers} && ref $syntax_input->{nameservers} eq 'ARRAY' && @{ $syntax_input->{nameservers} } ) { + while (my ($index, $ns_ip) = each @{ $syntax_input->{nameservers} }) { + my ( $ns, $ns_syntax ) = $self->_check_domain( $ns_ip->{ns} ); + push @errors, { path => "/nameservers/$index/ns", message => $ns_syntax->{message} } if ( $ns_syntax->{status} eq 'nok' ); - push @errors, { path => "/nameservers/$index/ip", message => 'Invalid IP address' } - unless ( !$ns_ip->{ip} - || Zonemaster::Engine::Net::IP::ip_is_ipv4( $ns_ip->{ip} ) - || Zonemaster::Engine::Net::IP::ip_is_ipv6( $ns_ip->{ip} ) ); - } + push @errors, { path => "/nameservers/$index/ip", message => 'Invalid IP address' } + unless ( !$ns_ip->{ip} + || Zonemaster::Engine::Net::IP::ip_is_ipv4( $ns_ip->{ip} ) + || Zonemaster::Engine::Net::IP::ip_is_ipv6( $ns_ip->{ip} ) ); } - - return @errors; - }; - if ($@) { - handle_exception('start_domain_test_validate_syntax', $@, '008'); } - else { - return @errors; - } -}; + + return @errors; +} $json_schemas{start_domain_test} = joi->object->strict->props( domain => $zm_validator->domain_name->required, @@ -360,19 +359,15 @@ sub start_domain_test { my ( $self, $params ) = @_; my $result = 0; - eval { - $params->{domain} =~ s/^\.// unless ( !$params->{domain} || $params->{domain} eq '.' ); - die "No domain in parameters\n" unless ( $params->{domain} ); + $params->{domain} =~ s/^\.// unless ( !$params->{domain} || $params->{domain} eq '.' ); - $params->{priority} //= 10; - $params->{queue} //= 0; + die "No domain in parameters\n" unless ( $params->{domain} ); - $result = $self->{db}->create_new_test( $params->{domain}, $params, $self->{config}->ZONEMASTER_age_reuse_previous_test ); - }; - if ($@) { - handle_exception('start_domain_test', $@, '009'); - } + $params->{priority} //= 10; + $params->{queue} //= 0; + + $result = $self->{db}->create_new_test( $params->{domain}, $params, $self->{config}->ZONEMASTER_age_reuse_previous_test ); return $result; } @@ -383,16 +378,9 @@ $json_schemas{test_progress} = joi->object->strict->props( sub test_progress { my ( $self, $params ) = @_; - my $result = 0; - eval { - my $test_id = $params->{test_id}; - $result = $self->{db}->test_progress( $test_id ); - }; - if ($@) { - handle_exception('test_progress', $@, '010'); - } + my $test_id = $params->{test_id}; - return $result; + return $self->{db}->test_progress( $test_id ); } $json_schemas{get_test_params} = joi->object->strict->props( @@ -403,16 +391,7 @@ sub get_test_params { my $test_id = $params->{test_id}; - my $result = 0; - - eval { - $result = $self->{db}->get_test_params( $test_id ); - }; - if ($@) { - handle_exception('get_test_params', $@, '011'); - } - - return $result; + return $self->{db}->get_test_params( $test_id ); } $json_schemas{get_test_results} = joi->object->strict->props( @@ -457,61 +436,57 @@ sub get_test_results { my $test_info; my @zm_results; - eval{ - $test_info = $self->{db}->test_results( $params->{id} ); - foreach my $test_res ( @{ $test_info->{results} } ) { - my $res; - if ( $test_res->{module} eq 'NAMESERVER' ) { - $res->{ns} = ( $test_res->{args}->{ns} ) ? ( $test_res->{args}->{ns} ) : ( 'All' ); - } - elsif ($test_res->{module} eq 'SYSTEM' - && $test_res->{tag} eq 'POLICY_DISABLED' - && $test_res->{args}->{name} eq 'Example' ) - { - next; - } - $res->{module} = $test_res->{module}; - $res->{message} = $translator->translate_tag( $test_res ) . "\n"; - $res->{message} =~ s/,/, /isg; - $res->{message} =~ s/;/; /isg; - $res->{level} = $test_res->{level}; - - if ( $test_res->{module} eq 'SYSTEM' ) { - if ( $res->{message} =~ /policy\.json/ ) { - my ( $policy ) = ( $res->{message} =~ /\s(\/.*)$/ ); - if ( $policy ) { - my $policy_description = 'DEFAULT POLICY'; - $policy_description = 'SOME OTHER POLICY' if ( $policy =~ /some\/other\/policy\/path/ ); - $res->{message} =~ s/$policy/$policy_description/; - } - else { - $res->{message} = 'UNKNOWN POLICY FORMAT'; - } + $test_info = $self->{db}->test_results( $params->{id} ); + foreach my $test_res ( @{ $test_info->{results} } ) { + my $res; + if ( $test_res->{module} eq 'NAMESERVER' ) { + $res->{ns} = ( $test_res->{args}->{ns} ) ? ( $test_res->{args}->{ns} ) : ( 'All' ); + } + elsif ($test_res->{module} eq 'SYSTEM' + && $test_res->{tag} eq 'POLICY_DISABLED' + && $test_res->{args}->{name} eq 'Example' ) + { + next; + } + + $res->{module} = $test_res->{module}; + $res->{message} = $translator->translate_tag( $test_res ) . "\n"; + $res->{message} =~ s/,/, /isg; + $res->{message} =~ s/;/; /isg; + $res->{level} = $test_res->{level}; + + if ( $test_res->{module} eq 'SYSTEM' ) { + if ( $res->{message} =~ /policy\.json/ ) { + my ( $policy ) = ( $res->{message} =~ /\s(\/.*)$/ ); + if ( $policy ) { + my $policy_description = 'DEFAULT POLICY'; + $policy_description = 'SOME OTHER POLICY' if ( $policy =~ /some\/other\/policy\/path/ ); + $res->{message} =~ s/$policy/$policy_description/; } - elsif ( $res->{message} =~ /config\.json/ ) { - my ( $config ) = ( $res->{message} =~ /\s(\/.*)$/ ); - if ( $config ) { - my $config_description = 'DEFAULT CONFIGURATION'; - $config_description = 'SOME OTHER CONFIGURATION' if ( $config =~ /some\/other\/configuration\/path/ ); - $res->{message} =~ s/$config/$config_description/; - } - else { - $res->{message} = 'UNKNOWN CONFIG FORMAT'; - } + else { + $res->{message} = 'UNKNOWN POLICY FORMAT'; + } + } + elsif ( $res->{message} =~ /config\.json/ ) { + my ( $config ) = ( $res->{message} =~ /\s(\/.*)$/ ); + if ( $config ) { + my $config_description = 'DEFAULT CONFIGURATION'; + $config_description = 'SOME OTHER CONFIGURATION' if ( $config =~ /some\/other\/configuration\/path/ ); + $res->{message} =~ s/$config/$config_description/; + } + else { + $res->{message} = 'UNKNOWN CONFIG FORMAT'; } } - - push( @zm_results, $res ); } - $result = $test_info; - $result->{results} = \@zm_results; - }; - if ($@) { - handle_exception('get_test_results', $@, '012'); + push( @zm_results, $res ); } + $result = $test_info; + $result->{results} = \@zm_results; + $translator->locale( $previous_locale ); $result = $test_info; @@ -531,20 +506,11 @@ $json_schemas{get_test_history} = joi->object->strict->props( sub get_test_history { my ( $self, $params ) = @_; - my $results; + $params->{offset} //= 0; + $params->{limit} //= 200; + $params->{filter} //= "all"; - eval { - $params->{offset} //= 0; - $params->{limit} //= 200; - $params->{filter} //= "all"; - - $results = $self->{db}->get_test_history( $params ); - }; - if ($@) { - handle_exception('get_test_history', $@, '013'); - } - - return $results; + return $self->{db}->get_test_history( $params ); } $json_schemas{add_api_user} = joi->object->strict->props( @@ -555,22 +521,17 @@ sub add_api_user { my ( $self, $params, undef, $remote_ip ) = @_; my $result = 0; + my $allow = 0; - eval { - my $allow = 0; - if ( defined $remote_ip ) { - $allow = 1 if ( $remote_ip eq '::1' || $remote_ip eq '127.0.0.1' ); - } - else { - $allow = 1; - } + if ( defined $remote_ip ) { + $allow = 1 if ( $remote_ip eq '::1' || $remote_ip eq '127.0.0.1' ); + } + else { + $allow = 1; + } - if ( $allow ) { - $result = 1 if ( $self->{db}->add_api_user( $params->{username}, $params->{api_key} ) eq '1' ); - } - }; - if ($@) { - handle_exception('add_api_user', $@, '014'); + if ( $allow ) { + $result = 1 if ( $self->{db}->add_api_user( $params->{username}, $params->{api_key} ) eq '1' ); } return $result; @@ -605,15 +566,7 @@ sub add_batch_job { $params->{test_params}->{priority} //= 5; $params->{test_params}->{queue} //= 0; - my $results; - eval { - $results = $self->{db}->add_batch_job( $params ); - }; - if ($@) { - handle_exception('add_batch_job', $@, '015'); - } - - return $results; + return $self->{db}->add_batch_job( $params ); } $json_schemas{get_batch_job_result} = joi->object->strict->props( @@ -622,18 +575,9 @@ $json_schemas{get_batch_job_result} = joi->object->strict->props( sub get_batch_job_result { my ( $self, $params ) = @_; - my $result; - - eval { - my $batch_id = $params->{batch_id}; + my $batch_id = $params->{batch_id}; - $result = $self->{db}->get_batch_job_result($batch_id); - }; - if ($@) { - handle_exception('get_batch_job_result', $@, '016'); - } - - return $result; + return $self->{db}->get_batch_job_result($batch_id); } my $rpc_request = joi->object->props(