Bug 1184001 - deliver error report to sentry via cron instead of immediately

5 files changed, 97 insertions, 88 deletions

diff --git a/.gitignore b/.gitignore
index dfb2b8929..a829834c5 100644
--- a/.gitignore
+++ b/.gitignore
@@ -11,5 +11,6 @@
 /localconfig
 /index.html
 /template_cache
+/error_reports
 .DS_Store
 cpanfile
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index 61a33ae02..c6359be60 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -697,28 +697,30 @@ sub _bz_locations {
     # We have to return absolute paths for mod_perl. 
     # That means that if you modify these paths, they must be absolute paths.
     return {
-        'libpath'     => $libpath,
-        'ext_libpath' => "$libpath/lib",
+        'libpath'        => $libpath,
+        'ext_libpath'    => "$libpath/lib",
         # If you put the libraries in a different location than the CGIs,
         # make sure this still points to the CGIs.
-        'cgi_path'    => $libpath,
-        'templatedir' => "$libpath/template",
+        'cgi_path'       => $libpath,
+        'templatedir'    => "$libpath/template",
         'template_cache' => "$libpath/template_cache",
-        'project'     => $project,
-        'localconfig' => "$libpath/$localconfig",
-        'datadir'     => $datadir,
-        'attachdir'   => "$datadir/attachments",
-        'skinsdir'    => "$libpath/skins",
-        'graphsdir'   => "$libpath/graphs",
-        # $webdotdir must be in the web server's tree somewhere. Even if you use a 
-        # local dot, we output images to there. Also, if $webdotdir is 
-        # not relative to the bugzilla root directory, you'll need to 
-        # change showdependencygraph.cgi to set image_url to the correct 
+        'project'        => $project,
+        'localconfig'    => "$libpath/$localconfig",
+        'datadir'        => $datadir,
+        'attachdir'      => "$datadir/attachments",
+        'skinsdir'       => "$libpath/skins",
+        'graphsdir'      => "$libpath/graphs",
+        # $webdotdir must be in the web server's tree somewhere. Even if you use a
+        # local dot, we output images to there. Also, if $webdotdir is
+        # not relative to the bugzilla root directory, you'll need to
+        # change showdependencygraph.cgi to set image_url to the correct
         # location.
         # The script should really generate these graphs directly...
-        'webdotdir'   => "$datadir/webdot",
-        'extensionsdir' => "$libpath/extensions",
-        'assetsdir'   => "$datadir/assets",
+        'webdotdir'      => "$datadir/webdot",
+        'extensionsdir'  => "$libpath/extensions",
+        'assetsdir'      => "$datadir/assets",
+        # error_reports store error/warnings destined for sentry
+        'error_reports'  => "$libpath/error_reports",
     };
 }
 
