Merge pull request #146 from catalyst/issue145

issue #145: catch errors when indexing documents

Author: marcusboon

classes/search/activity.php

@@ -53,28 +53,40 @@ public function get_document($record, $options = []) {
             return false;
         }
 
-        $cms = new cms($cm->instance);
-        $renderer = new renderer($cms);
-        $value = $renderer->get_html();
-        $title = $cms->get('name');
-        $valueformat = FORMAT_HTML;
-
-        // Prepare associative array with data from DB.
-        $doc = \core_search\document_factory::instance($record->id, $this->componentname, $this->areaname);
-        $doc->set('title', content_to_text($title, false));
-        $doc->set('content', content_to_text($value, $valueformat));
-        $doc->set('contextid', $context->id);
-        $doc->set('courseid', $record->course);
-        $doc->set('owneruserid', \core_search\manager::NO_OWNER_ID);
-        $doc->set('modified', $record->timemodified);
-
-        // Check if this document should be considered new.
-        if (isset($options['lastindexedtime']) && ($options['lastindexedtime'] < $record->timecreated)) {
-            // If the document was created after the last index time, it must be new.
-            $doc->set_is_new(true);
-        }
+        // As mod_cms is a complicated activity that may use data from different modules dynamically,
+        // there are many moving parts that potentially can break and throw errors/exception while doing global search indexing.
+        // We would like to prevent that if possible.
+        try {
+            $cms = new cms($cm->instance);
+            $renderer = new renderer($cms);
+            $value = $renderer->get_html();
+            $title = $cms->get('name');
+            $valueformat = FORMAT_HTML;
+
+            // Prepare associative array with data from DB.
+            $doc = \core_search\document_factory::instance($record->id, $this->componentname, $this->areaname);
+            $doc->set('title', content_to_text($title, false));
+            $doc->set('content', content_to_text($value, $valueformat));
+            $doc->set('contextid', $context->id);
+            $doc->set('courseid', $record->course);
+            $doc->set('owneruserid', \core_search\manager::NO_OWNER_ID);
+            $doc->set('modified', $record->timemodified);
+
+            // Check if this document should be considered new.
+            if (isset($options['lastindexedtime']) && ($options['lastindexedtime'] < $record->timecreated)) {
+                // If the document was created after the last index time, it must be new.
+                $doc->set_is_new(true);
+            }
 
-        return $doc;
+            return $doc;
+
+        } catch (\Throwable $ex) {
+            debugging('Error getting mod_cms document for global search.'
+                . ' cmid: ' . $cm->id . ' '  . ' courseid: ' . $cm->course . ' '
+                . $ex->getMessage(), DEBUG_DEVELOPER);
+
+            return  false;
+        }
     }
 
     /**

tests/search/search_test.php

@@ -384,4 +384,38 @@ public function test_check_access(): void {
         $this->setUser($user2);
         $this->assertEquals(\core_search\manager::ACCESS_DENIED, $searcharea->check_access($cms->id));
     }
+
+    /**
+     * Test getting document catches errors.
+     *
+     * @return void
+     * @covers ::get_document
+     */
+    public function test_getting_document_catch_errors(): void {
+        global $DB;
+
+        $searcharea = \core_search\manager::get_search_area($this->cmsareaid);
+        $course = self::getDataGenerator()->create_course();
+
+        $generator = self::getDataGenerator()->get_plugin_generator('mod_cms');
+        $record = new \stdClass();
+        $record->course = $course->id;
+        $record->customfield_overview = 'Test overview text 1';
+        $record->typeid = $this->cmstype->get('id');
+        $cms = $generator->create_instance_with_data($record);
+
+        // Let's break cms record so getting a document would throw an exception.
+        $cms->typeid = 8888;
+        $DB->update_record('cms', $cms);
+
+        // Test that an exception is not thrown, but debugging is triggered instead.
+        $this->assertEmpty($searcharea->get_document($cms));
+        $debuggingmessages = $this->getDebuggingMessages();
+        $this->assertDebuggingCalled();
+
+        $this->assertStringContainsString(
+            'Error getting mod_cms document for global search. cmid: ' . $cms->cmid . '  courseid: '. $course->id,
+            reset($debuggingmessages)->message
+        );
+    }
 }