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

Migration to HTTP/2 network protocol #1172

Merged
merged 4 commits into from
Aug 11, 2021
Merged

Migration to HTTP/2 network protocol #1172

merged 4 commits into from
Aug 11, 2021

Conversation

CastagnaIT
Copy link
Owner

@CastagnaIT CastagnaIT commented May 25, 2021

Check if this PR fulfills these requirements:

Types of changes

  • New feature (non-breaking change which adds functionality)
  • Feature change (non-breaking change which change behaviour of an existing functionality)
  • Improvement (non-breaking change which improve functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

This is currently experimental

Last year the website is migrated to the HTTP/2 protocol
currently the web session is managed by the Requests module which does not support HTTP/2

This PR introduces the use of the HTTPX module, although the HTTP/2 support is still under development in this module seems to work well.
I think that the HTTPX module is the most up-to-date HTTP/2 module of this moment.

The HTTPX package is not provided with Kodi and is unlikely to be included for our request, because this add-on has not been approved for Kodi repository, it could be added if other approved add-ons will use this module,
in the meantime, you will need to include it as internal package including all dependencies and subdependencies.

This PR provides a first solution to this future problem and i think that this PR will not be included in the master branch until the current HTTP1.1 protocol will cause other problems with the endpoints or the website discontinue the support.

TODO List:

  • To check if all dependencies and sub-dependencies of each module are in place
  • Check cookies code
  • Check TCP keep-alive communications
  • To implement close session on addon closing
  • Although HTTPX imitates Requests with similar properties/arguments a quick recheck of each method is needed
  • Test all endpoints (currently tested: login, main menu and playback)
  • Test standard login on various systems to ensure no regressions (to check if "Incorrect password" error will be fixed or worsened)
  • Investigate for error: httpx.RemoteProtocolError: <ConnectionTerminated error_code:ErrorCodes.COMPRESSION_ERROR, last_stream_id:2147483647, additional_data:485041434b202d20696c6c6567616c20696e6465>
    happen only sometimes and after playing multiple videos, ATM cause not know, could be also an httpx bug (https://paste.kodi.tv/iyubevuxey) (UPD 23/06/2021 after updated all sub-deps seem there is no more errors at least on Windows)

From my tests this PR seem to fully fix the user+password login issues,
i have tried test logging several times on Windows, Linux Mint, CoreELEC with the result of 100% login success,

hoping it is not a coincidence it would be helpful if someone report login test feedbacks

@CastagnaIT CastagnaIT added the WIP PR that is still being worked on label May 25, 2021
@CastagnaIT CastagnaIT force-pushed the http2 branch 3 times, most recently from c6e59d9 to 9a77c5b Compare June 14, 2021 07:28
@CastagnaIT CastagnaIT force-pushed the http2 branch 2 times, most recently from 0a79771 to 5bf0865 Compare June 16, 2021 16:14
@CastagnaIT CastagnaIT added the Testers needed To resolve this issue other testers are required label Jun 17, 2021
@Essam312
Copy link

I tested the login in a clean installation of Kodi 19.1 with the email and password and it worked without any problem without the need for using the authentication key, I'm using windows 10.

@CastagnaIT
Copy link
Owner Author

good to hear!

@CastagnaIT
Copy link
Owner Author

hi @betatester3016
when you will have time, can you do a test with this version?
plugin.video.netflix_1.16.2+matrix.1_20210718_http2_t1.zip

I need to know a simply thing, if the standard login (user+password) works without raise error "incorrect password",
please take attention when you write the credentials (to not invalidate results),
of course before delete previous addon versions and related data (if installed).

On systems:

  • Linux desktop computer (like ubuntu etc... as you prefeer, x86)
  • ARM device with LibreElec (or CoreElec)

@betatester3016
Copy link

@CastagnaIT

  • Ubuntu 20.04.2 LTS / Kodi 19 on x86
  • CoreELEC 19.2 RC2 on S905X3

I confirm being able to log in with username and password on both O.S. when using: plugin.video.netflix_1.16.2+matrix.1_20210718_http2_t1

The results can be replicated by uninstalling and reinstalling the provided add on.

@CastagnaIT CastagnaIT removed the WIP PR that is still being worked on label Jul 21, 2021
@CastagnaIT
Copy link
Owner Author

Thank you i am testing/using this version from about 1 month, and to me works good, i do not see wrong behaviours/errors
i'm thinking of merge it soon

@Ronny-nerd
Copy link

Ronny-nerd commented Aug 10, 2021

Android 9 = Nvidia Shield TV
and plugin.video.netflix_1.16.2+matrix.1_20210718_http2_t1
Nightly Kodi Matrix 19.1

No Problem with user+password login = no authentication key = all OK

@CastagnaIT
Copy link
Owner Author

thanks for the report i think to include this on next release if i will find a bit of time

@CastagnaIT CastagnaIT removed the Testers needed To resolve this issue other testers are required label Aug 11, 2021
@CastagnaIT CastagnaIT merged commit bd6b580 into master Aug 11, 2021
@CastagnaIT CastagnaIT deleted the http2 branch August 11, 2021 16:19
CastagnaIT added a commit that referenced this pull request Oct 28, 2022
Currently there is no way to enable TCP KeepAlive directly on httpx
module

ref. PR:#1172
CastagnaIT added a commit that referenced this pull request Sep 13, 2023
Currently there is no way to enable TCP KeepAlive directly on httpx
module

ref. PR:#1172
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.

4 participants