Fix corruption of ZIP files and improve error reporting (#6)

* make sure responsetext is not null * avoid HTML error output bleeding into the ZIP archive data stream * include detailed error report in ZIP archive, if errors occur
PhilippImhof · Aug 7, 2024 · 0bed30d · 0bed30d
1 parent aa7f0e6
commit 0bed30d
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 14 deletions.
diff --git a/lang/en/quiz_essaydownload.php b/lang/en/quiz_essaydownload.php
@@ -25,6 +25,8 @@
 
 $string['byattempt'] = 'Attempt';
 $string['byquestion'] = 'Question';
+$string['errorfilename'] = 'error-{$a}.txt';
+$string['errormessage'] = 'An internal error occurred. The archive is probably incomplete. Please contact the developers of the Essay responses downloader plugin (quiz_essaydownload) and send them the details below:';
 $string['essaydownload'] = 'Download essay responses';
 $string['groupby'] = 'Group by';
 $string['groupby_help'] = 'The archive can be structured by question or by attempt:<ul><li>If you group by question, the archive will have a folder for every question. Inside each folder, you will have a folder for every attempt.</li><li>If you group by attempt, the archive will have a folder for every attempt. Inside each folder, you will have a folder for every question.</li></ul>';

diff --git a/report.php b/report.php
@@ -283,8 +283,8 @@ public function get_details_for_attempt(int $attemptid): array {
             $questionfolder = self::clean_filename($questionfolder);
 
             $details[$questionfolder] = [];
-            $details[$questionfolder]['questiontext'] = $quba->get_question_summary($slot);
-            $details[$questionfolder]['responsetext'] = $quba->get_response_summary($slot);
+            $details[$questionfolder]['questiontext'] = $quba->get_question_summary($slot) ?? '';
+            $details[$questionfolder]['responsetext'] = $quba->get_response_summary($slot) ?? '';
 
             $qa = $quba->get_question_attempt($slot);
             $details[$questionfolder]['attachments'] = $qa->get_last_qt_files('attachments', $quba->get_owning_context()->id);
@@ -311,6 +311,9 @@ protected function process_and_download(): void {
         // In the end, we want to know whether the archive is empty or not.
         $emptyarchive = true;
 
+        // Counter in case of errors.
+        $errors = 0;
+
         // Iterate over every attempt and every question.
         foreach ($this->attempts as $attemptid => $attemptpath) {
             $questions = $this->get_details_for_attempt($attemptid);
@@ -323,21 +326,30 @@ protected function process_and_download(): void {
                     $path = $questionpath . '/' . $attemptpath;
                 }
 
-                if ($this->options->questiontext) {
-                    $zipwriter->add_file_from_string($path . '/' . 'questiontext.txt', $questiondetails['questiontext']);
-                    $emptyarchive = false;
-                }
+                try {
+                    if ($this->options->questiontext) {
+                        $zipwriter->add_file_from_string($path . '/' . 'questiontext.txt', $questiondetails['questiontext']);
+                        $emptyarchive = false;
+                    }
 
-                if ($this->options->responsetext) {
-                    $zipwriter->add_file_from_string($path . '/' . 'response.txt', $questiondetails['responsetext']);
-                    $emptyarchive = false;
-                }
+                    if ($this->options->responsetext) {
+                        $zipwriter->add_file_from_string($path . '/' . 'response.txt', $questiondetails['responsetext']);
+                        $emptyarchive = false;
+                    }
 
-                if (!empty($questiondetails['attachments']) && $this->options->attachments) {
-                    $emptyarchive = false;
-                    foreach ($questiondetails['attachments'] as $file) {
-                        $zipwriter->add_file_from_stored_file($path . '/attachments/' . $file->get_filename(), $file);
+                    if (!empty($questiondetails['attachments']) && $this->options->attachments) {
+                        $emptyarchive = false;
+                        foreach ($questiondetails['attachments'] as $file) {
+                            $zipwriter->add_file_from_stored_file($path . '/attachments/' . $file->get_filename(), $file);
+                        }
                     }
+                } catch (Throwable $e) {
+                    $emptyarchive = false;
+                    $errors++;
+                    $message = get_string('errormessage', 'quiz_essaydownload');
+                    $message .= "\n\n" . $e->getMessage();
+                    $message .= "\n\n" . $e->getTraceAsString();
+                    $zipwriter->add_file_from_string(get_string('errorfilename', 'quiz_essaydownload', $errors), $message);
                 }
             }
         }
@@ -348,6 +360,7 @@ protected function process_and_download(): void {
             $this->notification(get_string('nothingtodownload', 'quiz_essaydownload'));
         } else {
             $zipwriter->finish();
+            exit();
         }
     }