Display an error message instead of a 500 page when the search string is invalid
authorVincent Tondellier <tonton+hg@team1664.org>
Sun, 18 Dec 2016 20:14:31 +0100
changeset 121 5a99941ed0ca
parent 120 1a0fa98037fa
child 122 8692800ec9ba
Display an error message instead of a 500 page when the search string is invalid
lib/CrashTest/Controller/CrashGroups.pm
lib/CrashTest/Controller/CrashReports.pm
lib/CrashTest/Plugin/Storage/Sql/Model/CrashGroup.pm
lib/CrashTest/Plugin/Storage/Sql/Model/CrashReport.pm
lib/CrashTest/Plugin/Storage/Sql/Utils.pm
lib/CrashTest/files/templates/layouts/_messages.html.ep
lib/CrashTest/files/templates/layouts/main.html.ep
--- a/lib/CrashTest/Controller/CrashGroups.pm	Sun Dec 04 01:16:39 2016 +0100
+++ b/lib/CrashTest/Controller/CrashGroups.pm	Sun Dec 18 20:14:31 2016 +0100
@@ -24,7 +24,13 @@
     $self->validation->required('page')->like(qr/^[0-9]+$/);
     $page = scalar $self->validation->param("page") if $self->validation->is_valid('page');
 
-    my ($results, $pager) = $self->crash_groups->index($page, $crashs_per_page, $self->req->param('search'));
+    my ($results, $pager, $err) = $self->crash_groups->index($page, $crashs_per_page, $self->req->param('search'));
+
+    if($err) {
+        $self->app->log->info($err);
+        $self->stash(error => "Syntax error in search string: $err");
+    }
+
 
     #$self->app->log->debug(dumper $results);
 
@@ -51,12 +57,17 @@
 
     my $search = $self->req->param('search');
     if(defined($search) && $search ne "") {
-        $search .= " AND " . "group_id=$group->{id}";
+        $search = "(" . $search . ") AND " . "group_id=$group->{id}";
     } else {
         $search = "group_id=$group->{id}";
     }
 
-    my ($results, $pager) = $self->crash_reports->index($page, $crashs_per_page, $search);
+    my ($results, $pager, $err) = $self->crash_reports->index($page, $crashs_per_page, $search);
+
+    if($err) {
+        $self->app->log->info($err);
+        $self->stash(warning => "Syntax error in search string: $err");
+    }
 
     #$self->app->log->debug(dumper $results);
 
--- a/lib/CrashTest/Controller/CrashReports.pm	Sun Dec 04 01:16:39 2016 +0100
+++ b/lib/CrashTest/Controller/CrashReports.pm	Sun Dec 18 20:14:31 2016 +0100
@@ -30,7 +30,12 @@
     $self->validation->required('page')->like(qr/^[0-9]+$/);
     $page = scalar $self->validation->param("page") if $self->validation->is_valid('page');
 
-    my ($results, $pager) = $self->crash_reports->index($page, $crashs_per_page, $self->req->param('search'));
+    my ($results, $pager, $err) = $self->crash_reports->index($page, $crashs_per_page, $self->req->param('search'));
+
+    if($err) {
+        $self->app->log->info($err);
+        $self->stash(error => "Syntax error in search string: $err");
+    }
 
     #$self->app->log->debug(dumper $results);
 
--- a/lib/CrashTest/Plugin/Storage/Sql/Model/CrashGroup.pm	Sun Dec 04 01:16:39 2016 +0100
+++ b/lib/CrashTest/Plugin/Storage/Sql/Model/CrashGroup.pm	Sun Dec 18 20:14:31 2016 +0100
@@ -52,10 +52,14 @@
 
     my $where = "";
     my @values = ();
+    my $err;
     if(defined($search_str) && $search_str ne "") {
-        my $q = $self->_build_query_from_search_string($search_str);
-        $where = "WHERE " . $q->[0];
-        @values = @{$q->[1]};
+        eval {
+            my $q = $self->_build_query_from_search_string($search_str);
+            $where = "WHERE " . $q->[0];
+            @values = @{$q->[1]};
+        };
+        $err = $@;
     }
 
     my $count = $self->db->query("
@@ -102,7 +106,7 @@
         @values, $pager->skipped, $pager->entries_per_page
     )->expand->hashes;
 
