Don't eat GuzzleExceptions when requests fail

[cameron1729]

NB: As we are testing situations in which Elasticsearch is unreachable, it is not necessary to have an Elasticsearch service running locally to complete testing.

Prerequisites

1. Install local_aws (this plugin depends on it)
2. Set up global search in Moodle:
   1. Browse to "Site administration" > "Advanced features"
   2. Turn on "Global search"
   3. Press "Save changes"
3. Enable the Elastic search plugin
   1. Browse to "Site administration" > "Plugins" > "Search" > "Manage global search"
   2. Under "Select search engine" select "Elastic"
   3. Press "Save changes"
4. Configure the Elastic search plugin
   1. Browse to "Site administration" > "Plugins" > "Search" > "Elastic" > "Plugin settings"
   2. Set "Hostname" to "localhost"
   3. Verify an error like "The configured elastic server at localhost:9200 returned the status code 503" is displayed (this is fine as we will hack the is_server_ready function to make the indexing scheduled task run)

Testing

1. Hack is_server_ready in classes/engine.php so that it immediately returns true:

   public function is_server_ready($stack = false) {
   return true;
   ...

2. Run the scheduled task: php admin/cli/scheduled_task.php --execute="\\core\\task\\search_index_task"
3. Verify an error is present in the output like

   Scheduled task failed: Global search indexing (core\task\search_index_task),cURL error 7: Failed to connect to localhost port 9200 after 0 ms: Connection refused (see https://curl.haxx.se/libcurl/c/libcurl-errors.html)

4. Undo the hack from step 1.
5. Re-run the task
6. Verify the output looks like:

   Scheduled task failed: Global search indexing (core\task\search_index_task),The search engine is not available. Please contact your administrator.

Bonus

Set up an Elasticsearch server and verify that the plugin works correctly when properly configured

!fixes #117

[cameron1729]

Following on from my comment here - I think the reason this guzzle_exception thing was added is to handle the case where the Elasticsearch service URL is invalid on the configuration page. Without additional handling, the stack trace and everything will appear there, when really we just want a status code.

Instead of handling the situation where the server can't be accessed directly http_action, I've updated get_server_status_code to use a try/catch. http_action is supposed to return a response, so it doesn't feel right that it should try to handle the case where the server is unreachable (hence there cannot be a response) - it feels appropriate to throw an exception and leave it to calling code to either surface the exception or gracefully handle it.

[dmitriim]

Thanks for the fix @cameron1729 Any chance you can add automated tests to cover this change?

[dmitriim]

Also pinging @marxjohnson here in case he has a spare minute to have a look.

[cameron1729]

@dmitriim looks like the plugin has some decent coverage already so hopefully won't be too difficult to update for this change.

[dmitriim]

well, no tests exploded after the code change.

[cameron1729]

@dmitriim just pushed a commit adding some new tests. I wanted to see if it would be possible to modify existing ones - but they are all using Guzzle mock stuff, and in those cases exceptions don't trigger the same way they do when trying to make a real curl request (even though some people claim it should be possible I couldn't get it to work).

I just added two new tests which try to really call unreachable URIs.

[marxjohnson]

@cameron1729 Your explanation and solution both look good to me!