Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ async def base_long_running_error_handler(request, handler):
return await handler(request)
except (TaskNotFoundError, TaskNotCompletedError) as exc:
_logger.debug("", exc_info=True)
error_fields = dict(code=exc.code, message=f"{exc}")
error_fields = {"code": exc.code, "message": f"{exc}"}
raise web.HTTPNotFound(
reason=f"{json_dumps(error_fields)}",
text=f"{json_dumps(error_fields)}",
) from exc
except TaskCancelledError as exc:
# NOTE: only use-case would be accessing an already cancelled task
# which should not happen, so we return a conflict
_logger.debug("", exc_info=True)
error_fields = dict(code=exc.code, message=f"{exc}")
raise web.HTTPConflict(reason=f"{json_dumps(error_fields)}") from exc
error_fields = {"code": exc.code, "message": f"{exc}"}
raise web.HTTPConflict(text=f"{json_dumps(error_fields)}") from exc
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,11 @@ async def middleware_handler(request: web.Request, handler: Handler):
log_exception = None
raise resp from exc
except asyncio.CancelledError as exc:
resp = web.HTTPInternalServerError(reason=f"{exc}")
resp = web.HTTPInternalServerError(text=f"{exc}")
log_exception = exc
raise resp from exc
except Exception as exc: # pylint: disable=broad-except
resp = web.HTTPInternalServerError(reason=f"{exc}")
resp = web.HTTPInternalServerError(text=f"{exc}")
resp.__cause__ = exc
log_exception = exc
raise resp from exc
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ async def parse_request_body_as(
try:
body = await request.json()
except json.decoder.JSONDecodeError as err:
raise web.HTTPBadRequest(reason=f"Invalid json in body: {err}") from err
raise web.HTTPBadRequest(text=f"Invalid json in body: {err}") from err

if hasattr(model_schema_cls, "model_validate"):
# NOTE: model_schema can be 'list[T]' or 'dict[T]' which raise TypeError
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,24 +81,22 @@ async def _middleware_handler(request: web.Request, handler: Handler): # noqa:
return await handler(request)

except web.HTTPError as err:
# TODO: differenciate between server/client error
if not err.reason:
err.set_status(err.status_code, reason="Unexpected error")

err.content_type = MIMETYPE_APPLICATION_JSON

if not err.text or not is_enveloped_from_text(err.text):
error = ErrorGet(
error_message = err.text or err.reason or "Unexpected error"
error_model = ErrorGet(
errors=[
ErrorItemType.from_error(err),
],
status=err.status,
logs=[
LogMessageType(message=err.reason, level="ERROR"),
LogMessageType(message=error_message, level="ERROR"),
],
message=err.reason,
message=error_message,
)
err.text = EnvelopeFactory(error=error).as_text()
err.text = EnvelopeFactory(error=error_model).as_text()

raise

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ def create_http_error(
)

return http_error_cls(
# Multiline not allowed in HTTP reason
reason=reason.replace("\n", " ") if reason else None,
reason=safe_status_message(reason),
text=json_dumps(
payload,
),
Expand Down Expand Up @@ -129,3 +128,29 @@ def _pred(obj) -> bool:
assert len(http_statuses) == len(found), "No duplicates" # nosec

return http_statuses


def safe_status_message(message: str | None, max_length: int = 50) -> str | None:
"""
Truncates a status-message (i.e. `reason` in HTTP errors) to a maximum length, replacing newlines with spaces.

If the message is longer than max_length, it will be truncated and "..." will be appended.

This prevents issues such as:
- `aiohttp.http_exceptions.LineTooLong`: 400, message: Got more than 8190 bytes when reading Status line is too long.
- Multiline not allowed in HTTP reason attribute (aiohttp now raises ValueError).

See:
- When to use http status and/or text messages https://github.com/ITISFoundation/osparc-simcore/pull/7760
- [RFC 9112, Section 4.1: HTTP/1.1 Message Syntax and Routing](https://datatracker.ietf.org/doc/html/rfc9112#section-4.1) (status line length limits)
- [RFC 9110, Section 15.5: Reason Phrase](https://datatracker.ietf.org/doc/html/rfc9110#section-15.5) (reason phrase definition)
"""
if not message:
return None

flat_message = message.replace("\n", " ")
if len(flat_message) <= max_length:
return flat_message

# Truncate and add ellipsis
return flat_message[: max_length - 3] + "..."
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ async def raise_error(_request: web.Request):

@staticmethod
async def raise_error_with_reason(_request: web.Request):
raise web.HTTPNotFound(reason="I did not find it")
raise web.HTTPNotFound(reason="A short phrase")

@staticmethod
async def raise_success(_request: web.Request):
Expand Down
55 changes: 55 additions & 0 deletions packages/service-library/tests/aiohttp/test_rest_responses.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,58 @@ def tests_exception_to_response(skip_details: bool, error_code: ErrorCodeStr | N
assert response_json["error"]["message"] == expected_reason
assert response_json["error"]["supportId"] == error_code
assert response_json["error"]["status"] == response.status


@pytest.mark.parametrize(
"input_message, expected_output",
[
(None, None), # None input returns None
("", None), # Empty string returns None
("Simple message", "Simple message"), # Simple message stays the same
(
"Message\nwith\nnewlines",
"Message with newlines",
), # Newlines are replaced with spaces
("A" * 100, "A" * 47 + "..."), # Long message gets truncated with ellipsis
(
"Line1\nLine2\nLine3" + "X" * 100,
"Line1 Line2 Line3" + "X" * 30 + "...",
), # Combined case: newlines and truncation with ellipsis
],
ids=[
"none_input",
"empty_string",
"simple_message",
"newlines_replaced",
"long_message_truncated",
"newlines_and_truncation",
],
)
def test_safe_status_message(input_message: str | None, expected_output: str | None):
from servicelib.aiohttp.rest_responses import safe_status_message

result = safe_status_message(input_message)
assert result == expected_output

# Test with custom max_length
custom_max = 10
result_custom = safe_status_message(input_message, max_length=custom_max)

# Check length constraint is respected
if result_custom is not None:
assert len(result_custom) <= custom_max
# Check that ellipsis is added when truncated
if input_message and len(input_message.replace("\n", " ")) > custom_max:
assert result_custom.endswith("...")

# Verify it can be used in a web response without raising exceptions
try:
# This would fail with long or multiline reasons
if result is not None:
web.Response(reason=result)

# Test with custom length result too
if result_custom is not None:
web.Response(reason=result_custom)
except ValueError:
pytest.fail("safe_status_message result caused an exception in web.Response")
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def _handle_client_exceptions(app: web.Application) -> Iterator[ClientSession]:

except ClientResponseError as err:
if err.status == status.HTTP_404_NOT_FOUND:
raise web.HTTPNotFound(reason=MSG_CATALOG_SERVICE_NOT_FOUND)
raise web.HTTPNotFound(text=MSG_CATALOG_SERVICE_NOT_FOUND) from err
raise web.HTTPServiceUnavailable(
reason=MSG_CATALOG_SERVICE_UNAVAILABLE
) from err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from common_library.error_codes import create_error_code
from common_library.json_serialization import json_dumps
from models_library.rest_error import ErrorGet
from servicelib.aiohttp.rest_responses import safe_status_message
from servicelib.aiohttp.web_exceptions_extension import get_all_aiohttp_http_exceptions
from servicelib.logging_errors import create_troubleshotting_log_kwargs
from servicelib.status_codes_utils import is_5xx_server_error, is_error
Expand Down Expand Up @@ -40,8 +41,7 @@ def create_error_response(error: ErrorGet, status_code: int) -> web.Response:
return web.json_response(
data={"error": error.model_dump(exclude_unset=True, mode="json")},
dumps=json_dumps,
# NOTE: Multiline not allowed in HTTP reason attribute (aiohttp now raises ValueError)
reason=error.message.replace("\n", " ") if error.message else None,
reason=safe_status_message(error.message),
status=status_code,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,4 @@ class SDSException(HTTPBadRequest): # pylint: disable=too-many-ancestors
"""Basic exception for errors raised inside the module"""

def __init__(self, message: str):
# Multiline not allowed in HTTP reason attribute
super().__init__(reason=message.replace("\n", " ") if message else None)
super().__init__(text=message)
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from aiohttp.abc import AbstractStreamWriter
from aiohttp.typedefs import LooseHeaders
from aiohttp.web import BaseRequest, FileResponse
from servicelib.aiohttp.rest_responses import safe_status_message


class CleanupFileResponse(FileResponse): # pylint: disable=too-many-ancestors
Expand All @@ -27,8 +28,7 @@ def __init__(
path=path,
chunk_size=chunk_size,
status=status,
# Multiline not allowed in HTTP reason
reason=reason.replace("\n", " ") if reason else None,
reason=safe_status_message(reason),
headers=headers,
)
self.remove_tmp_dir_cb = remove_tmp_dir_cb
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ def _get_error_context(
error_context=_get_error_context(user),
)
)
raise web.HTTPServiceUnavailable(reason=MSG_CANT_SEND_MAIL) from err
raise web.HTTPServiceUnavailable(text=MSG_CANT_SEND_MAIL) from err

# NOTE: Always same response: guideline #1
return flash_response(MSG_EMAIL_SENT.format(email=request_body.email), "INFO")
Expand All @@ -241,7 +241,7 @@ async def initiate_change_email(request: web.Request):

async with pass_or_acquire_connection(get_asyncpg_engine(request.app)) as conn:
if await UsersRepo.is_email_used(conn, email=request_body.email):
raise web.HTTPUnprocessableEntity(reason="This email cannot be used")
raise web.HTTPUnprocessableEntity(text="This email cannot be used")

# Reset if previously requested
confirmation = await db.get_confirmation({"user": user, "action": CHANGE_EMAIL})
Expand All @@ -268,7 +268,7 @@ async def initiate_change_email(request: web.Request):
except Exception as err: # pylint: disable=broad-except
_logger.exception("Can not send change_email_email")
await db.delete_confirmation(confirmation)
raise web.HTTPServiceUnavailable(reason=MSG_CANT_SEND_MAIL) from err
raise web.HTTPServiceUnavailable(text=MSG_CANT_SEND_MAIL) from err

return flash_response(MSG_CHANGE_EMAIL_REQUESTED)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ async def register(request: web.Request):

await db.delete_confirmation_and_user(user, _confirmation)

raise web.HTTPServiceUnavailable(reason=user_error_msg) from err
raise web.HTTPServiceUnavailable(text=user_error_msg) from err

return flash_response(
"You are registered successfully! To activate your account, please, "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,7 @@ async def _stop_dynamic_service_task(

except (RPCServerError, ServiceWaitingForManualInterventionError) as exc:
# in case there is an error reply as not found
raise web.HTTPNotFound(reason=f"{exc}") from exc
raise web.HTTPNotFound(text=f"{exc}") from exc

except ServiceWasNotFoundError:
# in case the service is not found reply as all OK
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ async def update_project_inputs(request: web.Request) -> web.Response:
for input_update in inputs_updates:
node_id = input_update.key
if node_id not in current_inputs:
raise web.HTTPBadRequest(reason=f"Invalid input key [{node_id}]")
raise web.HTTPBadRequest(text=f"Invalid input key [{node_id}]")

workbench[node_id].outputs = {KeyIDStr("out_1"): input_update.value}
partial_workbench_data[node_id] = workbench[node_id].model_dump(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ async def open_project(request: web.Request) -> web.Response:
client_session_id = await request.json()

except json.JSONDecodeError as exc:
raise web.HTTPBadRequest(reason="Invalid request body") from exc
raise web.HTTPBadRequest(text="Invalid request body") from exc

try:
project_type: ProjectType = await _projects_service.get_project_type(
Expand All @@ -74,7 +74,7 @@ async def open_project(request: web.Request) -> web.Response:
)
if project_type is ProjectType.TEMPLATE and user_role < UserRole.USER:
# only USERS/TESTERS can do that
raise web.HTTPForbidden(reason="Wrong user role to open/edit a template")
raise web.HTTPForbidden(text="Wrong user role to open/edit a template")

project = await _projects_service.get_project_for_user(
request.app,
Expand All @@ -101,7 +101,7 @@ async def open_project(request: web.Request) -> web.Response:
app=request.app,
max_number_of_studies_per_user=product.max_open_studies_per_user,
):
raise HTTPLockedError(reason="Project is locked, try later")
raise HTTPLockedError(text="Project is locked, try later")

# the project can be opened, let's update its product links
await _projects_service.update_project_linked_product(
Expand Down Expand Up @@ -175,7 +175,7 @@ async def close_project(request: web.Request) -> web.Response:
client_session_id = await request.json()

except json.JSONDecodeError as exc:
raise web.HTTPBadRequest(reason="Invalid request body") from exc
raise web.HTTPBadRequest(text="Invalid request body") from exc

# ensure the project exists
await _projects_service.get_project_for_user(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ async def add_project_tag(request: web.Request):
request.match_info["project_uuid"],
)
except KeyError as err:
raise web.HTTPBadRequest(reason=f"Invalid request parameter {err}") from err
raise web.HTTPBadRequest(text=f"Invalid request parameter {err}") from err

project = await tags_api.add_tag(
request.app,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -469,10 +469,10 @@ async def create_project( # pylint: disable=too-many-arguments,too-many-branche
)

except JsonSchemaValidationError as exc:
raise web.HTTPBadRequest(reason="Invalid project data") from exc
raise web.HTTPBadRequest(text="Invalid project data") from exc

except ProjectNotFoundError as exc:
raise web.HTTPNotFound(reason=f"Project {exc.project_uuid} not found") from exc
raise web.HTTPNotFound(text=f"Project {exc.project_uuid} not found") from exc

except (ProjectInvalidRightsError, WorkspaceAccessForbiddenError) as exc:
raise web.HTTPForbidden from exc
Expand All @@ -485,7 +485,7 @@ async def create_project( # pylint: disable=too-many-arguments,too-many-branche
user_id=user_id,
simcore_user_agent=simcore_user_agent,
)
raise web.HTTPNotFound(reason=f"{exc}") from exc
raise web.HTTPNotFound(text=f"{exc}") from exc

except asyncio.CancelledError:
log.warning(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ async def wrapper(request: web.Request) -> web.StreamResponse:
return await handler(request)

except (ValueError, PricingUnitDuplicationError) as exc:
raise web.HTTPBadRequest(reason=f"{exc}") from exc
raise web.HTTPBadRequest(text=f"{exc}") from exc

except RPCServerError as exc:
# NOTE: This will be improved; we will add a mapping between
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ async def wrapper(request: web.Request) -> web.StreamResponse:
return await handler(request)

except WalletAccessForbiddenError as exc:
raise web.HTTPForbidden(reason=f"{exc}") from exc
raise web.HTTPForbidden(text=f"{exc}") from exc

return wrapper

Expand Down
Loading
Loading