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

Upgrade httpx #596

Merged
merged 1 commit into from
Jan 5, 2022
Merged

Upgrade httpx #596

merged 1 commit into from
Jan 5, 2022

Conversation

dalf
Copy link
Member

@dalf dalf commented Dec 11, 2021

What does this PR do?

Upgrade to httpx 0.21.2

Change in searx.network.client:

  • remove close_connections_for_url : httpcore closes the connections now.
  • keep AsyncProxyTransportFixed to map the exception from httpx_socks to httpx exception WITH the "request" object.
  • remove AsyncHTTPTransportFixed: the retry mecanism has moved to searx.network.network to decrease the number of access to the private API of httpx / httpcore.

httpx.RemoteProtocolError happens when a server closes an HTTP/2 connection. In the master branch, AsyncHTTPTransportFixed closes all connections. In this PR, the httpx.AsyncClient is closed. It makes no difference except when different proxies are specified, for example:

proxies:
  https://bing.com/: socks5h://127.0.0.1:1337
  all://: socks5h://127.0.0.1:1338

With this configuration, and with an httpx.RemoteProtocolError exception coming from the proxy on the port 1337:

  • in the master branch: connections using the proxy on the port 1337 are closed. Connections using the proxy on the port 1338 are untouched.
  • in this PR: connections using the proxy on ports 1337 and 1338 are closed.

@unixfox @tiekoetter @paulgio I don't know you use this configuration?

An alternative configuration not impacted by this PR:

outgoing:
	proxies:
		all://: socks://127.0.0.1:1337	

engines:
  - name: bing
    engine: bing
    shortcut: bi
	proxies:
		all://: socks://127.0.0.1:1338

This is WIP, todo list:

  • use a released httpcore version
  • check for memory leak over a week
  • check for error log over a week:
    • in the master branch, there is a close connections + retry mechanism in a case of specific error.
      this a workaround to fix some of the httpcore issues
    • the latest httpcore commit seems to have fix most of them
    • the change in this commit remove the "close connections + retry"
    • an error log over a long time can confirm that httpcore has been fixed

Why is this change important?

  • Remove a bunch of hack to fix an old httpcore version
  • With this change, httpx can be upgraded easily (= just change requirements.txt)

How to test this PR locally?

A lot of changes have been made to httpx, httpcore and anyio since the last upgrade, a full check is require over a long time.

what can be done locally:

  • check the error for various HTTP error (domain not found)
  • check that HTTP proxy works
  • check that SOCKS proxy works
  • check that HTTP2 connection works
  • check this scenario:
    • make a HTTP2 request
    • wait for the server to disconnect
    • maka another HTTP2 request

Author's checklist

Related issues

Related to #589

@dalf dalf force-pushed the upgrade-httpx branch 5 times, most recently from 6759703 to c21749b Compare December 14, 2021 21:30
@mrpaulblack
Copy link
Member

I tested this on my instance over a week:
image

Memory Increased over the span of 3 days by 13megabytes which is IMO margin of error.

Also with this PR memory usage dropped a little and the performance is the same or even slightly better in general IMO

@dalf
Copy link
Member Author

dalf commented Dec 22, 2021

The only missing part is a new release of httpcore, after that the PR is ready to merge.

@unixfox
Copy link
Member

unixfox commented Dec 22, 2021

I don't use proxies anymore, but I do I've plans to add some in the future but not for the default engines, for some engines that block public instances like qwant.

@mrpaulblack
Copy link
Member

mrpaulblack commented Dec 22, 2021

@unixfox I have actually qwant on my instance enabled, because they never IP block it tbh (and on top of this qwant is really fast; around 400ms per request)...

@HLFH
Copy link
Contributor

HLFH commented Jan 1, 2022

Besides the missing httpcore release, the branches upgrade-httpx and/or test-uwsgi are ready to merge.

httpx 0.21.2 and httpcore 0.14.4 fix multiple issues:
* https://github.com/encode/httpx/releases/tag/0.21.2
* https://github.com/encode/httpcore/releases/tag/0.14.4

so most of the workarounds in searx.network have been removed.
@dalf dalf marked this pull request as ready for review January 5, 2022 17:47
@dalf dalf merged commit 9004e84 into searxng:master Jan 5, 2022
@dalf dalf deleted the upgrade-httpx branch January 5, 2022 18:13
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

4 participants