Reworked error handling to propogate errors downward, especially when webgui.debug is set

This commit is contained in:
Scott Walters 2011-07-26 01:26:03 -04:00
parent b815228a1b
commit e945a94c63
12 changed files with 132 additions and 110 deletions

View file

@ -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;

View file

@ -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

View file

@ -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
}

View file

@ -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 '<h1>' . $i18n->get('survey visualization') . '</h1>Survey Visualization requires the GraphViz module';
}

View file

@ -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;

View file

@ -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.',
},
);
{

View file

@ -4,6 +4,7 @@ use strict;
use WebGUI::BestPractices;
use Moose;
use MooseX::Storage;
use WebGUI::Exception;
=head1 NAME

View file

@ -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 } );
}

View file

@ -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 ) {

View file

@ -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]) : ();

View file

@ -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 );
}
}

View file

@ -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