🐛 web-api: Fixes handling of unexpected errors #7939

pcrespov · 2025-06-20T08:54:05Z

What do these changes do?

Improves robustness by correcting how unexpected exceptions are handled.
Now all 5XX are logged with supportID (i.e. OEC!)

⚠️ Important Note

Avoid including the string representation of raw exceptions in HTTP responses:

try:
    # ...
except Exception as err:
    raise web.HTTPInternalServerError(text="Some controlled human readable message")  # ✅ Proper handling
    # raise web.HTTPInternalServerError(text=f"{err}")  # ❌ Don't expose exception details

Related issue/s

Part of 🐛 Welcome credits created via an invitation link from the PO Center do not work #7932

How to test

cd packages/servicelib
make "install-dev[aiohttp]"
pytest -vv tests/aiohttp/test_rest_responses.py

Dev-ops

None

codecov · 2025-06-20T08:55:46Z

Codecov Report

Attention: Patch coverage is 76.59574% with 11 lines in your changes missing coverage. Please review.

Project coverage is 87.90%. Comparing base (c1d1065) to head (68bb19c).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7939      +/-   ##
==========================================
  Coverage   87.90%   87.90%              
==========================================
  Files        1846     1428     -418     
  Lines       71225    59370   -11855     
  Branches     1220      621     -599     
==========================================
- Hits        62609    52190   -10419     
+ Misses       8264     6969    -1295     
+ Partials      352      211     -141

Flag	Coverage Δ
integrationtests	`64.23% <50.00%> (+11.72%)`	⬆️
unittests	`86.22% <76.59%> (-0.41%)`	⬇️

Components	Coverage Δ
api	`∅ <ø> (∅)`
pkg_aws_library	`∅ <ø> (∅)`
pkg_dask_task_models_library	`∅ <ø> (∅)`
pkg_models_library	`∅ <ø> (∅)`
pkg_notifications_library	`∅ <ø> (∅)`
pkg_postgres_database	`∅ <ø> (∅)`
pkg_service_integration	`∅ <ø> (∅)`
pkg_service_library	`72.52% <75.67%> (-0.15%)`	⬇️
pkg_settings_library	`∅ <ø> (∅)`
pkg_simcore_sdk	`85.10% <ø> (+0.05%)`	⬆️
agent	`96.29% <ø> (ø)`
api_server	`92.31% <ø> (ø)`
autoscaling	`96.03% <ø> (ø)`
catalog	`92.29% <ø> (ø)`
clusters_keeper	`99.13% <ø> (ø)`
dask_sidecar	`91.79% <ø> (ø)`
datcore_adapter	`97.94% <ø> (ø)`
director	`76.73% <ø> (ø)`
director_v2	`91.00% <ø> (-0.12%)`	⬇️
dynamic_scheduler	`96.69% <ø> (ø)`
dynamic_sidecar	`90.09% <ø> (ø)`
efs_guardian	`89.65% <ø> (ø)`
invitations	`93.00% <ø> (ø)`
payments	`92.57% <ø> (ø)`
resource_usage_tracker	`89.00% <ø> (-0.11%)`	⬇️
storage	`87.53% <ø> (-0.32%)`	⬇️
webclient	`∅ <ø> (∅)`
webserver	`87.64% <80.00%> (+0.46%)`	⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1d1065...68bb19c. Read the comment docs.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull Request Overview

This PR standardizes error handling by removing the skip_internal_error_details toggle, ensuring internal exceptions never leak raw details, and cleans up related middleware code.

Dropped the skip_internal_error_details parameter from create_http_error and simplified its logic.
Updated middlewares to stop branching on environment flags and renamed variables for consistency.
Adjusted tests to remove obsolete skip_details parameter.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
packages/service-library/tests/aiohttp/test_rest_responses.py	Removed the obsolete `skip_details` parameter and related invocations in `tests_exception_to_response`.
packages/service-library/src/servicelib/aiohttp/rest_responses.py	Removed the `skip_internal_error_details` parameter and simplified the internal-error branch.
packages/service-library/src/servicelib/aiohttp/rest_middlewares.py	Removed environment-based detail toggles, introduced a single `error_message` variable, and renamed `resp` to `response`.
packages/service-library/src/servicelib/aiohttp/monitoring.py	Renamed `resp` to `response`, removed default reason text, and dropped special cases for cancelled/HTTPServerError.

Comments suppressed due to low confidence (1)

packages/service-library/src/servicelib/aiohttp/rest_responses.py:88

The docstring references skipping internal error details, but the skip_internal_error_details parameter was removed; update the documentation to reflect the current behavior.

    exceptions to the client in production

packages/service-library/src/servicelib/aiohttp/rest_middlewares.py

matusdrobuliak66

👍

sanderegg

thanks!

bisgaard-itis

Thanks for the quick fix

packages/service-library/src/servicelib/aiohttp/monitoring.py

pcrespov · 2025-06-20T15:59:48Z

@mergify queue

mergify · 2025-06-20T15:59:55Z

pcrespov · 2025-06-22T12:18:48Z

@mergify queue

mergify · 2025-06-22T12:18:55Z

queue

🛑 The pull request could not be merged

This could be related to an activated branch protection or ruleset rule that prevents us from merging. (details: 3 of 6 required status checks are expected.)

…d log details

…ioritize exception text

…upport ID logging

…ency in HTTP responses

sonarqubecloud · 2025-06-22T14:57:29Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

mergify · 2025-06-22T15:38:27Z

This pull request has been removed from the queue for the following reason: pull request dequeued.

Pull request #7939 has been dequeued. The pull request could not be merged. This could be related to an activated branch protection or ruleset rule that prevents us from merging. (details: 3 of 6 required status checks are expected.).

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.
If you do update this pull request, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

