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

[core] Rework HTTP networking #2861

Closed
wants to merge 394 commits into from
Closed

Conversation

coletdjnz
Copy link
Member

@coletdjnz coletdjnz commented Feb 23, 2022

Merge before #3668

Please follow the guide below

  • You will be asked some questions, please read them carefully and answer honestly
  • Put an x into all the boxes [ ] relevant to your pull request (like that [x])
  • Use Preview tab to see how your pull request will actually look like

Before submitting a pull request make sure you have:

In order to be accepted and merged into yt-dlp each piece of code must be in public domain or released under Unlicense. Check one of the following options:

  • I am the original author of this code and I am willing to release it under Unlicense
  • I am not the original author of this code but it is in public domain or released under Unlicense (provide reliable evidence)

What is the purpose of your pull request?

  • Bug fix
  • Improvement
  • New extractor
  • New feature

Description of your pull request and other information

This overhaul changes the underlying network request handling in yt-dlp.

The main goal of this was to abstract away all the network code to no longer directly interact with urllib for HTTP requests. This was so we could easily add support for more modern, updated HTTP libraries such as urllib3 to bring persistent connections, while also keeping our no-hard-dependency requirement.

TODO: https://github.com/coletdjnz/yt-dlp-dev/projects/2

Changes

The new HTTP networking interface:

  • Our own Request/Response/Exception classes that we have full control over.
  • Introduces RequestHandlers - these send supported HTTP requests and return HTTP responses
  • Introduces the RequestDirector - this allows us to use multiple RequestHandlers
  • This allows us to support multiple HTTP client libraries (each contained within a RequestHandler)

Other Improvements:

Known issues / regressions

  • Expected: urllib (and requests) handlers check the proxy scheme to see check if it is supported.
    • urllib does not support https:// proxies, so it will not be able to handle a request with one.
    • Previously, urllib would erroneously treat https:// proxies as http:// proxies. It is possible some users have wrongly supplied an https:// proxy, so their configurations will break. We never supported https:// proxies and the behavior was undefined, so we should not have to support compat for this.
  • Expected: if a user has ALL_PROXY set for any other program, yt-dlp will now pick it up.
  • External extractors/downloaders etc. using urlopen() catching urllib exceptions other than HTTPError. We only provide compat for HTTPError, the others not. We could add compat but I imagine it is unlikely there would be an issue.

Future

This branch has been force updated. Old commit history can be found in this branch: https://github.com/coletdjnz/yt-dlp-dev/tree/network-overhaul-v2

yt_dlp/networking/utils.py Outdated Show resolved Hide resolved
@pukkandan pukkandan force-pushed the master branch 2 times, most recently from a63ff77 to b14d523 Compare May 18, 2022 03:35
@coletdjnz coletdjnz removed help-wanted Extra attention is needed needs-testing Patch needs testing labels Jun 8, 2022
yt_dlp/utils.py Outdated Show resolved Hide resolved
Copy link
Member

@pukkandan pukkandan left a comment

Choose a reason for hiding this comment

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

My thoughts after skimming through

yt_dlp/extractor/common.py Outdated Show resolved Hide resolved
yt_dlp/utils.py Outdated Show resolved Hide resolved
yt_dlp/utils.py Outdated Show resolved Hide resolved
yt_dlp/utils.py Outdated Show resolved Hide resolved
yt_dlp/utils.py Outdated Show resolved Hide resolved
yt_dlp/YoutubeDL.py Outdated Show resolved Hide resolved
yt_dlp/YoutubeDL.py Outdated Show resolved Hide resolved
yt_dlp/YoutubeDL.py Outdated Show resolved Hide resolved
yt_dlp/YoutubeDL.py Outdated Show resolved Hide resolved
yt_dlp/YoutubeDL.py Outdated Show resolved Hide resolved
yt_dlp/YoutubeDL.py Outdated Show resolved Hide resolved
@pukkandan pukkandan force-pushed the network-overhaul branch 2 times, most recently from 15115a9 to c21bc7d Compare June 24, 2022 20:30
pukkandan added a commit to pukkandan/yt-dlp-dev that referenced this pull request Jul 15, 2023
No actual changes - code is only moved around
pukkandan pushed a commit to pukkandan/yt-dlp-dev that referenced this pull request Jul 15, 2023
New networking interface consists of a `RequestDirector` that directs
each `Request` to appropriate `RequestHandler` and returns the
`Response` or raises `RequestError`. The handlers define adapters to
transform its internal Request/Response/Errors to our interfaces.

User-facing changes:
- Fix issues with per request proxies on redirects for urllib
- Support for `ALL_PROXY` environment variable for proxy setting
- Support for `socks5h` proxy
   - Closes yt-dlp#6325, ytdl-org/youtube-dl#22618, ytdl-org/youtube-dl#28093
- Raise error when using `https` proxy instead of silently converting it to `http`

Authored by: coletdjnz
pukkandan pushed a commit to pukkandan/yt-dlp-dev that referenced this pull request Jul 15, 2023
@pukkandan pukkandan closed this Jul 15, 2023
@pukkandan
Copy link
Member

Merged in c365dba 227bf1a 3d2623a

aalsuwaidi pushed a commit to aalsuwaidi/yt-dlp that referenced this pull request Apr 21, 2024
No actual changes - code is only moved around
aalsuwaidi pushed a commit to aalsuwaidi/yt-dlp that referenced this pull request Apr 21, 2024
New networking interface consists of a `RequestDirector` that directs
each `Request` to appropriate `RequestHandler` and returns the
`Response` or raises `RequestError`. The handlers define adapters to
transform its internal Request/Response/Errors to our interfaces.

User-facing changes:
- Fix issues with per request proxies on redirects for urllib
- Support for `ALL_PROXY` environment variable for proxy setting
- Support for `socks5h` proxy
   - Closes yt-dlp#6325, ytdl-org/youtube-dl#22618, ytdl-org/youtube-dl#28093
- Raise error when using `https` proxy instead of silently converting it to `http`

Authored by: coletdjnz
aalsuwaidi pushed a commit to aalsuwaidi/yt-dlp that referenced this pull request Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: networking
Archived in project
Development

Successfully merging this pull request may close these issues.

Please add socks5h proxy support
8 participants