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

Implement handleErrorCauseZIO for executing ZIO effects on error #2513

Merged
merged 3 commits into from
Nov 15, 2023

Conversation

SHSongs
Copy link
Contributor

@SHSongs SHSongs commented Nov 9, 2023

  • Error Handling in Routes: Implemented the handleErrorCauseZIO function that is triggered upon encountering an error, ensuring that side effects are performed.

  • Testing: Added tests to check that handleErrorCauseZIO correctly executes ZIO effects using TestConsole, when handling errors.

cc: @guersam

@codecov-commenter
Copy link

codecov-commenter commented Nov 9, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (735d9b7) 64.95% compared to head (e5efa2b) 65.06%.
Report is 6 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2513      +/-   ##
==========================================
+ Coverage   64.95%   65.06%   +0.11%     
==========================================
  Files         137      137              
  Lines        7208     7243      +35     
  Branches     1288     1248      -40     
==========================================
+ Hits         4682     4713      +31     
- Misses       2526     2530       +4     
Files Coverage Δ
zio-http/src/main/scala/zio/http/Handler.scala 63.28% <100.00%> (+0.17%) ⬆️
zio-http/src/main/scala/zio/http/Routes.scala 72.22% <0.00%> (-4.25%) ⬇️
zio-http/src/main/scala/zio/http/Route.scala 74.19% <66.66%> (-1.81%) ⬇️

... and 4 files with indirect coverage changes

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

@SHSongs SHSongs changed the title Implement handleErrorCauseZIO for executing side effects on error Implement handleErrorCauseZIO for executing ZIO effects on error Nov 9, 2023
)(implicit trace: Trace): Handler[R1, Err1, In, Out1] =
self.foldCauseHandler(
err =>
if (err.isInterruptedOnly) Handler.fromZIO(f(err.asInstanceOf[Cause[Nothing]])) else Handler.fromZIO(f(err)),
Copy link
Member

Choose a reason for hiding this comment

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

These code paths do not differ, they both create the handler from f(err). You shouldn't need the cast or the branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  /**
   * Transforms all failures except pure interruption.
   */
  final def mapErrorCause[Err2](f: Cause[Err] => Err2)(implicit trace: Trace): Handler[R, Err2, In, Out] =
    self.foldCauseHandler(
      err => if (err.isInterruptedOnly) Handler.failCause(err.asInstanceOf[Cause[Nothing]]) else Handler.fail(f(err)),
      Handler.succeed(_),
    )

I have referred to mapErrorCause. mapErrorCause casts Cause[Nothing] if isInterruptedOnly is true. Do you have more information about its origin?
Additionally, do you recommend any modifications to the mapErrorCause?

Copy link
Contributor

Choose a reason for hiding this comment

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

@SHSongs f should not be applied when it's interrupt only. Please double check the usage of f in the original code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reviewed the usage of f. The f used transforms all failures, except for pure interruption. I also made changes to the code, following the original.

@jdegoes jdegoes merged commit b3b4cb0 into zio:main Nov 15, 2023
14 checks passed
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.

4 participants