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 access to Env in error handler #2870

Closed
davidlar opened this issue May 28, 2024 · 6 comments · Fixed by #3115
Closed

Add access to Env in error handler #2870

davidlar opened this issue May 28, 2024 · 6 comments · Fixed by #3115
Labels

Comments

@davidlar
Copy link
Contributor

Hi!

Would it be possible to add support for accessing Env in e.g.
Routes.handleErrorRequestCauseZIO(f: (Request, Cause[Err]) => ZIO[Any, Nothing, Response]): Route[Env, Nothing]

so it becomes

Routes.handleErrorRequestCauseZIO(f: (Request, Cause[Err]) => ZIO[Env, Nothing, Response]): Route[Env, Nothing]?
It would be great to be able to get logged in user, correlation id etc when generating the error message.

Thanks!

@davidlar davidlar added the enhancement New feature or request label May 28, 2024
@jdegoes
Copy link
Member

jdegoes commented Jun 4, 2024

/bounty $50

Copy link

algora-pbc bot commented Jun 4, 2024

💎 $50 bounty • ZIO

Steps to solve:

  1. Start working: Comment /attempt #2870 with your implementation plan
  2. Submit work: Create a pull request including /claim #2870 in the PR body to claim the bounty
  3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

Thank you for contributing to zio/zio-http!

Add a bountyShare on socials

Attempt Started (GMT+0) Solution
🟢 @987Nabil #3115

@987Nabil
Copy link
Contributor

987Nabil commented Jun 6, 2024

@jdegoes I tried this and came to have an issue with this definition of Route

  private final case class Augmented[InEnv, -OutEnv, +Err](
    route: Route[InEnv, Err],
    aspect: Handler[InEnv, Response, Request, Response] => Handler[OutEnv, Response, Request, Response],
  ) extends Route[OutEnv, Err] {
    def location: Trace = route.location

    def routePattern: RoutePattern[_] = route.routePattern

    override def toHandler(implicit ev: Err <:< Response, trace: Trace): Handler[OutEnv, Response, Request, Response] =
      aspect(route.toHandler)

    override def toString() = s"Route.Augmented(${route}, ${aspect})"
  }

I think this makes it currently impossible to have the an Env added in the xxxZIO operators on Route/Routes.
The error transformation can be done, since the aspect does not depend on Err. It will only be called, when Err is already eliminated.
But aspect depends on InEnv. If I modify InEnv via mapErrorZIO for example, the aspect can't be applied to the Routes[InEnv with NewEnv, NewErr] anymore.

We could restrict transform, which is the only way to create an Augmented route, to Route[Any, Err]. So aspect would be Handler[Any, Response, Request, Response] => Handler[OutEnv, Response, Request, Response].

But I am not sure that is a good idea, because then you might need to provide InEnv that you never need, since you want to ignore the original handler like transform(_ => Handler.ok) for example.

@jdegoes
Copy link
Member

jdegoes commented Aug 18, 2024

@987Nabil Have you tried to make InEnv contravariant?

Copy link

algora-pbc bot commented Sep 9, 2024

💡 @987Nabil submitted a pull request that claims the bounty. You can visit your bounty board to reward.

987Nabil added a commit to 987Nabil/zio-http that referenced this issue Sep 9, 2024
jdegoes pushed a commit that referenced this issue Sep 9, 2024
* Remove route builder

* Env access in handle error (#2870)
Copy link

algora-pbc bot commented Sep 9, 2024

🎉🎈 @987Nabil has been awarded $50! 🎈🎊

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 a pull request may close this issue.

3 participants