diff --git a/lib/WebGUI.pm b/lib/WebGUI.pm index ba2288a5b..c5902c533 100644 --- a/lib/WebGUI.pm +++ b/lib/WebGUI.pm @@ -27,7 +27,7 @@ use WebGUI::Config; use WebGUI::Pluggable; use WebGUI::Paths; use WebGUI::Types; -use Try::Tiny; +use WebGUI::Exception; extends 'Plack::Component'; @@ -99,7 +99,7 @@ sub call { # Construct the PSGI response - try { + eval { # Ask PSGI server for a streaming writer object by returning only the first # two elements of the array reference my $writer = $responder->( [ $psgi_response->[0], $psgi_response->[1] ] ); @@ -116,17 +116,19 @@ sub call { # Close the session, because the WebGUI::Middleware::Session didn't $session->close; delete $env->{'webgui.session'}; - } - catch { + }; + if ( my $e = WebGUI::Error->caught ) { if ($response->writer) { # Response has already been started, so log error and close writer - $session->request->TRACE("Error detected after streaming response started"); + $session->request->TRACE( + "Error detected after streaming response started: " . $e->message . "\n" . $e->trace->as_string + ); $response->writer->close; } else { $responder->( [ 500, [ 'Content-Type' => 'text/plain' ], [ "Internal Server Error" ] ] ); } - }; + } } } } @@ -156,20 +158,26 @@ sub handle { # ); # return; + local $SIG{__DIE__} = sub { WebGUI::Error::RunTime->throw( message => $_[0] ); }; + # Look for the template preview HTTP headers WebGUI::Asset::Template->processVariableHeaders($session); # TODO: refactor the following loop, find all instances of "chunked" and "empty" in codebase, etc.. for my $handler (@{$session->config->get("contentHandlers")}) { my $output = eval { WebGUI::Pluggable::run($handler, "handler", [ $session ] )}; - if ( my $e = WebGUI::Error->caught ) { - $session->log->error($e->package.":".$e->line." - ".$e->full_message); - $session->log->debug($e->package.":".$e->line." - ".$e->trace); - } - elsif ( $@ ) { - $session->log->error( $@ ); + if ( $@ ) { + # re-throwing errors back out to plack is useless; to get the exception through to any middleware that + # want to report on it, we have to stash it in $env + # as long as our $SIG{__DIE__} is in effect, errors should always be objects + my $e = WebGUI::Error->caught; + $session->request->env->{'webgui.error'} = $e if $session->request->env->{'webgui.debug'}; + $session->log->error($e->package.":".$e->line." - ".$e->full_message, $@); + $session->log->debug($e->package.":".$e->line." - ".$e->trace, $@); } else { + + # Not an error # Stop if the contentHandler is going to stream the response body return if $session->response->streaming; diff --git a/lib/WebGUI/Account.pm b/lib/WebGUI/Account.pm index be6b22273..ca3b4ce62 100644 --- a/lib/WebGUI/Account.pm +++ b/lib/WebGUI/Account.pm @@ -198,12 +198,11 @@ sub callMethod { } #Try to call the method - my $output = eval { $self->$method(@{$args}) }; - - #Croak on error - if($@) { - croak "Unable to run $method on $module: $@"; - return undef; + my $output = eval { $self->$method(@{$args}); }; + if( $@ ) { + my $e = WebGUI::Error->caught; + $e->{message} = "Unable to run $method on $module: $e->{message}"; + $e->rethrow; } #Return the output from the method call diff --git a/lib/WebGUI/Asset.pm b/lib/WebGUI/Asset.pm index 6f6b94239..3228cb7dd 100644 --- a/lib/WebGUI/Asset.pm +++ b/lib/WebGUI/Asset.pm @@ -746,41 +746,51 @@ Any leftover part of the requested URL. sub dispatch { my ($self, $fragment) = @_; return undef if $fragment; + my $session = $self->session; my $state = $self->get('state'); + ##Only allow interaction with assets in certain states return if $state ne 'published' && $state ne 'archived' && !$session->isAdminOn; - my $func = $session->form->param('func') || 'view'; - my $viewing = $func eq 'view' ? 1 : 0; - my $sub = $self->can('www_'.$func); - if (!$sub && $func ne 'view') { - $sub = $self->can('www_view'); - $viewing = 1; + + # needed for tests that call straight here but otherwise redundant with same in WebGUI.pm + local $SIG{__DIE__} = sub { WebGUI::Error::RunTime->throw( message => $_[0] ); }; + + + for my $func ( $session->form->param('func'), 'view' ) { + + # if there's no output from the user specified func, try view next + + my $viewing = $func eq 'view' ? 1 : 0; + my $sub = $self->can('www_'.$func); + + if (!$sub && $func ne 'view') { + $sub = $self->can('www_view'); + $viewing = 1; + } + + return undef unless $sub; + + my $output = eval { $self->$sub(); }; + + if ( $@ ) { + my $e = Exception::Class->caught(); + # previously, this only handled WebGUI::Error::ObjectNotFound::Template + my $errstr = sprintf( + "Couldn't call method ``%s'' on asset for url ``%s'': Error: ``%s''", + "www_$func", $session->url->getRequestedUrl, $e->error, + ); + $errstr .= " templateId: " . $e->templateId if $e->can('templateId') and $e->templateId; + $errstr .= " assetId: " . $e->assetId if $e->can('assetId') and $e->assetId; + $session->log->error($errstr); + $e->rethrow if $session->request->env->{'webgui.debug'}; + } + + return $output if $output || $viewing; + } - return undef unless $sub; - my $output = eval { $self->$sub(); }; - if (my $e = Exception::Class->caught('WebGUI::Error::ObjectNotFound::Template')) { - #WebGUI::Error::ObjectNotFound::Template - $session->log->error(sprintf "%s templateId: %s assetId: %s", $e->error, $e->templateId, $e->assetId); - } - elsif ($@) { - my $message = $@; - $session->log->error("Couldn't call method www_".$func." on asset for url: ".$session->url->getRequestedUrl." Root cause: ".$message); - } - return $output if $output || $viewing; - ##No output, try the view method instead - $output = eval { $self->www_view }; - if (my $e = Exception::Class->caught('WebGUI::Error::ObjectNotFound::Template')) { - $session->log->error(sprintf "%s templateId: %s assetId: %s", $e->error, $e->templateId, $e->assetId); - return "chunked"; - } - elsif ($@) { - warn "logged another warn: $@"; - my $message = $@; - $session->log->warn("Couldn't call method www_view on asset for url: ".$session->url->getRequestedUrl." Root cause: ".$@); - return "chunked"; - } - return $output; + + return ''; # not reached } diff --git a/lib/WebGUI/Asset/Wobject/Survey.pm b/lib/WebGUI/Asset/Wobject/Survey.pm index 1a53b811d..b02c7c09f 100644 --- a/lib/WebGUI/Asset/Wobject/Survey.pm +++ b/lib/WebGUI/Asset/Wobject/Survey.pm @@ -470,7 +470,7 @@ sub graph { my $session = $self->session; - eval { local $SIG{'__DIE__'}; require GraphViz }; + eval { require GraphViz }; if ($@) { return; } @@ -710,7 +710,7 @@ sub www_graph { my $i18n = WebGUI::International->new($session, "Asset_Survey"); - eval { local $SIG{'__DIE__'}; require GraphViz }; + eval { require GraphViz }; if ($@) { return '

' . $i18n->get('survey visualization') . '

Survey Visualization requires the GraphViz module'; } diff --git a/lib/WebGUI/Content/Asset.pm b/lib/WebGUI/Content/Asset.pm index 999ff8103..67b10455c 100644 --- a/lib/WebGUI/Content/Asset.pm +++ b/lib/WebGUI/Content/Asset.pm @@ -79,15 +79,21 @@ sub dispatch { $fragment =~ s/$url//; $session->asset($asset); my $output = eval { $asset->dispatch($fragment); }; - if ( $@ ) { - $session->log->error( "Problem with dispatching $url: " . $@ ); + if( $@ ) { + my $e = WebGUI::Error->caught('WebGUI::Error'); + if( $session->request->env->{'webgui.debug'} ) { + $e->rethrow; + } else + { + $session->log->error( "Problem with dispatching $url: " . $e, $e ); + } } return $output if defined $output; } } $session->clearAsset; if ($session->isAdminOn) { - my $asset = WebGUI::Asset->newByUrl($session, $session->url->getRefererUrl) || WebGUI::Asset->getDefault($session); + my $asset = eval { WebGUI::Asset->newByUrl($session, $session->url->getRefererUrl) } || WebGUI::Asset->getDefault($session); return $asset->addMissing($assetUrl); } return undef; diff --git a/lib/WebGUI/Exception.pm b/lib/WebGUI/Exception.pm index 17c93d0e9..4d278490f 100644 --- a/lib/WebGUI/Exception.pm +++ b/lib/WebGUI/Exception.pm @@ -205,75 +205,75 @@ use Exception::Class ( 'WebGUI::Error' => { description => "A general error occured.", - }, + }, 'WebGUI::Error::OverrideMe' => { isa => 'WebGUI::Error', description => 'This method should be overridden by subclasses.', - }, + }, 'WebGUI::Error::MethodNotFound' => { isa => 'WebGUI::Error', description => q|Called a method that doesn't exist.|, fields => 'method' - }, + }, 'WebGUI::Error::InvalidObject' => { isa => 'WebGUI::Error::InvalidParam', description => "Expected to get a reference to an object type that wasn't gotten.", fields => ["expected","got"], - }, + }, 'WebGUI::Error::InvalidParam' => { isa => 'WebGUI::Error', description => "Expected to get a param we didn't get.", fields => ["param"], - }, + }, 'WebGUI::Error::Compile' => { isa => 'WebGUI::Error', description => "Unable to compile the requested class", fields => ["class", "cause"], - }, + }, 'WebGUI::Error::ObjectNotFound' => { isa => 'WebGUI::Error', description => "The object you were trying to retrieve does not exist.", fields => ["id"], - }, + }, 'WebGUI::Error::ObjectNotFound::Template' => { isa => 'WebGUI::Error', description => "The template an asset was trying to retrieve does not exist.", fields => [qw/templateId assetId/], - }, + }, 'WebGUI::Error::InvalidFile' => { isa => 'WebGUI::Error', description => "The file you have provided has errors.", fields => [qw{ brokenFile brokenLine }], - }, + }, 'WebGUI::Error::Template' => { isa => 'WebGUI::Error', description => "A template has errors that prevent it from being processed.", - }, + }, - 'WebGUI::Error::Connection' => { + 'WebGUI::Error::Connection' => { isa => 'WebGUI::Error', description => "Couldn't establish a connection.", fields => [qw{ resource }], - }, + }, 'WebGUI::Error::Fatal' => { isa => 'WebGUI::Error', @@ -290,6 +290,11 @@ use Exception::Class ( description => 'A module was requested that does not exist in the configuration file.', fields => [qw{ module configKey }], }, + + 'WebGUI::Error::RunTime' => { + isa => 'WebGUI::Error', + description => 'Perl runtime error.', + }, ); { diff --git a/lib/WebGUI/FormBuilder.pm b/lib/WebGUI/FormBuilder.pm index dae2f414b..848411a71 100644 --- a/lib/WebGUI/FormBuilder.pm +++ b/lib/WebGUI/FormBuilder.pm @@ -4,6 +4,7 @@ use strict; use WebGUI::BestPractices; use Moose; use MooseX::Storage; +use WebGUI::Exception; =head1 NAME diff --git a/lib/WebGUI/FormBuilder/Role/HasFields.pm b/lib/WebGUI/FormBuilder/Role/HasFields.pm index 56069a0c5..56cac015c 100644 --- a/lib/WebGUI/FormBuilder/Role/HasFields.pm +++ b/lib/WebGUI/FormBuilder/Role/HasFields.pm @@ -2,7 +2,7 @@ package WebGUI::FormBuilder::Role::HasFields; use strict; use Moose::Role; -use Try::Tiny; +use WebGUI::Exception; use Carp qw(confess); requires 'session', 'pack', 'unpack'; @@ -56,11 +56,13 @@ sub addField { # Load the class # Try to load the WebGUI Field first in case we conveniently overlap with a common name # (like Readonly) - if ( $INC{'WebGUI/Form/'. ucfirst $file} || try { local $SIG{'__DIE__'}; require 'WebGUI/Form/' . ucfirst $file } ) { + if ( $INC{'WebGUI/Form/'. ucfirst $file} || eval { require 'WebGUI/Form/' . ucfirst $file } ) { $type = 'WebGUI::Form::' . ucfirst $type; } - elsif ( !$INC{$file} && !try { require $file; } ) { - confess sprintf "Could not load form control class %s", $type; + elsif ( !$INC{$file} && ! eval { require $file; } ) { + my $e = WebGUI::Error->caught; + $e->{message} = "Could not load form control class $type"; + $e->rethrow; } $field = $type->new( $self->session, { @properties } ); } diff --git a/lib/WebGUI/Middleware/Session.pm b/lib/WebGUI/Middleware/Session.pm index 1f66360b8..4858da36b 100644 --- a/lib/WebGUI/Middleware/Session.pm +++ b/lib/WebGUI/Middleware/Session.pm @@ -47,13 +47,8 @@ sub call { $app = Plack::Middleware::SimpleLogger->wrap( $app ); } - my $session = try { - $env->{'webgui.session'} = WebGUI::Session->open( $config, $env ); - } catch { - # We don't have a logger object, so for now just warn() the error - warn "Unable to instantiate WebGUI::Session - $_"; - return; # make sure $session assignment is undef - }; + my $session = $env->{'webgui.session'} = WebGUI::Session->open( $config, $env ) or + die "Unable to instantiate WebGUI::Session - $_"; if ( !$session ) { diff --git a/lib/WebGUI/Middleware/StackTrace.pm b/lib/WebGUI/Middleware/StackTrace.pm index b0ffbb47e..6378f7214 100644 --- a/lib/WebGUI/Middleware/StackTrace.pm +++ b/lib/WebGUI/Middleware/StackTrace.pm @@ -14,49 +14,37 @@ use WebGUI::Session::Log; BEGIN { - our $StackTraceClass = "Devel::StackTrace"; - if (try { require Devel::StackTrace::WithLexicals; 1 }) { - # Optional since it needs PadWalker - $StackTraceClass = "Devel::StackTrace::WithLexicals"; - } - no warnings 'redefine'; - my $old_fatal = *WebGUI::Session::Log::fatal{CODE} || sub { }; + if (eval { require Devel::StackTrace::WithLexicals; 1 }) { + # Optional since it needs PadWalker - *WebGUI::Session::Log::fatal = sub { - my $self = shift; - my $message = shift; - $self->{_stacktrace} ||= $StackTraceClass->new; # favor the first stack trace - $self->{_message} ||= $message; - $old_fatal->($self, $message, @_); - }; - - my $old_error = *WebGUI::Session::Log::error{CODE}; - - *WebGUI::Session::Log::error = sub { - my $self = shift; - my $message = shift; - $self->{_stacktrace} ||= $StackTraceClass->new; - $self->{_message} ||= $message; - $old_error->($self, $message, @_); - }; + my $old_new = Devel::StackTrace->can('new'); + *Devel::StackTrace::new = sub { + my $self = $old_new ? $old_new->(@_) : { }; + bless $self, 'Devel::StackTrace::WithLexicals'; # rebless + }; + } } sub call { my($self, $env) = @_; - my $res = try { $self->app->($env) }; + # this won't be Middleware called by the .psgi in the default config unless $env->{'webgui.debug'} is true - if( my $trace = $env->{'webgui.session'}->log->{_stacktrace} ) { + local $SIG{__DIE__} = sub { + WebGUI::Error::RunTime->throw(error => $@); + }; - undef $env->{'webgui.session'}->log->{_stacktrace}; # the stack trace modules do create circular references; this is necessary - # this should also keep us from doing this work twice if we get stacked twice + my $res = try { $self->app->($env) }; # XXX this try is useless; plack doesn't let errors cross middlewares + + if( my $e = delete $env->{'webgui.error'} ) { + + my $trace = $e->trace; + my $message = $e->error; my $text = trace_as_string($trace); - my $message = $env->{'webgui.session'}->log->{_message}; - delete $env->{'webgui.session'}->log->{_message}; my @previous_html = $res && $res->[2] ? (map ref $_ ? @{ $_ } : $_, $res->[2]) : (); diff --git a/lib/WebGUI/Session/Log.pm b/lib/WebGUI/Session/Log.pm index 9d66f0d58..e20e568e1 100644 --- a/lib/WebGUI/Session/Log.pm +++ b/lib/WebGUI/Session/Log.pm @@ -19,7 +19,7 @@ use strict; use WebGUI::Paths; use WebGUI::Exception; use Sub::Uplevel; -use Scalar::Util qw(weaken); +use Scalar::Util qw(weaken blessed); =head1 NAME @@ -147,8 +147,15 @@ The message to use. sub fatal { my $self = shift; my $message = shift; + my $error_obj = shift; Sub::Uplevel::uplevel( 1, $self->getLogger, { level => 'fatal', message => $message}); - WebGUI::Error::Fatal->throw( error => $message ); + if( blessed $error_obj and $error_obj->can('rethrow') ) { + # Exception::Class objects have valuable stack traces built in to them; rethrow the existing error to preserve that if possible + $error_obj->rethrow; + } else + { + WebGUI::Error::Fatal->throw( error => $message ); + } } diff --git a/t/Asset/dispatch.t b/t/Asset/dispatch.t index 0189b2a3a..b3addf595 100644 --- a/t/Asset/dispatch.t +++ b/t/Asset/dispatch.t @@ -137,7 +137,8 @@ $session->request->setup_body( { WebGUI::Test->interceptLogging(sub { my $log_data = shift; is( $td->dispatch, "www_view", "if a query method throws a Template exception, view is returned instead" ); - is $log_data->{error}, 'Template not found templateId: This is a GUID assetId: '. $td->getId, '... and logged an error'; + my $template_id = $td->getId; + ok $log_data->{error} =~ m /Template not found/ && $log_data->{error} =~ m/templateId: / && $log_data->{error} =~ m/$template_id/, '... and logged an error'; }); WebGUI::Test->interceptLogging(sub { @@ -146,7 +147,7 @@ WebGUI::Test->interceptLogging(sub { func => 'dies', } ); is( $td->dispatch, "www_view", "if a query method dies, view is returned instead" ); - is $log_data->{warn}, "Couldn't call method www_dies on asset for url: / Root cause: ...aside from that bullet\n", '.. and logged a warn'; + ok $log_data->{error} =~ m/Couldn't call method/ && $log_data->{error} =~ m/www_dies/ && $log_data->{error} =~ m/\.\.\.aside from that bullet/, '.. and logged an error'; }); #vim:ft=perl