diff --git a/Bugzilla/Install/Filesystem.pm b/Bugzilla/Install/Filesystem.pm
index 4323975bc..34d0ee4ab 100644
--- a/Bugzilla/Install/Filesystem.pm
+++ b/Bugzilla/Install/Filesystem.pm
@@ -115,18 +115,19 @@ sub DIR_ALSO_WS_SERVE { _suexec() ? 0001 : 0 };
 # by this group. Otherwise someone may find it possible to change the cgis
 # when exploiting some security flaw somewhere (not necessarily in Bugzilla!)
 sub FILESYSTEM {
-    my $datadir       = bz_locations()->{'datadir'};
-    my $attachdir     = bz_locations()->{'attachdir'};
-    my $extensionsdir = bz_locations()->{'extensionsdir'};
-    my $webdotdir     = bz_locations()->{'webdotdir'};
-    my $templatedir   = bz_locations()->{'templatedir'};
-    my $libdir        = bz_locations()->{'libpath'};
-    my $extlib        = bz_locations()->{'ext_libpath'};
-    my $skinsdir      = bz_locations()->{'skinsdir'};
-    my $localconfig   = bz_locations()->{'localconfig'};
+    my $datadir        = bz_locations()->{'datadir'};
+    my $attachdir      = bz_locations()->{'attachdir'};
+    my $extensionsdir  = bz_locations()->{'extensionsdir'};
+    my $webdotdir      = bz_locations()->{'webdotdir'};
+    my $templatedir    = bz_locations()->{'templatedir'};
+    my $libdir         = bz_locations()->{'libpath'};
+    my $extlib         = bz_locations()->{'ext_libpath'};
+    my $skinsdir       = bz_locations()->{'skinsdir'};
+    my $localconfig    = bz_locations()->{'localconfig'};
     my $template_cache = bz_locations()->{'template_cache'};
-    my $graphsdir     = bz_locations()->{'graphsdir'};
-    my $assetsdir     = bz_locations()->{'assetsdir'};
+    my $graphsdir      = bz_locations()->{'graphsdir'};
+    my $assetsdir      = bz_locations()->{'assetsdir'};
+    my $error_reports  = bz_locations()->{'error_reports'};
 
     # We want to set the permissions the same for all localconfig files
     # across all PROJECTs, so we do something special with $localconfig,
@@ -207,6 +208,8 @@ sub FILESYSTEM {
         # Writeable directories
          $template_cache    => { files => CGI_READ,
                                   dirs => DIR_CGI_OVERWRITE },
+         $error_reports     => { files => CGI_READ,
+                                  dirs => DIR_CGI_WRITE },
          $attachdir         => { files => CGI_WRITE,
                                   dirs => DIR_CGI_WRITE },
          $webdotdir         => { files => WS_SERVE,
@@ -296,6 +299,8 @@ sub FILESYSTEM {
         $graphsdir              => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE,
         $webdotdir              => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE,
         $assetsdir              => DIR_CGI_WRITE | DIR_ALSO_WS_SERVE,
+        $template_cache         => DIR_CGI_WRITE,
+        $error_reports          => DIR_CGI_WRITE,
         # Directories that contain content served directly by the web server.
         "$skinsdir/custom"      => DIR_WS_SERVE,
         "$skinsdir/contrib"     => DIR_WS_SERVE,
@@ -359,6 +364,8 @@ EOT
                                           contents => HT_DEFAULT_DENY },
         "$datadir/.htaccess"         => { perms    => WS_SERVE,
                                           contents => HT_DEFAULT_DENY },
+        "$error_reports/.htaccess"   => { perms    => WS_SERVE,
+                                          contents => HT_DEFAULT_DENY },
 
         "$graphsdir/.htaccess" => { perms => WS_SERVE, contents => <<EOT
 # Allow access to .png and .gif files.
diff --git a/Bugzilla/Sentry.pm b/Bugzilla/Sentry.pm
index 4206118da..73f316a1d 100644
--- a/Bugzilla/Sentry.pm
+++ b/Bugzilla/Sentry.pm
@@ -17,9 +17,9 @@ our @EXPORT = qw(
 );
 
 use Carp;
-use Data::Dumper;
 use DateTime;
 use File::Temp;
+use JSON ();
 use LWP::UserAgent;
 use Sys::Hostname;
 use URI;
@@ -215,10 +215,11 @@ sub sentry_handle_error {
             if exists $ENV{$field};
     }
 
+    my $now = DateTime->now();
     my $data = {
         event_id    => $id,
         message     => $message,
-        timestamp   => DateTime->now->iso8601(),
+        timestamp   => $now->iso8601(),
         level       => $level,
         platform    => 'Other',
         logger      => $logger,
@@ -235,17 +236,19 @@ sub sentry_handle_error {
         },
     };
 
-    my $fh = File::Temp->new( UNLINK => 0 );
+    my $fh = File::Temp->new(
+        DIR      => bz_locations()->{error_reports},
+        TEMPLATE => $now->ymd('') . $now->hms('') . '-XXXX',
+        SUFFIX   => '.dump',
+        UNLINK   => 0,
+
+    );
     if (!$fh) {
-        warn "Failed to create temp file: $!\n";
+        warn "Failed to create dump file: $!\n";
         return;
     }
-    print $fh Dumper($data);
-    close($fh) or die $!;
-    my $filename = $fh->filename;
-
-    my $command = bz_locations()->{'cgi_path'} . "/sentry.pl '$filename' &";
-    system($command);
+    print $fh JSON->new->utf8(1)->pretty(0)->allow_nonref(1)->encode($data);
+    close($fh);
     return 1;
 }
 
@@ -259,7 +262,7 @@ sub _write_to_error_log {
             Apache2::ServerRec::warn($message);
         }
     } else {
-        print STDERR "$message\n";
+        print STDERR $message, "\n";
     }
 }
 
diff --git a/sentry.pl b/sentry.pl
index 9b37694d8..3dbc6f489 100755
--- a/sentry.pl
+++ b/sentry.pl
@@ -26,67 +26,63 @@ use lib "$Bin/lib";
 
 use Bugzilla;
 use Bugzilla::Constants;
-use Bugzilla::RNG qw(irand);
 use Fcntl qw(:flock);
-use File::Slurp;
+use File::Slurp qw(read_file);
 use HTTP::Request::Common;
-use JSON ();
 use LWP::UserAgent;
-use POSIX qw(setsid nice);
-use Safe;
+use POSIX qw(nice);
 use URI;
 
 Bugzilla->usage_mode(USAGE_MODE_CMDLINE);
 nice(19);
 
-# detach
-open(STDIN, '<', '/dev/null');
-open(STDOUT, '>', '/dev/null');
-open(STDERR, '>', '/dev/null');
-setsid();
+exit(1) unless Bugzilla->params->{sentry_uri};
+my $uri = URI->new(Bugzilla->params->{sentry_uri});
+my $header = build_header($uri);
+exit(1) unless $header;
 
-# grab sentry server url
-my $sentry_uri = Bugzilla->params->{sentry_uri} || '';
-exit(1) unless $sentry_uri;
-
-# read data dump
-exit(1) unless my $filename = shift;
-my $dump = read_file($filename);
-unlink($filename);
-
-# deserialise
-my $cpt = new Safe;
-$cpt->reval($dump) || exit(1);
-my $data = ${$cpt->varglob('VAR1')};
-
-# split the sentry uri
-my $uri = URI->new($sentry_uri);
-my ($public_key, $secret_key) = split(/:/, $uri->userinfo);
-$uri->userinfo(undef);
-my $project_id = $uri->path;
-$project_id =~ s/^\///;
-$uri->path("/api/$project_id/store/");
+flock(DATA, LOCK_EX);
+foreach my $file (glob(bz_locations()->{error_reports} . '/*.dump')) {
+    eval {
+        send_file($uri, $header, $file);
+    };
+}
 
-# build the message
-my $message = JSON->new->utf8(1)->pretty(0)->allow_nonref(1)->encode($data);
-my %header = (
-    'X-Sentry-Auth' => sprintf(
-        "Sentry sentry_version=%s, sentry_timestamp=%s, sentry_key=%s, sentry_client=%s, sentry_secret=%s",
-        '2.0',
-        (time),
-        $public_key,
-        'bugzilla/4.2',
-        $secret_key,
-    ),
-    'Content-Type' => 'application/json'
-);
+sub build_header {
+    my ($uri) = @_;
+
+    # split the sentry uri
+    return undef unless $uri->userinfo && $uri->path;
+    my ($public_key, $secret_key) = split(/:/, $uri->userinfo);
+    $uri->userinfo(undef);
+    my $project_id = $uri->path;
+    $project_id =~ s/^\///;
+    $uri->path("/api/$project_id/store/");
+
+    # build the header
+    return {
+        'X-Sentry-Auth' => sprintf(
+            "Sentry sentry_version=%s, sentry_timestamp=%s, sentry_key=%s, sentry_client=%s, sentry_secret=%s",
+            '2.0',
+            (time),
+            $public_key,
+            'bmo/' . BUGZILLA_VERSION,
+            $secret_key,
+        ),
+        'Content-Type' => 'application/json'
+    };
+}
 
-# ensure we send warnings one at a time per webhead
-flock(DATA, LOCK_EX);
+sub send_file {
+    my ($uri, $header, $filename) = @_;
+    # read data dump
+    my $message = read_file($filename);
+    unlink($filename);
 
-# and post to sentry
-my $request = POST $uri->canonical, %header, Content => $message;
-my $response = LWP::UserAgent->new(timeout => 10)->request($request);
+    # and post to sentry
+    my $request = POST $uri->canonical, %$header, Content => $message;
+    my $response = LWP::UserAgent->new(timeout => 10)->request($request);
+}
 
 __DATA__
 this exists so the flock() code works.