[WWcvs] webwork-modperl: I've changed errorOutput($error,$details) to accept a

[Mike G. v. a. <we...@ma...>]

Log Message:
-----------
I've changed errorOutput($error,$details) to accept a reference to a string
or to an array for $details.  These are automatically converted to the proper form.
References should be used when the $details points to a large number of bytes, such
as the contents of a file.

This version is not faster than the original method where $details was always a 
string, sometimes a very long string.  It does seem to save memory however.

Further savings in memory could be obtained by print directly, but there are a few instances
which call errorOutput and then do further formatting before printing.  

At the moment it doesn't seem to be worth it to make the change from return to print.

Modified Files:
--------------
    webwork-modperl/lib/WeBWorK:
        ContentGenerator.pm
    webwork-modperl/lib/WeBWorK/ContentGenerator:
        Hardcopy.pm

Revision Data
-------------
Index: ContentGenerator.pm
===================================================================
RCS file: /webwork/cvs/system/webwork-modperl/lib/WeBWorK/ContentGenerator.pm,v
retrieving revision 1.140
retrieving revision 1.141
diff -Llib/WeBWorK/ContentGenerator.pm -Llib/WeBWorK/ContentGenerator.pm -u -r1.140 -r1.141
--- lib/WeBWorK/ContentGenerator.pm
+++ lib/WeBWorK/ContentGenerator.pm
@@ -1671,7 +1671,18 @@
 		my %headers = $r->headers_in;
 		join("", map { CGI::Tr(CGI::td(CGI::small($_)), CGI::td(CGI::small($headers{$_}))) } keys %headers);
 	};
-	
+
+	# if it is a long report pass details by reference rather than by value
+	# for consistency we automatically convert all forms of $details into
+	# a reference to an array.
+
+	if (ref($details) =~ /SCALAR/i) {
+		$details = [$$details];
+	} elsif (ref($details) =~/ARRAY/i) {
+		# no change needed	
+	} else {
+	   $details = [$details];
+	}
 	return
 		CGI::h2("WeBWorK Error"),
 		CGI::p(<<EOF),
@@ -1681,9 +1692,16 @@
 you are a professor, please consult the error output below for more information.
 EOF
 		CGI::h3("Error messages"),
+
 		CGI::p(CGI::code($error)),
 		CGI::h3("Error details"),
-		CGI::code(CGI::p($details)),
+		
+		CGI::start_code(), CGI::start_p(),
+		@{ $details },
+		#CGI::code(CGI::p(@expandedDetails)), 
+		# not using inclusive CGI calls here saves about 30Meg of memory!
+		CGI::end_p(),CGI::end_code(),
+		
 		CGI::h3("Request information"),
 		CGI::table({border=>"1"},
 			CGI::Tr(CGI::td("Time"), CGI::td($time)),
@@ -1692,7 +1710,9 @@
 			CGI::Tr(CGI::td("HTTP Headers"), CGI::td(
 				CGI::table($headers),
 			)),
-		);
+		),
+	;  
+	
 }
 
 =item warningOutput($warnings)
Index: Hardcopy.pm
===================================================================
RCS file: /webwork/cvs/system/webwork-modperl/lib/WeBWorK/ContentGenerator/Hardcopy.pm,v
retrieving revision 1.56
retrieving revision 1.57
diff -Llib/WeBWorK/ContentGenerator/Hardcopy.pm -Llib/WeBWorK/ContentGenerator/Hardcopy.pm -u -r1.56 -r1.57
--- lib/WeBWorK/ContentGenerator/Hardcopy.pm
+++ lib/WeBWorK/ContentGenerator/Hardcopy.pm
@@ -47,6 +47,8 @@
 use WeBWorK::Utils qw(readFile makeTempDirectory);
 use Apache::Constants qw(:common REDIRECT);
 
+our $HardcopyTimer = new WeBWorK::Timing if $WeBWorK::Timing::Enabled;
+
 =head1 CONFIGURATION VARIABLES
 
 =over
@@ -71,6 +73,7 @@
 	my $authz = $r->authz;
 	my $userID  = $r->param("user");
 	
+	$HardcopyTimer->start if $WeBWorK::Timing::Enabled;;
 	my $singleSet       = $r->urlpath->arg("setID");
 	my @sets            = $r->param("hcSet");
 	my @users           = $r->param("hcUser");
