Repository: CS-SI/eodag (python)

Issue:
Raise an error when a request to a backend provider fails
When a request to a provider such as cop_ads fails because of a malformed request (e.g. providing a date range with an end date before the start date), eodag logs the error, which may be unnotified by the user. Instead it should raise an error.

Pull request:
fix(plugins): raise error when http download order fails
Fix #1337 

When a `HttpDownload.order_download()` fails, the error is logged but this can easily go unnoticed by the user if logs aren't enabled. The purpose of this pull request is to raise an exception instead. 

However, this exception has a `status_code` attribute set to None, which makes the rest server crash when this status code is compared with integer values:
```
  File "/home/al/venvs/dedl_lab/lib/python3.12/site-packages/starlette/responses.py", line 66, in init_headers
    and not (self.status_code < 200 or self.status_code in (204, 304))
             ^^^^^^^^^^^^^^^^^^^^^^
TypeError: '<' not supported between instances of 'NoneType' and 'int'
```

This pull request provides a fix for that by ensuring that the status code defaults to 500.

Patch:
diff --git a/docs/plugins_reference/auth.rst b/docs/plugins_reference/auth.rst
index 8f4b0206..fbad669f 100644
--- a/docs/plugins_reference/auth.rst
+++ b/docs/plugins_reference/auth.rst
@@ -9,6 +9,8 @@ Multiple authentication plugins can be defined per provider under ``auth``, ``se
 using *matching settings* :attr:`~eodag.config.PluginConfig.matching_url` and
 :attr:`~eodag.config.PluginConfig.matching_conf` as configuration parameters.
 Credentials are automatically shared between plugins having the same *matching settings*.
+Authentication plugins without *matching settings* configured will not be shared and will automatically match their
+provider.
 
 Authentication plugins must inherit the following class and implement :meth:`authenticate`:
 
diff --git a/eodag/plugins/download/http.py b/eodag/plugins/download/http.py
index 83926fba..2302c53b 100644
--- a/eodag/plugins/download/http.py
+++ b/eodag/plugins/download/http.py
@@ -79,6 +79,7 @@ from eodag.utils.exceptions import (
     DownloadError,
     MisconfiguredError,
     NotAvailableError,
+    RequestError,
     TimeOutError,
 )
 
@@ -193,18 +194,11 @@ class HTTPDownload(Download):
                     logger.debug(ordered_message)
                     product.properties["storageStatus"] = STAGING_STATUS
                 except RequestException as e:
-                    if hasattr(e, "response") and (
-                        content := getattr(e.response, "content", None)
-                    ):
-                        error_message = f"{content.decode('utf-8')} - {e}"
-                    else:
-                        error_message = str(e)
-                    logger.warning(
-                        "%s could not be ordered, request returned %s",
-                        product.properties["title"],
-                        error_message,
-                    )
                     self._check_auth_exception(e)
+                    title = product.properties["title"]
+                    message = f"{title} could not be ordered"
+                    raise RequestError.from_error(e, message) from e
+
                 return self.order_response_process(response, product)
         except requests.exceptions.Timeout as exc:
             raise TimeOutError(exc, timeout=timeout) from exc
diff --git a/eodag/plugins/manager.py b/eodag/plugins/manager.py
index 32335d87..671868c7 100644
--- a/eodag/plugins/manager.py
+++ b/eodag/plugins/manager.py
@@ -335,12 +335,8 @@ class PluginManager:
                 ):
                     # url matches
                     return True
-            if not plugin_matching_conf and not plugin_matching_url:
-                # plugin without match criteria
-                return True
-            else:
-                # no match
-                return False
+            # no match
+            return False
 
         # providers configs with given provider at first
         sorted_providers_config = deepcopy(self.providers_config)
@@ -354,7 +350,20 @@ class PluginManager:
                 auth_conf = getattr(provider_conf, key, None)
                 if auth_conf is None:
                     continue