-    return ($results, $pager);
+    return ($results, $pager, $err);
 }
 
 sub get {
--- a/lib/CrashTest/Plugin/Storage/Sql/Model/CrashReport.pm	Sun Dec 04 01:16:39 2016 +0100
+++ b/lib/CrashTest/Plugin/Storage/Sql/Model/CrashReport.pm	Sun Dec 18 20:14:31 2016 +0100
@@ -57,10 +57,14 @@
 
     my $where = "";
     my @values = ();
+    my $err;
     if(defined($search_str) && $search_str ne "") {
-        my $q = $self->_build_query_from_search_string($search_str);
-        $where = "WHERE " . $q->[0];
-        @values = @{$q->[1]};
+        eval {
+            my $q = $self->_build_query_from_search_string($search_str);
+            $where = "WHERE " . $q->[0];
+            @values = @{$q->[1]};
+        };
+        $err = $@;
     }
 
     my $count = $self->db->query("
@@ -101,7 +105,7 @@
         $pager->skipped, $pager->entries_per_page
     )->expand->hashes;
 
-    return ($results, $pager);
+    return ($results, $pager, $err);
 }
 
 sub get {
--- a/lib/CrashTest/Plugin/Storage/Sql/Utils.pm	Sun Dec 04 01:16:39 2016 +0100
+++ b/lib/CrashTest/Plugin/Storage/Sql/Utils.pm	Sun Dec 18 20:14:31 2016 +0100
@@ -74,8 +74,18 @@
         strict          => 1,
     );
 
-    my $query = $parser->parse($search, 1)
-        or die "Error in query: " . $parser->err;
+    my $query;
+    eval {
+        $query = $parser->parse($search, 1);
+    };
+    if($@) {
+        # strip file and line info from message
+        $@ =~ /(.+)(?:\s+at\s+[^\s]+\s+line\s+\d+\.)$/;
+        my $msg = $1 || $@;
+        die($msg . "\n");
+    } elsif(!defined($query)) {
+        die($parser->err . "\n");
+    }
 
     # reset before calling dbi
     @values = ();
--- a/lib/CrashTest/files/templates/layouts/_messages.html.ep	Sun Dec 04 01:16:39 2016 +0100
+++ b/lib/CrashTest/files/templates/layouts/_messages.html.ep	Sun Dec 18 20:14:31 2016 +0100
@@ -1,41 +1,18 @@
-% content_for 'success_messages' => begin
- % if ( defined($self->session->{success_messages}) && ( scalar( @{ $self->session->{success_messages}} ) > 0 ) ){
-  %foreach my $notice ( @{ $self->session->{success_messages}} ){
-   <div class="alert alert-success">
-    <a class="close" data-dismiss="alert">×</a>
-    <p><%= $notice; %></p>
-   </div>
-  %}
-  %delete($self->session->{success_messages});
- %}
-%end
-
-% content_for 'noticed_messages' => begin
- % if ( defined($self->session->{notice_messages}) && ( scalar( @{ $self->session->{notice_messages}} ) > 0 ) ){
-  %foreach my $notice ( @{ $self->session->{notice_messages}} ){
-   <div class="alert alert-info">
-    <a class="close" data-dismiss="alert">×</a>
-    <p><%= $notice; %></p>
-   </div>
-  %}
-  %delete($self->session->{notice_messages});
- %}
-%end
-
-% content_for 'error_messages' => begin
- % if ( defined($self->session->{error_messages}) && ( scalar( @{ $self->session->{error_messages}} ) > 0 ) ){
-  <div id="error-messages" class="error-messages">
-   %foreach my $error ( @{ $self->session->{error_messages}} ){
-    <div class="alert alert-error">
-     <a class="close" data-dismiss="alert">×</a>
-     <p><%= $error; %></p>
-    </div>
-   %}
-   %delete($self->session->{error_messages});
-  </div>
- %}
-%end
-
-<%= content_for 'error_messages' %>
-<%= content_for 'noticed_messages' %>
-<%= content_for 'success_messages' %>
+% for my $msgtype (qw<success info warning error danger>) {
+%   my $msg = stash($msgtype);
+%   unless($msg) {
+%     $msg = flash($msgtype);
+%   }
+%   my $alert_class = $msgtype;
+%   $alert_class =~ s/error/danger/;
+%   if($msg) {
+      %=t div => (class => "alert alert-$alert_class") => begin
+        %=t button => (type => "button", class => "close", 'data-dismiss' => "alert", 'aria-label' => "Close") => begin
+          %=t span => ('aria-hidden' => "true") => begin
+            &times;
+          % end
+        % end
+        %=t p => $msg
+      % end
+%   }
+% }
--- a/lib/CrashTest/files/templates/layouts/main.html.ep	Sun Dec 04 01:16:39 2016 +0100
+++ b/lib/CrashTest/files/templates/layouts/main.html.ep	Sun Dec 18 20:14:31 2016 +0100
@@ -20,6 +20,7 @@
     %= include("layouts/_navbar")
     <div class="container-fluid">
       <%= content_for 'hero_unit' %>
+      %= include("layouts/_messages")
 
       <%= content %>
     </div>