pcrespov · 2025-06-22T15:45:46Z

@mergify queue

mergify · 2025-06-22T15:45:53Z

queue

🛑 The pull request has been removed from the queue `default`

Pull request #7939 has been dequeued. The pull request could not be merged. This could be related to an activated branch protection or ruleset rule that prevents us from merging. (details: 3 of 6 required status checks are expected.).

You can take a look at Queue: Embarked in merge queue check runs for more details about the failure.

pcrespov self-assigned this Jun 20, 2025

pcrespov added a:services-library issues on packages/service-libs release Preparation for pre-release/release labels Jun 20, 2025

pcrespov marked this pull request as ready for review June 20, 2025 08:54

pcrespov requested review from GitHK, bisgaard-itis, Copilot, giancarloromeo and matusdrobuliak66 June 20, 2025 08:54

giancarloromeo changed the title ~~🐛 web-api: Fixes handling of unexpected erros~~ 🐛 web-api: Fixes handling of unexpected errors Jun 20, 2025

pcrespov requested a review from sanderegg June 20, 2025 08:55

Copilot AI reviewed Jun 20, 2025

View reviewed changes

packages/service-library/src/servicelib/aiohttp/rest_middlewares.py Outdated Show resolved Hide resolved

matusdrobuliak66 approved these changes Jun 20, 2025

View reviewed changes

giancarloromeo approved these changes Jun 20, 2025

View reviewed changes

sanderegg approved these changes Jun 20, 2025

View reviewed changes

bisgaard-itis approved these changes Jun 20, 2025

View reviewed changes

pcrespov added this to the Engage milestone Jun 20, 2025

mrnicegyu11 reviewed Jun 20, 2025

View reviewed changes

packages/service-library/src/servicelib/aiohttp/monitoring.py Show resolved Hide resolved

pcrespov force-pushed the is7932/fix-handling-unexpected-exceptions branch from 4b494c4 to 8ac1c97 Compare June 20, 2025 15:59

pcrespov enabled auto-merge (squash) June 20, 2025 15:59

pcrespov added the 🤖-automerge marks PR as ready to be merged for Mergify label Jun 20, 2025

pcrespov added 4 commits June 22, 2025 16:56

rm is_prod flag

4bd7caa

🐛 [Backend] Fix exception handling in middleware_factory

7e2ba87

response and log_response

737e700

cleanup

acc1124

pcrespov added 14 commits June 22, 2025 16:56

logs 500

7a763ef

refactors

549944d

🐛 [Backend] Refactor 5XX error logging into a dedicated function

a754a15

🐛 Enhance error handling in failing task test to include supportId an…

5f6947d

…d log details

🐛 Refactor user error message handling in HTTP error middleware to pr…

5185fd2

…ioritize exception text

🐛 Refactor error handling to improve user error message clarity and s…

98b9232

…upport ID logging

🐛 Refactor error handling to replace 'reason' with 'text' for consist…

673147d

…ency in HTTP responses

cleanup

61c297d

error

98dc7aa

fixes test

0d55bfe

minor

9903c16

fixes messages errors

e9ab4e6

bad import

e6cc461

fixes test

68bb19c

pcrespov force-pushed the is7932/fix-handling-unexpected-exceptions branch from 89dcb0d to 68bb19c Compare June 22, 2025 14:56

pcrespov merged commit 8116821 into ITISFoundation:master Jun 22, 2025
146 of 149 checks passed

pcrespov mentioned this pull request Jun 22, 2025

♻️✅ invitations service: small refactoring and cleanup #7945

Merged

pcrespov deleted the is7932/fix-handling-unexpected-exceptions branch June 22, 2025 16:25

matusdrobuliak66 pushed a commit that referenced this pull request Jun 23, 2025

cherry pick web-api: Fixes handling of unexpected errors #7939

74a886a

pcrespov mentioned this pull request Jun 23, 2025

🐛♻️ webserver error handling: Fix incorrect error logging for web.HTTPCreated; cleanup success response handling #7952

Merged

matusdrobuliak66 mentioned this pull request Jun 23, 2025

🚀 Release v1.83.0 #7682

Closed

92 tasks

matusdrobuliak66 mentioned this pull request Aug 5, 2025

🚀 Release v1.84.0 #7947

Closed

88 tasks

🐛 web-api: Fixes handling of unexpected errors #7939

🐛 web-api: Fixes handling of unexpected errors #7939

Uh oh!

Conversation

pcrespov commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What do these changes do?

⚠️ Important Note

Related issue/s

How to test

Dev-ops

Uh oh!

codecov bot commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

matusdrobuliak66 left a comment

Choose a reason for hiding this comment

Uh oh!

sanderegg left a comment

Choose a reason for hiding this comment

Uh oh!

bisgaard-itis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pcrespov commented Jun 20, 2025

Uh oh!

mergify bot commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🟠 Waiting for conditions to match

Uh oh!

pcrespov commented Jun 22, 2025

Uh oh!

mergify bot commented Jun 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛑 The pull request could not be merged

Uh oh!

sonarqubecloud bot commented Jun 22, 2025

Quality Gate passed

Uh oh!

mergify bot commented Jun 22, 2025

Uh oh!

pcrespov commented Jun 22, 2025

Uh oh!

mergify bot commented Jun 22, 2025

🛑 The pull request has been removed from the queue default

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

pcrespov commented Jun 20, 2025 •

edited

Loading

codecov bot commented Jun 20, 2025 •

edited

Loading

mergify bot commented Jun 20, 2025 •

edited

Loading

mergify bot commented Jun 22, 2025 •

edited

Loading

🛑 The pull request has been removed from the queue `default`