@@ -185,14 +188,19 @@
 sub body {
 	my ($self) = @_;
 	
+	$HardcopyTimer->continue("Hardcopy: printing generation errors") if defined($HardcopyTimer);
 	if ($self->{generationError}) {
 		if (ref $self->{generationError} eq "ARRAY") {
 			my ($disposition, @rest) = @{$self->{generationError}};
 			if ($disposition eq "PGFAIL") {
 				$self->multiErrorOutput(@{$self->{errors}});
+				$HardcopyTimer->continue("Hardcopy: end printing generation errors") if defined($HardcopyTimer);
+	            $HardcopyTimer->save if defined($HardcopyTimer);
 				return "";
 			} elsif ($disposition eq "FAIL") {
 				print $self->errorOutput(@rest);
+				$HardcopyTimer->continue("Hardcopy: end printing generation errors") if defined($HardcopyTimer);
+	            $HardcopyTimer->save if defined($HardcopyTimer);
 				return "";
 			} elsif ($disposition eq "RETRY") {
 				print $self->errorOutput(@rest);
@@ -201,9 +209,14 @@
 			}
 		} else {
 			# not something we were expecting...
+			$HardcopyTimer->continue("Hardcopy: end printing generation errors") if defined($HardcopyTimer);
+	        $HardcopyTimer->save if defined($HardcopyTimer);
 			die $self->{generationError};
 		}
 	}
+	$HardcopyTimer->continue("Hardcopy: end printing generation errors") if defined($HardcopyTimer);
+	$HardcopyTimer->save if defined($HardcopyTimer);
+
 	if (@{$self->{warnings}}) {
 		# FIXME: this code will only be reached if there was also a
 		# generation error, because otherwise the module will send
@@ -441,7 +454,9 @@
 	my $pdfFileURL = undef;
 	if ($self->{hardcopy_format} eq 'pdf' ) {
 		my $errors = '';
+		$HardcopyTimer->continue("begin latex2pdf") if defined($HardcopyTimer);
 		$pdfFileURL = eval { $self->latex2pdf($tex, $tempDir, $fileName) };
+		$HardcopyTimer->continue("end latex2pdf") if defined($HardcopyTimer);
 		if ($@) {
 			$errors = $@;
 			#$errors =~ s/\n/<br>/g;  # make this readable on HTML FIXME make this a Utils. filter (Error2HTML)
@@ -546,30 +561,43 @@
 	# Alert the world that the tex file did not process perfectly.
 	if ($pdflatexResult) {
 		# something bad happened
-		my $textErrorMessage = "Call to $pdflatex failed: $!\n".CGI::br();
-		
+		my @textErrorMessage = ();
+		push @textErrorMessage , "Call to $pdflatex failed: $!\n",CGI::br();
+		if (-e $wd) {
+			push @textErrorMessage , "Working directory preserved at '$wd'.\n",
+			CGI::p("Investigating the contents of the working directory can be useful for debugging ",
+			 "errors which arise while processing the tex file, but it requires direct access to the server.\n"
+			);
+		} else {
+			push @textErrorMessage,  "Working directory $wd was not created.\n",CGI::br() ;
+		}
 		if (-e $hardcopyFilePath ) {
 			 # FIXME  Misuse of html tags!!!
-			$textErrorMessage.= "<h4>Some pdf output was produced and is available ". CGI::a({-href=>$hardcopyFileURL},"here.</h4>").CGI::hr();
+			push @textErrorMessage, CGI::h4("<h4>Some pdf output was produced and is available ",
+			     CGI::a({-href=>$hardcopyFileURL},"here.  ")),CGI::p("Looking at these
+			     fragments of typeset output can help with debugging."), CGI::hr();
 		}
 		# report logfile
 		if (-e $logFile) {
-			$textErrorMessage .= "pdflatex ran, but did not succeed. This suggests an error in the TeX\n".CGI::br();
-			$textErrorMessage .= "version of one of the problems, or a problem with the pdflatex system.\n".CGI::br();
+			push @textErrorMessage , "pdflatex ran, but did not succeed. This suggests an error in the TeX\n", CGI::br();
+			push @textErrorMessage , "version of one of the problems, or a problem with the pdflatex system.\n",CGI::br();
+			$HardcopyTimer->continue("Hardcopy: read log file") if defined($HardcopyTimer);
 			my $logFileContents = eval { readTexErrorLog($logFile) };
 			$logFileContents    .=  CGI::hr().CGI::hr();
+			$HardcopyTimer->continue("Hardcopy: format log file") if defined($HardcopyTimer);
 			$logFileContents    .= eval { formatTexFile($texFile)     };
+			$HardcopyTimer->continue("Hardcopy: end processing log file") if defined($HardcopyTimer);
 			if ($@) {
-				$textErrorMessage .= "Additionally, the pdflatex log file could not be read, though it exists.\n".CGI::br();
+				push @textErrorMessage, "Additionally, the pdflatex log file could not be read, though it exists.\n", CGI::br();
 			} else {
-				$textErrorMessage .= "The essential contents of the TeX log are as follows:\n".CGI::hr().CGI::br();
-				$textErrorMessage .= "$logFileContents\n".CGI::br().CGI::br();
+				push @textErrorMessage, "The essential contents of the TeX log are as follows:\n",CGI::hr(),CGI::br();
+				push @textErrorMessage, $logFileContents, CGI::br(), CGI::br();
 			}
 		} else {
-			$textErrorMessage .= "No log file was created, suggesting that pdflatex never ran. Check the WeBWorK\n".CGI::br();
-			$textErrorMessage .= "configuration to ensure that the path to pdflatex is correct.\n".CGI::br();
+			push @textErrorMessage, "No log file was created, suggesting that pdflatex never ran. Check the WeBWorK\n",CGI::br();
+			push @textErrorMessage, "configuration to ensure that the path to pdflatex is correct.\n", CGI::br();
 		}
-		die $textErrorMessage;
+		die \@textErrorMessage;
 	}
 	
 
@@ -717,7 +745,7 @@
 }
 
 sub getProblemTeX {
-    $WeBWorK::timer1 ->continue("hardcopy: begin processing problem") if defined($WeBWorK::timer1);
+    $HardcopyTimer ->continue("hardcopy: begin processing problem") if defined($HardcopyTimer);
 	my ($self, $effectiveUser, $setName, $problemNumber, $pgFile) = @_;
 	my $r      = $self->r;
 	my $ce     = $r->ce;
@@ -852,7 +880,7 @@
 			$pg->{body_text} .= $correctTeX;
 		}
 	}
-	$WeBWorK::timer1 ->continue("hardcopy: end processing problem") if defined($WeBWorK::timer1);
+	$HardcopyTimer ->continue("hardcopy: end processing problem") if defined($HardcopyTimer);
 	return $pg->{body_text};
 }