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

Add DaprInternalError.as_json_safe_dict for actors #765

Merged

Conversation

Druid-of-Luhn
Copy link
Contributor

Description

The FastAPI and Flask extensions for actors serialise the value of any raised DaprInternalError to JSON, which fails if the error contains bytes in its _raw_response_bytes field.

This change adds a new as_json_safe_dict method and uses it in place of the as_dict method in the FastAPI and Flask extensions.

Two unit tests for the as_json_safe_dict method are included.

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #761

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation (n/a)

The FastAPI and Flask extensions for actors serialise the value of
any raised DaprInternalError to JSON, which fails if the error
contains bytes in its `_raw_response_bytes` field.

This change adds a new `as_json_safe_dict` method and uses it in
place of the `as_dict` method in the FastAPI and Flask extensions.

Two unit tests for the `as_json_safe_dict` method are included.

Signed-off-by: Billy Brown <druidofluhn@gmail.com>
@Druid-of-Luhn Druid-of-Luhn requested review from a team as code owners January 7, 2025 11:44
@elena-kolevska elena-kolevska merged commit 0f6e96a into dapr:main Jan 9, 2025
13 checks passed
@elena-kolevska
Copy link
Contributor

@holopin-bot @Druid-of-Luhn Thanks for your contribution!

Copy link

holopin-bot bot commented Jan 9, 2025

Looks like @elena-kolevska is trying to award a sticker, but something went wrong while doing so. See this page for more information.

Copy link

codecov bot commented Jan 9, 2025

Codecov Report

Attention: Patch coverage is 55.55556% with 8 lines in your changes missing coverage. Please review.

Project coverage is 86.13%. Comparing base (bffb749) to head (7000b63).
Report is 19 commits behind head on main.

Files with missing lines Patch % Lines
ext/dapr-ext-fastapi/dapr/ext/fastapi/actor.py 42.85% 4 Missing ⚠️
ext/flask_dapr/flask_dapr/actor.py 20.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #765      +/-   ##
==========================================
- Coverage   86.63%   86.13%   -0.50%     
==========================================
  Files          84       89       +5     
  Lines        4473     4978     +505     
==========================================
+ Hits         3875     4288     +413     
- Misses        598      690      +92     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Druid-of-Luhn
Copy link
Contributor Author

Thanks @elena-kolevska for your suggested approach in the original issue! It was also quite a nice codebase to contribute to.

@marcduiker
Copy link
Contributor

@holopin-bot @Druid-of-Luhn Thank you! Here's a digital badge as a small token of appreciation.

Copy link

holopin-bot bot commented Jan 10, 2025

Congratulations @Druid-of-Luhn, the maintainer of this repository has issued you a badge! Here it is: https://holopin.io/claim/cm5qyr2bl55350clbu2bfdb2u

This badge can only be claimed by you, so make sure that your GitHub account is linked to your Holopin account. You can manage those preferences here: https://holopin.io/account.
Or if you're new to Holopin, you can simply sign up with GitHub, which will do the trick!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] InternalDaprError in actor method call can cause serialisation error when returning exception details
3 participants