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鈥檒l 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

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,

@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
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.

None yet

3 participants