-                if _is_auth_plugin_matching(auth_conf, matching_url, matching_conf):
+                # plugin without configured match criteria: only works for given provider
+                unconfigured_match = (
+                    True
+                    if (
+                        not getattr(auth_conf, "matching_conf", {})
+                        and not getattr(auth_conf, "matching_url", None)
+                        and provider == plugin_provider
+                    )
+                    else False
+                )
+
+                if unconfigured_match or _is_auth_plugin_matching(
+                    auth_conf, matching_url, matching_conf
+                ):
                     auth_conf.priority = provider_conf.priority
                     plugin = cast(
                         Authentication,
diff --git a/eodag/resources/product_types.yml b/eodag/resources/product_types.yml
index b54de976..08bb7428 100644
--- a/eodag/resources/product_types.yml
+++ b/eodag/resources/product_types.yml
@@ -2866,7 +2866,7 @@ ERA5_LAND:
   sensorType: ATMOSPHERIC
   license: proprietary
   title: ERA5-Land hourly data from 1950 to present
-  missionStartDate: "1950-01-01T00:00:00Z"
+  missionStartDate: "1950-01-01T01:00:00Z"
 
 ERA5_LAND_MONTHLY:
   abstract: |
diff --git a/eodag/resources/providers.yml b/eodag/resources/providers.yml
index 097ff545..849094cc 100644
--- a/eodag/resources/providers.yml
+++ b/eodag/resources/providers.yml
@@ -1567,7 +1567,6 @@
       Content-Type: application/json
   auth: !plugin
     type: GenericAuth
-    matching_url: https://catalogue.onda-dias.eu
 
 ---
 !provider # MARK: astraea_eod
@@ -4189,36 +4188,12 @@
       variable:
         - '{{"variable": "{variable}"}}'
         - '$.null'
-      leadtime_hour:
-        - '{{"leadtime_hour": {leadtime_hour}}}'
-        - '$.null'
-      leadtime_month:
-        - '{{"leadtime_month": {leadtime_month}}}'
-        - '$.null'
-      origin:
-        - '{{"origin": "{origin}"}}'
-        - '$.null'
       system:
         - '{{"system": "{system}"}}'
         - '$.null'
-      format:
-        - '{{"format": "{format}"}}'
-        - '$.null'
-      pressure_level:
-        - '{{"pressure_level": {pressure_level}}}'
-        - '$.null'
-      model_level:
-        - '{{"model_level": {model_level}}}'
-        - '$.null'
-      sensor_and_algorithm:
-        - '{{"sensor_and_algorithm": "{sensor_and_algorithm}"}}'
-        - '$.null'
       version:
         - '{{"version": {version}}}'
         - '$.null'
-      time:
-        - '{{"time": {time}}}'
-        - '$.null'
       region:
         - '{{"region": {region}}}'
         - '$.null'
@@ -4228,72 +4203,18 @@
       source:
         - '{{"source": {source}}}'
         - '$.null'
-      quantity:
-        - '{{"quantity": "{quantity}"}}'
-        - '$.null'
-      input_observations:
-        - '{{"input_observations": "{input_observations}"}}'
-        - '$.null'
-      aggregation:
-        - '{{"aggregation": "{aggregation}"}}'
-        - '$.null'
       model:
         - '{{"model": {model}}}'
         - '$.null'
       level:
         - '{{"level": {level}}}'
         - '$.null'
-      forcing_type:
-        - '{{"forcing_type": "{forcing_type}"}}'
-        - '$.null'
-      sky_type:
-        - '{{"sky_type": {sky_type}}}'
-        - '$.null'
-      band:
-        - '{{"band": {band}}}'
-        - '$.null'
-      aerosol_type:
-        - '{{"aerosol_type": {aerosol_type}}}'
-        - '$.null'
       step:
         - '{{"step": {step}}}'
         - '$.null'
-      longitude:
-        - '{{"longitude": "{longitude}"}}'
-        - '$.null'
-      latitude:
-        - '{{"latitude": "{latitude}"}}'
-        - '$.null'
-      altitude:
-        - '{{"altitude": "{altitude}"}}'
-        - '$.null'
-      time_reference:
-        - '{{"time_reference": "{time_reference}"}}'
-        - '$.null'
-      grid:
-        - '{{"grid": "{grid}"}}'
-        - '$.null'
-      soil_level:
-        - '{{"soil_level": {soil_level}}}'
-        - '$.null'
-      year:
-        - '{{"year": {year}}}'
-        - '$.null'
-      month:
-        - '{{"month": {month}}}'
-        - '$.null'
-      day:
-        - '{{"day": {day}}}'
-        - '$.null'
       satellite:
         - '{{"satellite": {satellite}}}'
         - '$.null'
-      cdr_type:
-        - '{{"cdr_type": "{cdr_type}"}}'
-        - '$.null'
-      statistic:
-        - '{{"statistic": {statistic}}}'
-        - '$.null'
       sensor:
         - '{{"sensor": "{sensor}"}}'
         - '$.null'
@@ -4785,12 +4706,6 @@
       providerProductType:
         - '{{"productType": "{providerProductType}"}}'
         - '$.null'
-      timeliness:
-        - '{{"timeliness": "{timeliness}"}}'
-        - '$.null'
-      orbitDirection:
-        - '{{"orbitDirection": "{orbitDirection}"}}'
-        - '$.null'
       variable:
         - '{{"variable": {variable}}}'
         - '$.null'
@@ -5397,6 +5312,9 @@
       format: grib
       metadata_mapping:
         id: '$.id'
+        month:
+          - '{{"month": "{month}"}}'
+          - '$.null'
         startTimeFromAscendingNode:
           - |
             {{
diff --git a/eodag/rest/errors.py b/eodag/rest/errors.py
index 6ff067f4..574a3e37 100644
--- a/eodag/rest/errors.py
+++ b/eodag/rest/errors.py
@@ -126,7 +126,10 @@ async def eodag_errors_handler(request: Request, exc: Exception) -> ORJSONRespon
     if not isinstance(exc, EodagError):
         return starlette_exception_handler(request, exc)
 
-    code = EODAG_DEFAULT_STATUS_CODES.get(type(exc), getattr(exc, "status_code", 500))
+    exception_status_code = getattr(exc, "status_code", None)
+    default_status_code = exception_status_code or 500
+    code = EODAG_DEFAULT_STATUS_CODES.get(type(exc), default_status_code)
+
     detail = f"{type(exc).__name__}: {str(exc)}"
 
     if type(exc) in (MisconfiguredError, AuthenticationError, TimeOutError):


Test patch:
diff --git a/tests/integration/test_core_search.py b/tests/integration/test_core_search.py
index e5ae64b2..46b83844 100644
--- a/tests/integration/test_core_search.py
+++ b/tests/integration/test_core_search.py
@@ -495,6 +495,23 @@ class TestCoreSearch(unittest.TestCase):
             },
         )
 
+        # auth plugin without match configuration
+        self.dag.add_provider(
+            "provider_without_match_configured",
+            "https://foo.bar/baz/search",
+            search={"need_auth": True},
+            auth={
+                "type": "GenericAuth",
+                "credentials": {
+                    "username": "some-username",
+                    "password": "some-password",
+                },
+            },
+        )
+        self.dag.search(provider="provider_without_match_configured")
+        self.assertEqual(mock_query.call_args[0][1].auth.username, "some-username")
+        mock_query.reset_mock()
+
         # search endpoint matching
         self.dag.add_provider(
             "provider_matching_search_api",
diff --git a/tests/units/test_download_plugins.py b/tests/units/test_download_plugins.py
index cda5f43c..d51f3454 100644
--- a/tests/units/test_download_plugins.py
+++ b/tests/units/test_download_plugins.py
@@ -28,12 +28,14 @@ from tempfile import NamedTemporaryFile, TemporaryDirectory, gettempdir
 from typing import Any, Callable, Dict
 from unittest import mock
 
+import pytest
 import responses
 import yaml
+from requests import RequestException
 
 from eodag.api.product.metadata_mapping import DEFAULT_METADATA_MAPPING
 from eodag.utils import ProgressCallback
-from eodag.utils.exceptions import DownloadError, NoMatchingProductType
+from eodag.utils.exceptions import DownloadError, NoMatchingProductType, RequestError
 from tests import TEST_RESOURCES_PATH
 from tests.context import (
     DEFAULT_STREAM_REQUESTS_TIMEOUT,
@@ -1157,6 +1159,38 @@ class TestDownloadPluginHttp(BaseDownloadPluginTest):
             else:
                 del plugin.config.timeout
 
+    @mock.patch("eodag.plugins.download.http.requests.request", autospec=True)
+    def test_plugins_download_http_order_get_raises_if_request_failed(
+        self, mock_request
+    ):
+        """HTTPDownload.order_download() must raise an error if request to backend
+        provider failed"""
+
+        # Configure mock to raise an error
+        mock_response = mock_request.return_value.__enter__.return_value
+        mock_response.raise_for_status.side_effect = RequestException("Some error msg")
+
+        plugin = self.get_download_plugin(self.product)
+        self.product.properties["downloadLink"] = "https://peps.cnes.fr/dummy"
+        self.product.properties["orderLink"] = "http://somewhere/order"
+
+        auth_plugin = self.get_auth_plugin(plugin, self.product)
+        auth_plugin.config.credentials = {"username": "foo", "password": "bar"}
+        auth = auth_plugin.authenticate()
+
+        # Verify that a RequestError is raised
+        with pytest.raises(RequestError):
+            plugin.order_download(self.product, auth=auth)
+
+        mock_request.assert_called_once_with(
+            method="GET",
+            url=self.product.properties["orderLink"],
+            auth=auth,
+            headers=USER_AGENT,
+            timeout=5,
+            verify=True,
+        )
+
     @mock.patch("eodag.plugins.download.http.requests.request", autospec=True)
     def test_plugins_download_http_order_post(self, mock_request):
         """HTTPDownload.order_download() must request using orderLink and POST protocol"""
diff --git a/tests/units/test_http_server.py b/tests/units/test_http_server.py
index 0e16f846..863578eb 100644
--- a/tests/units/test_http_server.py
+++ b/tests/units/test_http_server.py
@@ -323,6 +323,7 @@ class RequestTestCase(unittest.TestCase):
         post_data: Optional[Any] = None,
         search_call_count: Optional[int] = None,
         search_result: SearchResult = None,
+        expected_status_code: int = 200,
     ) -> httpx.Response:
         if search_result:
             mock_search.return_value = search_result
@@ -354,7 +355,7 @@ class RequestTestCase(unittest.TestCase):
         elif expected_search_kwargs is not None:
             mock_search.assert_called_once_with(**expected_search_kwargs)
 
-        self.assertEqual(200, response.status_code, response.text)
+        self.assertEqual(expected_status_code, response.status_code, response.text)
 
         return response
 
@@ -1150,6 +1151,34 @@ class RequestTestCase(unittest.TestCase):
             expected_file
         ), f"File {expected_file} should have been deleted"
 
+    @mock.patch(
+        "eodag.plugins.authentication.base.Authentication.authenticate",
+        autospec=True,
+    )
+    @mock.patch(
+        "eodag.plugins.download.base.Download._stream_download_dict",
+        autospec=True,
+    )
+    def test_error_handler_supports_request_exception_with_status_code_none(
+        self, mock_stream_download: Mock, mock_auth: Mock
+    ):
+        """
+        A RequestError with a status code set to None (the default value) should not
+        crash the server. This test ensures that it doesn't crash the server and that a
+        500 status code is returned to the user.
+        """
+        # Make _stream_download_dict reaise an exception object with a status_code
+        # attribute set to None
+        exception = RequestError()
+        exception.status_code = None
+        mock_stream_download.side_effect = exception
+
+        response = self._request_valid_raw(
+            f"collections/{self.tested_product_type}/items/foo/download?provider=peps",
+            expected_status_code=500,
+        )
+        self.assertIn("RequestError", response.text)
+
     def test_root(self):
         """Request to / should return a valid response"""
         resp_json = self._request_valid("", check_links=False)