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

Feature: Add proxy handler for client API #1226

Merged
merged 2 commits into from
Jun 25, 2022

Conversation

pizzaeueu
Copy link
Contributor

@pizzaeueu pizzaeueu commented Apr 24, 2022

Hi there!
My attempt to provide a solution for #101

Why PR to zio-http 2.x ?

Actually, I don't know the right flow. Please correct me if I need to do it in a different way. I personally need it in 2.x, so that was a motivation 😌

How did you test it?

  1. Run locally any free forward-proxy, e.g. https://github.com/clue/docker-polipo
  2. Run the following code with updated zio-http version
import zhttp.http.{Proxy, Request, URL}
import zhttp.service.Client.Config
import zhttp.service.{ChannelFactory, Client, EventLoopGroup}
import zio.ZIO

object Main extends zio.ZIOAppDefault {
  override def run = {
    val req = Request(
      url = URL
        .fromString("https://gorest.co.in/public/v2/posts")
        .toOption
        .getOrElse(URL.empty)
    )
    val proxy = Proxy(
      URL.fromString("http://127.0.0.1:8123").toOption.get
    )
    Client
      .request(
        req,
        Config().withProxy(proxy),
      )
      .provide(
        ChannelFactory.auto,
        EventLoopGroup.auto(),
      )
      .flatMap(response => ZIO.log(response.status.toString))
  }
}

Result with Proxy

timestamp=2022-04-24T07:28:44.302564Z level=INFO thread=#zio-fiber-2 message="Ok" location=test.Main.run file=Main.scala line=26

Result without Proxy

timestamp=2022-04-24T07:30:29.919041Z level=INFO thread=#zio-fiber-2 message="Ok" location=test.Main.run file=Main.scala line=26

Results with wrong proxy address

... java.nio.channels.UnresolvedAddressException: java.nio.channels.UnresolvedAddressException ...
... Connection refused: /127.0.0.1:1111

etc

@pizzaeueu pizzaeueu force-pushed the add-proxy-handler branch 2 times, most recently from 7892e69 to 8f16da2 Compare April 27, 2022 08:15
@pizzaeueu pizzaeueu requested a review from tusharmath April 27, 2022 08:33
final case class Proxy(
url: URL,
credentials: Option[Credentials] = None,
headers: Option[Headers] = None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
headers: Option[Headers] = None,
headers: Headers = Headers.empty,

@pizzaeueu pizzaeueu force-pushed the add-proxy-handler branch from 9af868e to ab7b527 Compare May 1, 2022 12:53
@pizzaeueu pizzaeueu requested a review from tusharmath May 1, 2022 12:57
@codecov-commenter
Copy link

codecov-commenter commented May 12, 2022

Codecov Report

Merging #1226 (3e99fdc) into main (15c32be) will increase coverage by 2.12%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1226      +/-   ##
==========================================
+ Coverage   59.59%   61.71%   +2.12%     
==========================================
  Files          70       71       +1     
  Lines        2450     2756     +306     
  Branches       70       92      +22     
==========================================
+ Hits         1460     1701     +241     
- Misses        990     1055      +65     
Impacted Files Coverage Δ
zio-http/src/main/scala/zhttp/http/Proxy.scala 100.00% <100.00%> (ø)
zio-http/src/main/scala/zhttp/service/Client.scala 92.04% <100.00%> (+0.97%) ⬆️
...io-http/src/main/scala/zhttp/service/package.scala 100.00% <100.00%> (ø)
...io-http/src/main/scala/zhttp/http/PathSyntax.scala 91.30% <0.00%> (-8.70%) ⬇️
zio-http/src/main/scala/zhttp/service/Server.scala 42.37% <0.00%> (-4.30%) ⬇️
...io-http/src/main/scala/zhttp/service/Handler.scala 71.79% <0.00%> (-2.12%) ⬇️
zio-http/src/main/scala/zhttp/http/Cookie.scala 80.95% <0.00%> (-1.53%) ⬇️
zio-http/src/main/scala/zhttp/http/Path.scala 79.72% <0.00%> (+0.56%) ⬆️
zio-http/src/main/scala/zhttp/http/URL.scala 91.50% <0.00%> (+4.55%) ⬆️
... and 1 more

Continue to review full report at Codecov.

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

@pizzaeueu pizzaeueu requested a review from tusharmath May 19, 2022 13:17
@tusharmath tusharmath changed the base branch from zio-series/2.x to main May 26, 2022 06:10
@tusharmath
Copy link
Collaborator

@pizzaeueu Can you make the PR wrt main. We make a ZIO 2.0 branch only at the time of release.

@pizzaeueu pizzaeueu force-pushed the add-proxy-handler branch from 9d12ca6 to 52fc850 Compare May 26, 2022 09:13
@pizzaeueu pizzaeueu force-pushed the add-proxy-handler branch from 52fc850 to af85082 Compare May 26, 2022 09:21
@pizzaeueu
Copy link
Contributor Author

pizzaeueu commented May 26, 2022

Hi @tusharmath
Thanks for the feedback

Target branch was changed to main
All conflicts were fixed

@pizzaeueu
Copy link
Contributor Author

Hi @tusharmath
Sorry for disturbing you, but are there any chances to merge this MR?
Maybe I should apply any additional fixes or smth?

I really need such feature 😌

@tusharmath tusharmath enabled auto-merge (squash) June 25, 2022 10:39
@tusharmath tusharmath changed the title Add proxy handler for client API Feature: Add proxy handler for client API Jun 25, 2022
@tusharmath tusharmath merged commit 928decc into zio:main Jun 25, 2022
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.

3 participants