Rework error handling when a problem fails to render in a test.

First, fix a general issue when a problem fails to render that results
in a message that has no relation to the actual problem. The issue is
that the `create_ans_str_from_responses` method of the
`WeBWorK::Utils::ProblemProcessing` module is called with a `$pg` hash
that doesn't contain the right things, and the method is not safe
guarded against that. This fixes issue #2829.

Second, rework how error handling is done in tests.  Instead of just
rendering the errors for problems that failed to render, still render
all problems on the page.  Show the errors for the problems that failed,
but still render all of the problems that didn't fail.  This means, that
the problem navigation is still shown.   In fact the entire page renders
normally, but the error message is shown for the problems that fail to
render.  So the student can at least continue to work the rest of the
test normally.  The student can even grade the test on a page for which
a problem fails to render.

Note that the "request information" is no longer shown for these
messages when in a test.  A special `briefErrorOutput` stash value can
be set when the `templates/ContentGenerator/Base/error_output.html.ep`
template is rendered to skip that. Although, I really thing that
information should never be shown in the page for any exception that
occurs. I don't think that the time, method, uri, and HTTP headers of
the request, ever help with these exceptions.

Author: drgrice1

lib/WeBWorK/AchievementEvaluator.pm

@@ -16,7 +16,7 @@ use WeBWorK::WWSafe;
 
 our @EXPORT_OK = qw(checkForAchievements);
 
