Skip to content

Commit c76dc2b

Browse files
committed
Rework PG error/warning/debug message handling.
The PG messages are now dealt with completely separately from the webwork2 general warning handling. Furthermore PG warnings and debug messages are separately dealt with. The PG `internal_debug_messages` are no longer dealt with at all. They are unnecessary and inappropriately named, and will be removed from PG. Also, handling of the warnings is now much more consistent in all of the different places that problems are rendered.
1 parent cc358e3 commit c76dc2b

14 files changed

Lines changed: 118 additions & 183 deletions

File tree

htdocs/js/System/system.scss

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -374,13 +374,6 @@ h1.page-title {
374374
line-height: 1.4;
375375
}
376376

377-
.Warnings {
378-
code {
379-
white-space: normal;
380-
color: inherit;
381-
}
382-
}
383-
384377
.error-output {
385378
word-wrap: break-word;
386379
color: #d63384;

lib/WeBWorK/ContentGenerator.pm

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -813,7 +813,7 @@ there are pg errors.
813813
=cut
814814

815815
sub have_warnings ($c) {
816-
return $c->stash('warnings') || $c->{pgerrors};
816+
return $c->stash('warnings');
817817
}
818818

819819
=item exists_theme_file
@@ -1220,8 +1220,8 @@ Used to display a generic warning message at the top of the page
12201220
=cut
12211221

12221222
sub warningMessage ($c) {
1223-
return $c->maketext('<strong>Warning</strong>: There may be something wrong with this question. '
1224-
. 'Please inform your instructor including the warning messages below.');
1223+
return $c->maketext('<strong>Warning</strong>: WeBWorK has encountered warnings while processing your request. '
1224+
. 'See the warning messages below for details.');
12251225
}
12261226

12271227
=item $string = formatDateTime($date_time, $format_string, $timezone, $locale)

lib/WeBWorK/ContentGenerator/GatewayQuiz.pm

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1510,10 +1510,6 @@ async sub getProblemHTML ($c, $effectiveUser, $set, $formFields, $mergedProblem)
15101510
},
15111511
);
15121512

1513-
# Warnings in the renderPG subprocess will not be caught by the global warning handler of this process.
1514-
# So rewarn them and let the global warning handler take care of it.
1515-
warn $pg->{warnings} if $pg->{warnings};
1516-
15171513
# If the user can check answers and either this is not an answer submission or the problem_data form
15181514
# parameter was previously set, then set or update the problem_data form parameter.
15191515
$c->param('problem_data_' . $mergedProblem->problem_id => encode_json($pg->{PERSISTENCE_HASH} || '{}'))

lib/WeBWorK/ContentGenerator/Problem.pm

Lines changed: 3 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -594,10 +594,6 @@ async sub pre_header_initialize ($c) {
594594
}
595595
);
596596

597-
# Warnings in the renderPG subprocess will not be caught by the global warning handler of this process.
598-
# So rewarn them and let the global warning handler take care of it.
599-
warn $pg->{warnings} if $pg->{warnings};
600-
601597
debug('end pg processing');
602598

