Skip to content

Commit 5cc2074

Browse files
Merge pull request #140 from tsmilan/issue-139
[#139] Add configurable request timeout
2 parents 3ab7176 + 5b03172 commit 5cc2074

File tree

7 files changed

+69
-3
lines changed

7 files changed

+69
-3
lines changed

classes/esrequest.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ public function __construct($handler = false) {
5757
$this->signing = (isset($this->config->signing) ? (bool)$this->config->signing : false);
5858

5959
$config = [
60-
'connect_timeout' => intval($this->config->connecttimeout)
60+
'timeout' => isset($this->config->timeout) ? intval($this->config->timeout) : 0,
61+
'connect_timeout' => intval($this->config->connecttimeout),
6162
];
6263

6364
// Allow the caller to instantiate the Guzzle client with a custom handler.

classes/task/retry_errors_task.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ private function retry_single($errorid): array {
100100
$successcount++;
101101
} else {
102102
$errorcount++;
103-
$error->mark_failed();
104103
mtrace("Failed to retry error ID {$errorid}: {$result['message']}");
105104
}
106105

lang/en/search_elastic.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,8 @@
185185
$string['tikasendsize_help'] = 'Sending large files to Tika can cause out of memory issues. Therefore we limit it to a size in bytes.';
186186
$string['timecreated'] = 'Created';
187187
$string['timemodified'] = 'Timemodified';
188+
$string['timeout'] = 'Request timeout';
189+
$string['timeout_desc'] = 'Maximum time in seconds to wait for HTTP requests to complete. Defaults to 0 to wait indefinitely (the default behavior).';
188190
$string['type_indexing'] = 'Indexing';
189191
$string['type_tika'] = 'Tika';
190192
$string['usesimplequery'] = 'Use simple query';

settings.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@
5858
$settings->add(new admin_setting_configtext('search_elastic/connecttimeout', get_string('connecttimeout', 'search_elastic'),
5959
get_string('connecttimeout_help', 'search_elastic'), 5, PARAM_INT));
6060

61+
$settings->add(new admin_setting_configtext('search_elastic/timeout', get_string('timeout', 'search_elastic'),
62+
get_string('timeout_desc', 'search_elastic'), 0, PARAM_INT));
63+
6164
$settings->add(new admin_setting_heading('signingsettings', get_string('signingsettings', 'search_elastic'), ''));
6265
$settings->add(new admin_setting_configcheckbox('search_elastic/signing', get_string('signing', 'search_elastic'),
6366
get_string ('signing_help', 'search_elastic'), 0));

tests/esrequest_test.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,4 +429,28 @@ public function test_proxy_get() {
429429
$this->assertEquals('bar=blerg', $request->getUri()->getQuery());
430430
$this->assertEquals($expected, $lastrequestoptions['proxy']);
431431
}
432+
433+
/**
434+
* Test constructor applies configured timeout.
435+
*/
436+
public function test_constructor_configured_timeout(): void {
437+
$this->resetAfterTest();
438+
439+
// Mock config with timeout setting.
440+
set_config('timeout', 60, 'search_elastic');
441+
set_config('connecttimeout', 5, 'search_elastic');
442+
443+
$esrequest = new esrequest();
444+
445+
// Use reflection to access private client.
446+
$reflection = new \ReflectionClass($esrequest);
447+
$clientproperty = $reflection->getProperty('client');
448+
$clientproperty->setAccessible(true);
449+
$client = $clientproperty->getValue($esrequest);
450+
451+
// Assert configured timeout is applied.
452+
$clientconfig = $client->getConfig();
453+
$this->assertEquals(60, $clientconfig['timeout']);
454+
$this->assertEquals(5, $clientconfig['connect_timeout']);
455+
}
432456
}

tests/local/service/error_service_test.php

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,43 @@ public function test_get_error_count_different_statuses($status): void {
542542
$this->assertEquals(0, $othercount);
543543
}
544544

545+
/**
546+
* Test that extract_file_contents method handles timeout correctly.
547+
*/
548+
public function test_extract_file_contents_handles_timeout(): void {
549+
$this->resetAfterTest();
550+
551+
// Set up Tika configuration.
552+
set_config('tikahostname', 'http://127.0.0.1', 'search_elastic');
553+
set_config('tikaport', 9998, 'search_elastic');
554+
set_config('tikasendsize', 512000000, 'search_elastic');
555+
set_config('timeout', 5, 'search_elastic');
556+
set_config('connecttimeout', 2, 'search_elastic');
557+
558+
// Create a test file.
559+
$file = $this->create_test_file();
560+
561+
// Test with reflection since extract_file_contents is private.
562+
$reflection = new \ReflectionClass('search_elastic\local\service\error_service');
563+
$method = $reflection->getMethod('extract_file_contents');
564+
$method->setAccessible(true);
565+
566+
// Test that request timeout is handled properly and not throw an exception.
567+
$result = false;
568+
$exceptionthrown = false;
569+
570+
try {
571+
$result = $method->invoke(null, $file);
572+
$this->assertdebuggingcalledcount(1);
573+
} catch (\Exception $e) {
574+
$exceptionthrown = true;
575+
}
576+
577+
// Verify that timeout should be handled gracefully, and not crash the indexing task.
578+
$this->assertFalse($exceptionthrown);
579+
$this->assertFalse($result);
580+
}
581+
545582
public function tearDown(): void {
546583
parent::tearDown();
547584
if ($this->generator) {

version.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424

2525
defined('MOODLE_INTERNAL') || die();
2626

27-
$plugin->version = 2025062708;
27+
$plugin->version = 2025080100;
2828
$plugin->release = '4.2.6 (Build: 20250514)'; // Build same as version.
2929
$plugin->requires = 2023042405;
3030
$plugin->component = 'search_elastic';

0 commit comments

Comments
 (0)