-sub checkForAchievements ($problem_in, $pg, $c, %options) {
+sub checkForAchievements ($problem_in, $c, %options) {
 	my $db = $c->db;
 	my $ce = $c->ce;
 

lib/WeBWorK/ContentGenerator/GatewayQuiz.pm

@@ -887,9 +887,6 @@ async sub pre_header_initialize ($c) {
 	my @problems;
 	my @pg_results;
 
-	# pg errors are stored here.
-	$c->{errors} = [];
-
 	# Process the problems as needed.
 	my @mergedProblems;
 	if ($setID eq 'Undefined_Set') {
@@ -1511,17 +1508,6 @@ async sub getProblemHTML ($c, $effectiveUser, $set, $formFields, $mergedProblem)
 	# So rewarn them and let the global warning handler take care of it.
 	warn $pg->{warnings} if $pg->{warnings};
 
-	if ($pg->{flags}{error_flag}) {
-		push @{ $c->{errors} },
-			{
-				set     => $set->set_id . ',v' . $set->version_id,
-				problem => $mergedProblem->problem_id,
-				message => $pg->{errors},
-				context => $pg->{body_text},
-			};
-		$pg->{body_text} = undef;
-	}
-
 	# If the user can check answers and either this is not an answer submission or the problem_data form
 	# parameter was previously set, then set or update the problem_data form parameter.
 	$c->param('problem_data_' . $mergedProblem->problem_id => encode_json($pg->{PERSISTENCE_HASH} || '{}'))

lib/WeBWorK/ContentGenerator/Problem.pm

@@ -1549,7 +1549,7 @@ sub output_achievement_message ($c) {
 		&& $c->{submitAnswers}
 		&& $c->{problem}->set_id ne 'Undefined_Set')
 	{
-		return checkForAchievements($c->{problem}, $c->{pg}, $c);
+		return checkForAchievements($c->{problem}, $c);
 	}
 
 	return '';

lib/WeBWorK/Utils/ProblemProcessing.pm

@@ -298,7 +298,7 @@ sub create_ans_str_from_responses ($formFields, $pg, $needed_grading = 0) {
 	my @past_answers_order;
 	my @last_answer_order;
 
-	my %pg_answers_hash = %{ $pg->{PG_ANSWERS_HASH} };
+	my %pg_answers_hash = %{ $pg->{PG_ANSWERS_HASH} // {} };
 	for my $ans_id (@{ $pg->{flags}{ANSWER_ENTRY_ORDER} // [] }) {
 		$scores_string .= ($pg_answers_hash{$ans_id}{rh_ans}{score} // 0) >= 1 ? '1' : '0';
 		push @answerTypes, $pg_answers_hash{$ans_id}{rh_ans}{type} // '';
@@ -326,7 +326,7 @@ sub create_ans_str_from_responses ($formFields, $pg, $needed_grading = 0) {
 	# KEPT_EXTRA_ANSWERS needs to be stored in last_answer in order to preserve persistence items.
 	# The persistence items do not need to be stored in past_answers_string.
 	# Don't add _ext_data items.  Those are stored elsewhere.
-	for my $entry_id (@{ $pg->{flags}{KEPT_EXTRA_ANSWERS} }) {
+	for my $entry_id (@{ $pg->{flags}{KEPT_EXTRA_ANSWERS} // [] }) {
 		next if exists($answers_to_store{$entry_id}) || $entry_id =~ /^_ext_data/;
 		$answers_to_store{$entry_id} = $formFields->{$entry_id};
 		push @last_answer_order, $entry_id;

templates/ContentGenerator/Base/error_output.html.ep

@@ -1,6 +1,8 @@
 % use Date::Format;
 %
-<h2><%= maketext('WeBWorK Error') %></h2>
+% unless (stash->{briefErrorOutput}) {
+	<h2><%= maketext('WeBWorK Error') %></h2>
+% }
 <p>
 	<%= maketext(
 		'WeBWorK has encountered a software error while attempting to process this problem. It is likely that '
@@ -15,6 +17,7 @@
 <div class="error-output">
 	<%== ref $details =~ /SCALAR/i ? $$details : ref $details =~ /ARRAY/i ? join('', @$details) : $details %>
 </div>
+% last if stash->{briefErrorOutput};
 <h3><%= maketext('Request information') %></h3>
 <table class="table-bordered mb-2">
 	<tr>

templates/ContentGenerator/GatewayQuiz.html.ep

@@ -108,25 +108,6 @@
 	% last;
 % }
 %
-% # If there were translation errors, then show those and exit.
-% if (@{ $c->{errors} }) {
-	% my $errorNum = 1;
-	% my ($message, $context) = (c, c);
-	% for (@{ $c->{errors} }) {
-		% push(@$message, "$errorNum. ") if (@{ $c->{errors} } > 1);
-		% push(@$message, $_->{message}, tag('br'));
-		%
-		% my $line = begin
-			<p><%= (@{ $c->{errors} } > 1 ? "$errorNum." : '') %><%== $_->{context} %></p>
-			<div class="gwDivider"></div>
-		% end
-		% push @$context, $line->();
-	% }
-	<%= include 'ContentGenerator/Base/error_output', error => $message->join(''), details => $context->join('') =%>
-	%
-	% last;
-% }
-%
 % my $setID          = $c->{set}->set_id;
 % my $setVersionID   = $c->{set}->version_id;
 % my $numProbPerPage = $c->{set}->problems_per_page;
@@ -405,7 +386,7 @@
 	% # Problems can be shown, so output the main form and the problems.
 	% my $startTime = param('startTime') || time;
 	%
-	<%= form_for $action, name => 'gwquiz', method => 'POST', class => 'problem-main-form', begin =%>
+	<%= form_for $action, name => 'gwquiz', method => 'POST', class => 'problem-main-form mt-0', begin =%>
 		<%= $c->hidden_authen_fields =%>
 		%
 		% # Hacks to use a javascript link to trigger previews and jump to subsequent pages of a multipage test.
@@ -603,11 +584,21 @@
 						</div>
 					% }
 					%
-					<div class="problem-content col-lg-10" <%== get_problem_lang_and_dir(
-							$pg->{flags}, $ce->{perProblemLangAndDirSettingMode}, $ce->{language}
-						) %>>
-						<%== $pg->{body_text} =%>
-					</div>
+					% # If there were translation errors, then show those instead of the problem.
+					% if ($pg->{flags}{error_flag}) {
+						% stash->{briefErrorOutput} = 1;
+						<%= include 'ContentGenerator/Base/error_output',
+							error => $pg->{errors},
+							details => $pg->{body_text} =%>
+						%
+						% delete stash->{briefErrorOutput};
+					% } else {
+						<div class="problem-content col-lg-10" <%== get_problem_lang_and_dir(
+								$pg->{flags}, $ce->{perProblemLangAndDirSettingMode}, $ce->{language}
+							) %>>
+							<%== $pg->{body_text} =%>
+						</div>
+					% }
 					% if ($pg->{result}{msg}) {
 						<div class="mb-2"><b><%== maketext('Note: [_1]', tag('i', $pg->{result}{msg})) %></b></div>
 					% }
@@ -651,7 +642,7 @@
 					% }
 					%
 					% # Initialize the problem graders for the problem.
-					% if ($c->{will}{showProblemGrader}) {
+					% if ($c->{will}{showProblemGrader} && !$pg->{flags}{error_flag}) {
 						<%= WeBWorK::HTML::SingleProblemGrader->new($c, $pg, $problems->[ $probOrder->[$i] ])
 							->insertGrader =%>
 					% }
@@ -785,5 +776,5 @@
 % # If achievements enabled, check to see if there are new ones and output them.  Use the first
 % # problem to seed the data. However, all of the problems will be provided to the achievement evaluator.
 % if ($ce->{achievementsEnabled} && $c->{will}{recordAnswers} && $c->{submitAnswers} && $setID ne 'Undefined_Set') {
-	<%= checkForAchievements($problems->[0], $pg_results->[0], $c, setVersion => $setVersionID) =%>
+	<%= checkForAchievements($problems->[0], $c, setVersion => $setVersionID) =%>
 % }