Skip to content

Commit

Permalink
fix(plugins): raise error when http download order fails (#1338)
Browse files Browse the repository at this point in the history
  • Loading branch information
amarandon authored Oct 11, 2024
1 parent 7252ad5 commit 86baf25
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 14 deletions.
16 changes: 5 additions & 11 deletions eodag/plugins/download/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
DownloadError,
MisconfiguredError,
NotAvailableError,
RequestError,
TimeOutError,
)

Expand Down Expand Up @@ -193,18 +194,11 @@ def order_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
Expand Down
5 changes: 4 additions & 1 deletion eodag/rest/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
36 changes: 35 additions & 1 deletion tests/units/test_download_plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@
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,
Expand Down Expand Up @@ -1157,6 +1159,38 @@ def test_plugins_download_http_order_get(self, mock_request):
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"""
Expand Down
31 changes: 30 additions & 1 deletion tests/units/test_http_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ def _request_valid_raw(
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
Expand Down Expand Up @@ -354,7 +355,7 @@ def _request_valid_raw(
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

Expand Down Expand Up @@ -1150,6 +1151,34 @@ def test_download_item_from_collection_no_stream(
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)
Expand Down

0 comments on commit 86baf25

Please sign in to comment.