Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Fixes unhandled KeyError when missing keys in exception's msg_template #6188

Merged
merged 7 commits into from
Aug 15, 2024

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Aug 14, 2024

What do these changes do?

Addresses this error found in tip.science deploy

Unexpected KeyError: Oops! Something went wrong, 
but we've noted it down and we'll sort it out ASAP. 
Thanks for your patience! [OEC:139646582688800]

The api-server responds 500 to GET /v2/computations/ ... instead of 404 (in this case) due to an unhandled error

Details

  • 🧪 Reproduced logs in test_oec_139646582688800_missing_ctx_values_for_msg_template
  • 🎨 Overrides OsparcErrorMixin.__str__ to avoid raising KeyError when key is missing in template error messages. The message will include a default placeholder instead of raising and stopping execution

Related issue/s

How to test

  • SEE services/api-server/tests/unit/test_services_directorv2.py reproduces log report

Dev-ops checklist

Copy link
Contributor

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!

@pcrespov pcrespov enabled auto-merge (squash) August 14, 2024 12:54
Copy link

codecov bot commented Aug 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.0%. Comparing base (cafbf96) to head (70599d5).
Report is 437 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #6188      +/-   ##
=========================================
+ Coverage    84.5%   88.0%    +3.4%     
=========================================
  Files          10    1315    +1305     
  Lines         214   57071   +56857     
  Branches       25    1132    +1107     
=========================================
+ Hits          181   50261   +50080     
- Misses         23    6564    +6541     
- Partials       10     246     +236     
Flag Coverage Δ
integrationtests 64.8% <ø> (?)
unittests 85.8% <100.0%> (+1.3%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...odels-library/src/models_library/errors_classes.py 100.0% <100.0%> (ø)
...icelib/rabbitmq/rpc_interfaces/catalog/services.py 0.0% <ø> (ø)
...vice_api_server/exceptions/service_errors_utils.py 88.7% <100.0%> (ø)
...simcore_service_api_server/services/director_v2.py 80.2% <ø> (ø)

... and 1302 files with indirect coverage changes

@pcrespov pcrespov force-pushed the fix/oec_1396 branch 2 times, most recently from 59b631b to f1be1ae Compare August 14, 2024 18:24
Copy link

@pcrespov pcrespov disabled auto-merge August 15, 2024 08:28
@pcrespov pcrespov merged commit 5cd7d13 into ITISFoundation:master Aug 15, 2024
57 checks passed
@pcrespov pcrespov deleted the fix/oec_1396 branch August 15, 2024 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants