Rework PG error/warning/debug message handling.

htdocs/js/System/system.scss

@@ -374,13 +374,6 @@ h1.page-title {
 	line-height: 1.4;
 }
 
-.Warnings {
-	code {
-		white-space: normal;
-		color: inherit;
-	}
-}
-
 .error-output {
 	word-wrap: break-word;
 	color: #d63384;

lib/WeBWorK/ContentGenerator.pm

@@ -813,7 +813,7 @@ there are pg errors.
 =cut
 
 sub have_warnings ($c) {
-	return $c->stash('warnings') || $c->{pgerrors};
+	return $c->stash('warnings');
 }
 
 =item exists_theme_file
@@ -1220,8 +1220,8 @@ Used to display a generic warning message at the top of the page
 =cut
 
 sub warningMessage ($c) {
-	return $c->maketext('<strong>Warning</strong>: There may be something wrong with this question. '
-			. 'Please inform your instructor including the warning messages below.');
+	return $c->maketext('<strong>Warning</strong>: WeBWorK has encountered warnings while processing your request. '
+			. 'See the warning messages below for details.');
 }
 
 =item $string = formatDateTime($date_time, $format_string, $timezone, $locale)

lib/WeBWorK/ContentGenerator/GatewayQuiz.pm

@@ -274,14 +274,15 @@ sub get_instructor_comment ($c, $problem) {
 
 async sub pre_header_initialize ($c) {
 	# Make sure these are defined for the templates.
-	$c->stash->{problems}        = [];
-	$c->stash->{pg_results}      = [];
-	$c->stash->{startProb}       = 0;
-	$c->stash->{endProb}         = 0;
-	$c->stash->{numPages}        = 0;
-	$c->stash->{pageNumber}      = 0;
-	$c->stash->{problem_numbers} = [];
-	$c->stash->{probOrder}       = [];
+	$c->stash->{problems}            = [];
+	$c->stash->{pg_results}          = [];
+	$c->stash->{startProb}           = 0;
+	$c->stash->{endProb}             = 0;
+	$c->stash->{numPages}            = 0;
+	$c->stash->{pageNumber}          = 0;
+	$c->stash->{problem_numbers}     = [];
+	$c->stash->{probOrder}           = [];
+	$c->stash->{haveProblemWarnings} = 0;
 
 	# If authz->checkSet has failed, then this set is invalid.  No need to proceeded.
 	return if $c->{invalidSet};
@@ -1447,11 +1448,6 @@ sub nav ($c, $args) {
 	return '';
 }
 
-sub warningMessage ($c) {
-	return $c->maketext('<strong>Warning</strong>: There may be something wrong with a question in this test. '
-			. 'Please inform your instructor including the warning messages below.');
-}
-
 # Evaluation utility
 # $effectiveUser is the current effective user, $set is the merged set version, $formFields is a reference to the
 # hash of parameters from the input form that need to be passed to the translator, and $mergedProblem
@@ -1510,16 +1506,14 @@ async sub getProblemHTML ($c, $effectiveUser, $set, $formFields, $mergedProblem)
 		},
 	);
 
-	# Warnings in the renderPG subprocess will not be caught by the global warning handler of this process.
-	# So rewarn them and let the global warning handler take care of it.
-	warn $pg->{warnings} if $pg->{warnings};
-
 	# 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} || '{}'))
 		if $c->{can}{checkAnswers}
 		&& (!$c->{submitAnswers} || defined $c->param('problem_data_' . $mergedProblem->problem_id));
 
+	$c->stash->{haveProblemWarnings} = 1 if $pg->{warnings} || @{ $pg->{pgwarning} // [] };
+
 	return $pg;
 }
 

lib/WeBWorK/ContentGenerator/Problem.pm

@@ -594,10 +594,6 @@ async sub pre_header_initialize ($c) {
 		}
 	);
 