603599
$pg->{body_text} .= $c->hidden_field(
@@ -610,18 +606,8 @@ async sub pre_header_initialize ($c) {
610606
$can{showSolutions} &&= $pg->{flags}{solutionExists};
611607

612608
# Record errors
613-
$c->{pgdebug} = $pg->{debug_messages} if ref $pg->{debug_messages} eq 'ARRAY';
614-
$c->{pgwarning} = $pg->{warning_messages} if ref $pg->{warning_messages} eq 'ARRAY';
615-
$c->{pginternalerrors} = $pg->{internal_debug_messages} if ref $pg->{internal_debug_messages} eq 'ARRAY';
616-
# $c->{pgerrors} is defined if any of the above are defined, and is nonzero if any are non-empty.
617-
$c->{pgerrors} = @{ $c->{pgdebug} // [] } || @{ $c->{pgwarning} // [] } || @{ $c->{pginternalerrors} // [] }
618-
if defined $c->{pgdebug} || defined $c->{pgwarning} || defined $c->{pginternalerrors};
619-
620-
# If $c->{pgerrors} is not defined, then the PG messages arrays were not defined,
621-
# which means $pg->{pgcore} was not defined and the translator died.
622-
warn 'Processing of this PG problem was not completed. Probably because of a syntax error. '
623-
. 'The translator died prematurely and no PG warning messages were transmitted.'
624-
unless defined $c->{pgerrors};
609+
$c->{pgdebug} = $pg->{debug_messages} if ref $pg->{debug_messages} eq 'ARRAY';
610+
$c->{pgwarning} = $pg->{warning_messages} if ref $pg->{warning_messages} eq 'ARRAY';
625611

626612
# Store fields
627613
$c->{want} = \%want;
@@ -635,53 +621,6 @@ async sub pre_header_initialize ($c) {
635621
return;
636622
}
637623

638-
sub warnings ($c) {
639-
my $output = $c->c;
640-
641-
# Display warning messages
642-
if (!defined $c->{pgerrors}) {
643-
push(
644-
@$output,
645-
$c->tag(
646-
'div',
647-
$c->c(
648-
$c->tag('h3', style => 'color:red;', $c->maketext('PG question failed to render')),
649-
$c->tag('p', $c->maketext('Unable to obtain error messages from within the PG question.'))
650-
)->join('')
651-
)
652-
);
653-
} elsif ($c->{pgerrors} > 0) {
654-
my @pgdebug = @{ $c->{pgdebug} // [] };
655-
my @pgwarning = @{ $c->{pgwarning} // [] };
656-
my @pginternalerrors = @{ $c->{pginternalerrors} // [] };
657-
push(
658-
@$output,
659-
$c->tag(
660-
'div',
661-
$c->c(
662-
$c->tag('h2', $c->maketext('PG question processing error messages')),
663-
@pgdebug ? $c->c(
664-
$c->tag('h3', $c->maketext('PG debug messages')),
665-
$c->tag('p', $c->c(@pgdebug)->join($c->tag('br')))
666-
)->join('') : '',
667-
@pgwarning ? $c->c(
668-
$c->tag('h3', $c->maketext('PG warning messages')),
669-
$c->tag('p', $c->c(@pgwarning)->join($c->tag('br')))
670-
)->join('') : '',
671-
@pginternalerrors ? $c->c(
672-
$c->tag('h3', $c->maketext('PG internal errors')),
673-
$c->tag('p', $c->c(@pginternalerrors)->join($c->tag('br')))
674-
)->join('') : ''
675-
)->join('')
676-
)
677-
);
678-
}
679-
680-
push(@$output, $c->SUPER::warnings());
681-
682-
return $output->join('');
683-
}
684-
685624
sub head ($c) {
686625
return '' if ($c->{invalidSet});
687626
return $c->{pg}{head_text} if $c->{pg}{head_text};
@@ -1042,14 +981,7 @@ sub output_problem_body ($c) {
1042981
} else {
1043982
# For students render the body text of the problem with a message about error details.
1044983
return $c->c(
1045-
$c->tag(
1046-
'div',
1047-
id => 'output_problem_body',
1048-
class => 'text-dark',
1049-
style => 'color-scheme: light',
1050-
data => { bs_theme => 'light' },
1051-
$c->b($c->{pg}{body_text})
1052-
),
984+
$c->tag('div', $c->b($c->{pg}{body_text})),
1053985
$c->include(
1054986
'ContentGenerator/Base/error_output',
1055987
error => $c->{pg}{errors},

lib/WeBWorK/ContentGenerator/ShowMeAnother.pm

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -223,18 +223,8 @@ async sub pre_header_initialize ($c) {
223223
$c->{can}{showSolutions} &&= $pg->{flags}{solutionExists};
224224

225225
# Record errors
226-
$c->{pgdebug} = $pg->{debug_messages} if ref $pg->{debug_messages} eq 'ARRAY';
227-
$c->{pgwarning} = $pg->{warning_messages} if ref $pg->{warning_messages} eq 'ARRAY';
228-
$c->{pginternalerrors} = $pg->{internal_debug_messages} if ref $pg->{internal_debug_messages} eq 'ARRAY';
229-
# $c->{pgerrors} is defined if any of the above are defined, and is nonzero if any are non-empty.
230-
$c->{pgerrors} = @{ $c->{pgdebug} // [] } || @{ $c->{pgwarning} // [] } || @{ $c->{pginternalerrors} // [] }
231-
if defined $c->{pgdebug} || defined $c->{pgwarning} || defined $c->{pginternalerrors};
232-
233-
# If $c->{pgerrors} is not defined, then the PG messages arrays were not defined,
234-
# which means $pg->{pgcore} was not defined and the translator died.
235-
warn 'Processing of this PG problem was not completed. Probably because of a syntax error. '
236-
. 'The translator died prematurely and no PG warning messages were transmitted.'
237-
unless defined $c->{pgerrors};
226+
$c->{pgdebug} = $pg->{debug_messages} if ref $pg->{debug_messages} eq 'ARRAY';
227+
$c->{pgwarning} = $pg->{warning_messages} if ref $pg->{warning_messages} eq 'ARRAY';
238228

239229
$c->{pg} = $pg;
240230

lib/WeBWorK/Utils/Rendering.pm

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -252,10 +252,9 @@ sub renderPG ($c, $effectiveUser, $set, $problem, $psvn, $formFields, $translati
252252
};
253253

254254
if (ref($pg->{pgcore}) eq 'PGcore') {
255-
$ret->{internal_debug_messages} = $pg->{pgcore}->get_internal_debug_messages;
256-
$ret->{warning_messages} = $pg->{pgcore}->get_warning_messages();
257-
$ret->{debug_messages} = $pg->{pgcore}->get_debug_messages();
258-
$ret->{PG_ANSWERS_HASH} = {
255+
$ret->{warning_messages} = $pg->{pgcore}->get_warning_messages();
256+
$ret->{debug_messages} = $pg->{pgcore}->get_debug_messages();
257+
$ret->{PG_ANSWERS_HASH} = {
259258
map {
260259
$_ => {
261260
response_obj => unbless($pg->{pgcore}{PG_ANSWERS_HASH}{$_}->response_obj),
@@ -269,6 +268,8 @@ sub renderPG ($c, $effectiveUser, $set, $problem, $psvn, $formFields, $translati
269268
keys %{ $pg->{pgcore}{PG_alias}{resource_list} }
270269
};
271270
$ret->{PERSISTENCE_HASH} = $pg->{pgcore}{PERSISTENCE_HASH};
271+
} else {
272+
$ret->{render_fail} = 1;
272273
}
273274

274275
# Save the problem source. This is used by Caliper::Entity. Why?
@@ -277,7 +278,7 @@ sub renderPG ($c, $effectiveUser, $set, $problem, $psvn, $formFields, $translati
277278
$pg->free;
278279
return $ret;
279280
})->catch(sub ($err) {
280-
return { body_text => '', answers => {}, flags => { error_flag => 1 }, errors => $err };
281+
return { body_text => '', answers => {}, render_fail => 1, flags => { error_flag => 1 }, errors => $err };
281282
});
282283
}
283284

lib/WebworkWebservice/RenderProblem.pm

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -258,26 +258,23 @@ async sub renderProblem {
258258

259259
# New version of output:
260260
return {
261-
text => $pg->{body_text},
262-
header_text => $pg->{head_text},
263-
post_header_text => $pg->{post_header_text},
264-
answers => $pg->{answers},
265-
errors => $pg->{errors},
266-
pg_warnings => $pg->{warnings},
267-
PG_ANSWERS_HASH => $pg->{PG_ANSWERS_HASH},
268-
PERSISTENCE_HASH => $pg->{PERSISTENCE_HASH},
269-
problem_result => $pg->{result},
270-
problem_state => $pg->{state},
271-
flags => $pg->{flags},
272-
psvn => $psvn,
273-
problem_seed => $problemSeed,
274-
resource_list => $pg->{resource_list},
275-
warning_messages => ref $pg->{warning_messages} eq 'ARRAY' ? $pg->{warning_messages} : [],
276-
debug_messages => ref $pg->{debug_messages} eq 'ARRAY' ? $pg->{debug_messages} : [],
277-
internal_debug_messages => ref $pg->{internal_debug_messages} eq 'ARRAY'
278-
? $pg->{internal_debug_messages}
279-
: [],
280-
compute_time => logTimingInfo($beginTime, Benchmark->new),
261+
text => $pg->{body_text},
262+
header_text => $pg->{head_text},
263+
post_header_text => $pg->{post_header_text},
264+
answers => $pg->{answers},
265+
errors => $pg->{errors},
266+
pg_warnings => $pg->{warnings},
267+
PG_ANSWERS_HASH => $pg->{PG_ANSWERS_HASH},
268+
PERSISTENCE_HASH => $pg->{PERSISTENCE_HASH},
269+
problem_result => $pg->{result},
270+
problem_state => $pg->{state},
271+
flags => $pg->{flags},
272+
psvn => $psvn,
273+
problem_seed => $problemSeed,
274+
resource_list => $pg->{resource_list},
275+
warning_messages => ref $pg->{warning_messages} eq 'ARRAY' ? $pg->{warning_messages} : [],
276+
debug_messages => ref $pg->{debug_messages} eq 'ARRAY' ? $pg->{debug_messages} : [],
277+
compute_time => logTimingInfo($beginTime, Benchmark->new),
281278
};
282279
}
283280

templates/ContentGenerator/Base/error_output.html.ep

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,8 @@
55
% }
66
<p>
77
<%= maketext(
8-
'WeBWorK has encountered a software error while attempting to process this problem. It is likely that '
9-
. 'there is an error in the problem itself. If you are a student, report this error message to your '
10-
. 'professor to have it corrected. If you are a professor, please consult the error output below for '
8+
'WeBWorK has encountered a software error. If you are a student, report this error message to your '
9+
. 'instructor to have it corrected. If you are a instructor, please consult the error output below for '
1110
. 'more information.'
1211
) %>
1312
</p>
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
% if ($warnings) {
2+
<div class="row g-0 mt-3">
3+
<div class="col-12 alert alert-danger m-0">
4+
<h2><%= maketext('PG warning messages') %></h2>
5+
<ul class="m-0">
6+
% for (split m/\n+/, $warnings) {
7+
<li><div><%== $_ %></div></li>
8+
% }
9+
</ul>
10+
</div>
11+
</div>
12+
% }
13+
% if (@$warning_messages) {
14+
<div class="row g-0 mt-3">
15+
<div class="col-12 alert alert-danger m-0">
16+
<h2><%= maketext('PG processing warning messages') %></h2>
17+
<ul class="m-0">
18+
% for (@$warning_messages) {
19+
<li><div><%== $_ %></div></li>
20+
% }
21+
</ul>
22+
</div>
23+
</div>
24+
% }
25+
% if (@$debug_messages) {
26+
<div class="row g-0 mt-3">
27+
<div class="col-12 alert alert-info m-0 overflow-x-auto">
28+
<h2><%= maketext('PG debug messages') %></h2>
29+
% for (@$debug_messages) {
30+
<div class="my-3"><%== $_ %></div>
31+
% }
32+
</div>
33+
</div>
34+
% }

templates/ContentGenerator/Base/warning_output.html.ep

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,15 @@
33
<h2><%= maketext('WeBWorK Warnings') %></h2>
44
<p>
55
<%= maketext(
6-
'WeBWorK has encountered warnings while processing your request. If this occurred when viewing '
7-
. 'a problem, it was likely caused by an error or ambiguity in that problem. Otherwise, it may indicate '
8-
. 'a problem with the WeBWorK system itself. If you are a student, report these warnings to your '
9-
. 'professor to have them corrected. If you are a professor, please consult the warning output below '
10-
. 'for more information.'
6+
'WeBWorK has encountered warnings while processing your request. This indicates a problem with the WeBWorK '
7+
. 'system. If you are a student, report these warnings to your instructor to have them corrected. If you '
8+
. 'are a instructor, please consult the warning output below for more information.'
119
) %>
1210
</p>
1311
<h3><%= maketext('Warning messages') %></h3>
1412
<ul>
1513
% for (@$warnings) {
16-
<li><code><%= $_ %></code></li>
14+
<li><div><%= $_ %></div></li>
1715
% }
1816
</ul>
1917
<h3><%= maketext('Request information') %></h3>

0 commit comments

Comments
 (0)