-	# Warnings in the renderPG subprocess will not be caught by the global warning handler of this process.
-	# So rewarn them and let the global warning handler take care of it.
-	warn $pg->{warnings} if $pg->{warnings};
-
 	debug('end pg processing');
 
 	$pg->{body_text} .= $c->hidden_field(
@@ -609,20 +605,6 @@ async sub pre_header_initialize ($c) {
 	$can{showHints}     &&= $pg->{flags}{hintExists};
 	$can{showSolutions} &&= $pg->{flags}{solutionExists};
 
-	# Record errors
-	$c->{pgdebug}          = $pg->{debug_messages}          if ref $pg->{debug_messages} eq 'ARRAY';
-	$c->{pgwarning}        = $pg->{warning_messages}        if ref $pg->{warning_messages} eq 'ARRAY';
-	$c->{pginternalerrors} = $pg->{internal_debug_messages} if ref $pg->{internal_debug_messages} eq 'ARRAY';
-	# $c->{pgerrors} is defined if any of the above are defined, and is nonzero if any are non-empty.
-	$c->{pgerrors} = @{ $c->{pgdebug} // [] } || @{ $c->{pgwarning} // [] } || @{ $c->{pginternalerrors} // [] }
-		if defined $c->{pgdebug} || defined $c->{pgwarning} || defined $c->{pginternalerrors};
-
-	# If $c->{pgerrors} is not defined, then the PG messages arrays were not defined,
-	# which means $pg->{pgcore} was not defined and the translator died.
-	warn 'Processing of this PG problem was not completed.  Probably because of a syntax error. '
-		. 'The translator died prematurely and no PG warning messages were transmitted.'
-		unless defined $c->{pgerrors};
-
 	# Store fields
 	$c->{want} = \%want;
 	$c->{can}  = \%can;
@@ -635,53 +617,6 @@ async sub pre_header_initialize ($c) {
 	return;
 }
 
-sub warnings ($c) {
-	my $output = $c->c;
-
-	# Display warning messages
-	if (!defined $c->{pgerrors}) {
-		push(
-			@$output,
-			$c->tag(
-				'div',
-				$c->c(
-					$c->tag('h3', style => 'color:red;', $c->maketext('PG question failed to render')),
-					$c->tag('p',  $c->maketext('Unable to obtain error messages from within the PG question.'))
-				)->join('')
-			)
-		);
-	} elsif ($c->{pgerrors} > 0) {
-		my @pgdebug          = @{ $c->{pgdebug}          // [] };
-		my @pgwarning        = @{ $c->{pgwarning}        // [] };
-		my @pginternalerrors = @{ $c->{pginternalerrors} // [] };
-		push(
-			@$output,
-			$c->tag(
-				'div',
-				$c->c(
-					$c->tag('h2', $c->maketext('PG question processing error messages')),
-					@pgdebug ? $c->c(
-						$c->tag('h3', $c->maketext('PG debug messages')),
-						$c->tag('p',  $c->c(@pgdebug)->join($c->tag('br')))
-					)->join('') : '',
-					@pgwarning ? $c->c(
-						$c->tag('h3', $c->maketext('PG warning messages')),
-						$c->tag('p',  $c->c(@pgwarning)->join($c->tag('br')))
-					)->join('') : '',
-					@pginternalerrors ? $c->c(
-						$c->tag('h3', $c->maketext('PG internal errors')),
-						$c->tag('p',  $c->c(@pginternalerrors)->join($c->tag('br')))
-					)->join('') : ''
-				)->join('')
-			)
-		);
-	}
-
-	push(@$output, $c->SUPER::warnings());
-
-	return $output->join('');
-}
-
 sub head ($c) {
 	return ''                  if ($c->{invalidSet});
 	return $c->{pg}{head_text} if $c->{pg}{head_text};
@@ -1042,14 +977,7 @@ sub output_problem_body ($c) {
 		} else {
 			# For students render the body text of the problem with a message about error details.
 			return $c->c(
-				$c->tag(
-					'div',
-					id    => 'output_problem_body',
-					class => 'text-dark',
-					style => 'color-scheme: light',
-					data  => { bs_theme => 'light' },
-					$c->b($c->{pg}{body_text})
-				),
+				$c->tag('div', $c->b($c->{pg}{body_text})),
 				$c->include(
 					'ContentGenerator/Base/error_output',
 					error   => $c->{pg}{errors},
@@ -1534,7 +1462,8 @@ sub output_past_answer_button ($c) {
 				$c->hidden_field(selected_sets     => $c->{problem}->set_id),
 				$c->hidden_field(selected_users    => $c->{problem}->user_id),
 				$c->tag(
-					'p',
+					'div',
+					class => 'mb-3',
 					$c->submit_button(
 						$c->maketext('Show Past Answers'),
 						name  => 'action',

lib/WeBWorK/ContentGenerator/ShowMeAnother.pm

@@ -212,30 +212,12 @@ async sub pre_header_initialize ($c) {
 		}
 	);
 
-	# Warnings in the renderPG subprocess will not be caught by the global warning handler of this process.
-	# So rewarn them and let the global warning handler take care of it.
-	warn $pg->{warnings} if $pg->{warnings};
-
 	debug('end pg processing');
 
 	# Update and fix hint/solution options after PG processing
 	$c->{can}{showHints}     &&= $pg->{flags}{hintExists};
 	$c->{can}{showSolutions} &&= $pg->{flags}{solutionExists};
 
-	# Record errors
-	$c->{pgdebug}          = $pg->{debug_messages}          if ref $pg->{debug_messages} eq 'ARRAY';
-	$c->{pgwarning}        = $pg->{warning_messages}        if ref $pg->{warning_messages} eq 'ARRAY';
-	$c->{pginternalerrors} = $pg->{internal_debug_messages} if ref $pg->{internal_debug_messages} eq 'ARRAY';
-	# $c->{pgerrors} is defined if any of the above are defined, and is nonzero if any are non-empty.
-	$c->{pgerrors} = @{ $c->{pgdebug} // [] } || @{ $c->{pgwarning} // [] } || @{ $c->{pginternalerrors} // [] }
-		if defined $c->{pgdebug} || defined $c->{pgwarning} || defined $c->{pginternalerrors};
-
-	# If $c->{pgerrors} is not defined, then the PG messages arrays were not defined,
-	# which means $pg->{pgcore} was not defined and the translator died.
-	warn 'Processing of this PG problem was not completed.  Probably because of a syntax error. '
-		. 'The translator died prematurely and no PG warning messages were transmitted.'
-		unless defined $c->{pgerrors};
-
 	$c->{pg} = $pg;
 
 	return;

lib/WeBWorK/Utils/Rendering.pm

@@ -252,10 +252,9 @@ sub renderPG ($c, $effectiveUser, $set, $problem, $psvn, $formFields, $translati
 		};
 
 		if (ref($pg->{pgcore}) eq 'PGcore') {
-			$ret->{internal_debug_messages} = $pg->{pgcore}->get_internal_debug_messages;
-			$ret->{warning_messages}        = $pg->{pgcore}->get_warning_messages();
-			$ret->{debug_messages}          = $pg->{pgcore}->get_debug_messages();
-			$ret->{PG_ANSWERS_HASH}         = {
+			$ret->{warning_messages} = $pg->{pgcore}->get_warning_messages();
+			$ret->{debug_messages}   = $pg->{pgcore}->get_debug_messages();
+			$ret->{PG_ANSWERS_HASH}  = {
 				map {
 					$_ => {
 						response_obj => unbless($pg->{pgcore}{PG_ANSWERS_HASH}{$_}->response_obj),
@@ -269,6 +268,8 @@ sub renderPG ($c, $effectiveUser, $set, $problem, $psvn, $formFields, $translati
 					keys %{ $pg->{pgcore}{PG_alias}{resource_list} }
 			};
 			$ret->{PERSISTENCE_HASH} = $pg->{pgcore}{PERSISTENCE_HASH};
+		} else {
+			$ret->{render_fail} = 1;
 		}
 
 		# Save the problem source. This is used by Caliper::Entity. Why?
@@ -277,7 +278,7 @@ sub renderPG ($c, $effectiveUser, $set, $problem, $psvn, $formFields, $translati
 		$pg->free;
 		return $ret;
 	})->catch(sub ($err) {
-		return { body_text => '', answers => {}, flags => { error_flag => 1 }, errors => $err };
+		return { body_text => '', answers => {}, render_fail => 1, flags => { error_flag => 1 }, errors => $err };
 	});
 }
 

lib/WebworkWebservice/RenderProblem.pm

@@ -258,26 +258,23 @@ async sub renderProblem {
 
 	# New version of output:
 	return {
-		text                    => $pg->{body_text},
-		header_text             => $pg->{head_text},
-		post_header_text        => $pg->{post_header_text},
-		answers                 => $pg->{answers},
-		errors                  => $pg->{errors},
-		pg_warnings             => $pg->{warnings},
-		PG_ANSWERS_HASH         => $pg->{PG_ANSWERS_HASH},
-		PERSISTENCE_HASH        => $pg->{PERSISTENCE_HASH},
-		problem_result          => $pg->{result},
-		problem_state           => $pg->{state},
-		flags                   => $pg->{flags},
-		psvn                    => $psvn,
-		problem_seed            => $problemSeed,
-		resource_list           => $pg->{resource_list},
-		warning_messages        => ref $pg->{warning_messages} eq 'ARRAY' ? $pg->{warning_messages} : [],
-		debug_messages          => ref $pg->{debug_messages} eq 'ARRAY'   ? $pg->{debug_messages}   : [],
-		internal_debug_messages => ref $pg->{internal_debug_messages} eq 'ARRAY'
-		? $pg->{internal_debug_messages}
-		: [],
-		compute_time => logTimingInfo($beginTime, Benchmark->new),
+		text             => $pg->{body_text},
+		header_text      => $pg->{head_text},
+		post_header_text => $pg->{post_header_text},
+		answers          => $pg->{answers},
+		errors           => $pg->{errors},
+		pg_warnings      => $pg->{warnings},
+		PG_ANSWERS_HASH  => $pg->{PG_ANSWERS_HASH},
+		PERSISTENCE_HASH => $pg->{PERSISTENCE_HASH},
+		problem_result   => $pg->{result},
+		problem_state    => $pg->{state},
+		flags            => $pg->{flags},
+		psvn             => $psvn,
+		problem_seed     => $problemSeed,
+		resource_list    => $pg->{resource_list},
+		warning_messages => ref $pg->{warning_messages} eq 'ARRAY' ? $pg->{warning_messages} : [],
+		debug_messages   => ref $pg->{debug_messages} eq 'ARRAY'   ? $pg->{debug_messages}   : [],
+		compute_time     => logTimingInfo($beginTime, Benchmark->new),
 	};
 }
 

templates/ContentGenerator/Base/error_output.html.ep

@@ -5,9 +5,8 @@
 % }
 <p>
 	<%= maketext(
-		'WeBWorK has encountered a software error while attempting to process this problem. It is likely that '
-		. 'there is an error in the problem itself. If you are a student, report this error message to your '
-		. 'professor to have it corrected. If you are a professor, please consult the error output below for '
+		'WeBWorK has encountered a software error. If you are a student, report this error message to your '
+		. 'instructor to have it corrected. If you are a instructor, please consult the error output below for '
 		. 'more information.'
 		) %>
 </p>

templates/ContentGenerator/Base/feedback_macro_email.html.ep

@@ -6,6 +6,8 @@
 		% next if $key eq 'pg_object';    # Not used in internal feedback mechanism
 		<%= hidden_field $key => $value =%>
 	% }
-	<%= submit_button maketext($ce->{feedback_button_name}) || maketext('Email instructor'),
-		name => 'feedbackForm', class => 'btn btn-primary' =%>
+	<div class="mb-3">
+		<%= submit_button maketext($ce->{feedback_button_name}) || maketext('Email instructor'),
+			name => 'feedbackForm', class => 'btn btn-primary' =%>
+	</div>
 % end

templates/ContentGenerator/Base/problem_warning_and_debug_output.html.ep

@@ -0,0 +1,34 @@
+% if ($warnings) {
+	<div class="row g-0 mb-3">
+		<div class="col-12 alert alert-danger m-0">
+			<h2><%= maketext('PG warning messages') %></h2>
+			<ul class="m-0">
+				% for (split m/\n+/, $warnings) {
+					<li><div><%== $_ %></div></li>
+				% }
+			</ul>
+		</div>
+	</div>
+% }
+% if (@$warning_messages) {
+	<div class="row g-0 mb-3">
+		<div class="col-12 alert alert-danger m-0">
+			<h2><%= maketext('PG processing warning messages') %></h2>
+			<ul class="m-0">
+				% for (@$warning_messages) {
+					<li><div><%== $_ %></div></li>
+				% }
+			</ul>
+		</div>
+	</div>
+% }
+% if (@$debug_messages) {
+	<div class="row g-0 mb-3">
+		<div class="col-12 alert alert-info m-0 overflow-x-auto">
+			<h2><%= maketext('PG debug messages') %></h2>
+			% for (@$debug_messages) {
+				<div class="my-3"><%== $_ %></div>
+			% }
+		</div>
+	</div>
+% }

templates/ContentGenerator/Base/warning_output.html.ep

@@ -3,17 +3,15 @@
 <h2><%= maketext('WeBWorK Warnings') %></h2>
 <p>
 	<%= maketext(
-		'WeBWorK has encountered warnings while processing your request. If this occurred when viewing '
-			. 'a problem, it was likely caused by an error or ambiguity in that problem. Otherwise, it may indicate '
-			. 'a problem with the WeBWorK system itself. If you are a student, report these warnings to your '
-			. 'professor to have them corrected. If you are a professor, please consult the warning output below '
-			. 'for more information.'
+		'WeBWorK has encountered warnings while processing your request. This indicates a problem with the WeBWorK '
+			. 'system. If you are a student, report these warnings to your instructor to have them corrected. If you '
+			. 'are a instructor, please consult the warning output below for more information.'
 	) %>
 </p>
 <h3><%= maketext('Warning messages') %></h3>
 <ul>
 	% for (@$warnings) {
-		<li><code><%= $_ %></code></li>
+		<li><div><%= $_ %></div></li>
 	% }
 </ul>
 <h3><%= maketext('Request information') %></h3>