Add compile-time flag to remove code for QUIC #2364

syeopite · 2021-08-30T21:56:30Z

No description provided.

syeopite · 2021-08-31T21:52:32Z

Using Invidious without QUIC has some issues

unixfox · 2021-09-01T11:57:24Z

Why this got closed? I know how to fix the issues.

syeopite · 2021-09-28T22:15:01Z

There we go, that should fix the gzip issues. However, in order to make Invidious fully functional without lsquic, we're going to need to replace the few dozen usages of :authority with a HTTP/1.1 equivalent; otherwise, we can't support images. That's a bit out of scope for this PR though.

src/invidious/helpers/utils.cr

SamantazFox · 2021-09-28T22:59:41Z

src/invidious/helpers/utils.cr

-        conn = QUIC::Client.new(url)
-      else
+    DB::Pool(ConnectonClientType).new(initial_pool_size: 0, max_pool_size: capacity, max_idle_pool_size: capacity, checkout_timeout: timeout) do
+      conn = nil # Declare


This is not required

It is actually required. The disable_quic logic is considered a macro(?) so we can't actually access conn unless we declare it outside first. You can try to build Invidious without this yourself, but it'll just result in a compile time error.

Whaaaa, even the inline version doesn't work!
conn = {% unless flag?(:disable_quic) %} use_quic ? QUIC::Client.new(url) : {% end %} HTTP::Client.new(url)

This is definitely an upstream bug...

That works, though:

conn = ( {% unless flag?(:disable_quic) %} CONFIG.use_quic ? QUIC::Client.new(url) : HTTP::Client.new(url) {% else %} HTTP::Client.new(url) {% end %} )

src/invidious/routes/login.cr

src/invidious/helpers/utils.cr

TheFrenchGhosty · 2021-10-03T17:17:03Z

Blocked by #2364 (comment)

syeopite · 2021-10-08T15:00:42Z

Blocked by #2364 (comment)

Should be fixed.

syeopite · 2021-10-13T20:07:41Z

Running this PR causes the following to be outputted within the logs occasionally. Though, it doesn't seem to actually cause any noticeable effect.

Exception: SSL_connect: I/O error (OpenSSL::SSL::Error)
  from /usr/share/crystal/src/openssl/ssl/socket.cr:34:11 in 'initialize'
  from /usr/share/crystal/src/openssl/ssl/socket.cr:3:5 in 'new:context:sync_close:hostname'
  from src/invidious/helpers/crystal_class_overrides.cr:34:5 in 'io'
  from /usr/share/crystal/src/http/client.cr:664:5 in 'send_request'
  from /usr/share/crystal/src/http/client.cr:599:5 in 'exec_internal_single'
  from /usr/share/crystal/src/http/client.cr:586:5 in 'exec_internal'
  from /usr/share/crystal/src/http/client.cr:581:7 in 'exec'
  from /usr/share/crystal/src/http/client.cr:706:5 in 'exec'
  from /usr/share/crystal/src/http/client.cr:738:7 in 'exec'
  from /usr/share/crystal/src/http/client.cr:406:3 in 'head'
  from src/invidious.cr:1514:7 in '->'
  from /usr/share/crystal/src/primitives.cr:266:3 in '->'
  from src/invidious/helpers/handlers.cr:30:5 in 'process_request'
  from lib/kemal/src/kemal/route_handler.cr:17:7 in 'call'
  from /usr/share/crystal/src/http/server/handler.cr:28:7 in 'call_next'
  from lib/kemal/src/kemal/websocket_handler.cr:13:14 in 'call'
  from /usr/share/crystal/src/http/server/handler.cr:28:7 in 'call_next'
  from lib/kemal/src/kemal/filter_handler.cr:21:7 in 'call'
  from /usr/share/crystal/src/http/server/handler.cr:28:7 in 'call_next'
  from src/invidious/helpers/handlers.cr:212:5 in 'call'
  from /usr/share/crystal/src/http/server/handler.cr:28:7 in 'call_next'
  from src/invidious/helpers/handlers.cr:94:12 in 'call'
  from /usr/share/crystal/src/http/server/handler.cr:28:7 in 'call_next'
  from src/invidious/helpers/handlers.cr:145:12 in 'call'
  from /usr/share/crystal/src/http/server/handler.cr:28:7 in 'call_next'
  from src/invidious/helpers/handlers.cr:68:12 in 'call'
  from /usr/share/crystal/src/http/server/handler.cr:28:7 in 'call_next'
  from src/invidious/helpers/static_file_handler.cr:189:11 in 'call'
  from /usr/share/crystal/src/http/server/handler.cr:28:7 in 'call_next'
  from lib/kemal/src/kemal/exception_handler.cr:8:7 in 'call'
  from /usr/share/crystal/src/http/server/handler.cr:28:7 in 'call_next'
  from src/invidious/helpers/logger.cr:17:35 in 'call'
  from /usr/share/crystal/src/http/server/handler.cr:28:7 in 'call_next'
  from lib/kemal/src/kemal/init_handler.cr:12:7 in 'call'
  from /usr/share/crystal/src/http/server/request_processor.cr:51:11 in 'process'
  from /usr/share/crystal/src/http/server.cr:515:5 in 'handle_client'
  from /usr/share/crystal/src/http/server.cr:468:13 in '->'
  from /usr/share/crystal/src/primitives.cr:266:3 in 'run'
  from /usr/share/crystal/src/fiber.cr:92:34 in '->'
  from ???
Caused by: SSL_connect: Success (RuntimeError)

Though Invidious doesn't seem to crash anymore, which is great!

syeopite · 2021-10-16T15:44:13Z

For anyone that decides to test this PR in the future, could you please monitor the memory usage and crashes? I'm curious if this PR helps in any way, shape, or form on Invidious' general instability.

unixfox · 2021-10-22T13:36:24Z

Could you rebase? Thank you.

syeopite · 2021-10-23T11:14:43Z

Done

syeopite · 2021-11-02T02:11:23Z

src/invidious/yt_backend/youtube_api.cr

+    # and decompresses. However, explicitly applying it will remove this functionality.
+    #
+    # https://github.com/crystal-lang/crystal/issues/11252#issuecomment-929594741
+    {% unless flag?(:disable_quic) %}


TODO: Check for the configuration option as well

SamantazFox · 2021-11-05T15:45:53Z

src/invidious/routes/images.cr

+          end
+        else


Suggested change

end

else

end

else

{% end %}

# This can likely be optimized into a (small) pool sometime in the future.

SamantazFox · 2021-11-05T15:50:17Z

src/invidious/routes/images.cr

+        end
+      {% else %}
+        # This can likely be optimized into a (small) pool sometime in the future.
+        HTTP::Client.get("https://yt3.ggpht.com#{url}") do |resp|
+          return request_proc.call(resp)
+        end
+      {% end %}


Suggested change

end

{% else %}

# This can likely be optimized into a (small) pool sometime in the future.

HTTP::Client.get("https://yt3.ggpht.com#{url}") do |resp|

return request_proc.call(resp)

end

{% end %}

{% unless flag?(:disable_quic) %}

end

{% end %}

This, and the above, avoid duplicate code (only the macro condition is duplicated)

SamantazFox · 2021-11-05T15:57:21Z

src/invidious/routes/images.cr

+    request_proc = ->(response : HTTP::Client::Response) {
+      env.response.status_code = response.status_code
+      response.headers.each do |key, value|
+        if !RESPONSE_HEADERS_BLACKLIST.includes?(key.downcase)
+          env.response.headers[key] = value


this, and the code around (headers, response code check) is extremely similar between those three image proxy functions. This could be made into a separate function.

SamantazFox · 2021-11-08T19:53:49Z

src/invidious/routes/images.cr

+          if CONFIG.use_quic
+            if YT_POOL.client &.head(thumbnail_resource_path, headers).status_code == 200
+              name = thumb[:url] + ".jpg"
+              break
+            end
+          else
+            if HTTP::Client.head("https://i.ytimg.com#{thumbnail_resource_path}").status_code == 200
+              name = thumb[:url] + ".jpg"
+              break
+            end
+          end


I'm wondering something: why isn't the if CONFIG.use_quic check made at the YT_POOL pool level, rather than here?

Continuation of b0f127d

See iv-org#2577

syeopite · 2021-11-12T12:05:27Z

Just added a commit to disable quic by default due to #2577. Once merged, assuming the user hasn't modified the use_quic attribute, Invidious will work again ootb.

unixfox · 2021-11-12T18:11:53Z

For what is QUIC still used in the invidious code?

syeopite · 2021-11-12T22:38:09Z

QUIC shouldn't be used at all now when either use_quic is turned off, or the compile time flag disable_quic is applied.

To be absolutely certain of that, you can just go with the latter route and remove the dependency entirely from the libs folder.

Regression from iv-org#2364

syeopite closed this Aug 31, 2021

syeopite reopened this Aug 31, 2021

syeopite marked this pull request as draft August 31, 2021 21:52

syeopite closed this Sep 1, 2021

syeopite reopened this Sep 1, 2021

syeopite force-pushed the disable-quic-via-compile-time-flag branch from 1c585d6 to a5344cf Compare September 28, 2021 22:10

syeopite marked this pull request as ready for review September 28, 2021 22:15

syeopite requested a review from SamantazFox as a code owner September 28, 2021 22:15

SamantazFox requested changes Sep 28, 2021

View reviewed changes

TheFrenchGhosty added ready blocked require something else first and removed ready labels Oct 3, 2021

TheFrenchGhosty marked this pull request as draft October 3, 2021 17:16

syeopite marked this pull request as ready for review October 8, 2021 15:00

syeopite requested a review from a team as a code owner October 8, 2021 15:00

syeopite added ready and removed blocked require something else first labels Oct 8, 2021

syeopite removed the request for review from a team October 14, 2021 15:50

syeopite force-pushed the disable-quic-via-compile-time-flag branch from 345d70f to 4c0a8d9 Compare October 14, 2021 15:50

syeopite added the need-testing This feature needs to be deployed and tested to see if it's working, and doesn't break something label Oct 16, 2021

unixfox mentioned this pull request Oct 22, 2021

Replay invidious traffic to invidious without QUIC yewtudotbe/invidious-custom#4

Closed

syeopite force-pushed the disable-quic-via-compile-time-flag branch from 4c0a8d9 to f1f76d2 Compare October 23, 2021 11:14

syeopite force-pushed the disable-quic-via-compile-time-flag branch from f1f76d2 to abc957a Compare October 28, 2021 00:49

syeopite commented Nov 2, 2021

View reviewed changes

SamantazFox requested changes Nov 7, 2021

View reviewed changes

SamantazFox requested changes Nov 8, 2021

View reviewed changes

syeopite added 11 commits November 12, 2021 03:47

Add compile-time flag to remove code for QUIC

d379a36

Fix gzip decompression with HTTP::Client

b0f127d

Respect use_quic param and fix typos

2451221

Allow ggpht queries with QUIC disabled

c3747c2

Allow storyboard queries with QUIC disabled

6b84505

Use https for ggpht requests

547abe1

Use https for storyboard image requests

814c9e6

Allow thumbnail queries with QUIC disabled

83556ba

Fix copy-paste error

48191ac

Fix handling for maxres thumbnail

6ec4dcf

Remove of gzip header w/ use_quic config

65fbdbf

Continuation of b0f127d

syeopite force-pushed the disable-quic-via-compile-time-flag branch from abc957a to 65fbdbf Compare November 12, 2021 11:56

Disable quic by default

a120f14

See iv-org#2577

syeopite mentioned this pull request Nov 12, 2021

"Could not check out a connection in 5.0 seconds" - YouTube servers stopped serving HTTP3/QUIC #2577

Closed

TheFrenchGhosty merged commit f707f99 into iv-org:master Nov 12, 2021

syeopite deleted the disable-quic-via-compile-time-flag branch November 12, 2021 12:59

syeopite added a commit to syeopite/invidious that referenced this pull request Dec 9, 2023

Fix headers not being added in image requests

2154697

Regression from iv-org#2364

syeopite added a commit to syeopite/invidious that referenced this pull request Apr 25, 2024

Fix headers not being added in image requests

865040f

Regression from iv-org#2364

syeopite added a commit to syeopite/invidious that referenced this pull request Aug 25, 2024

Fix headers not being added in image requests

06e1a50

Regression from iv-org#2364

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add compile-time flag to remove code for QUIC #2364

Add compile-time flag to remove code for QUIC #2364

syeopite commented Aug 30, 2021

syeopite commented Aug 31, 2021

unixfox commented Sep 1, 2021

syeopite commented Sep 28, 2021 •

edited

Loading

SamantazFox Sep 28, 2021

syeopite Sep 29, 2021

SamantazFox Sep 29, 2021

SamantazFox Sep 29, 2021 •

edited

Loading

TheFrenchGhosty commented Oct 3, 2021

syeopite commented Oct 8, 2021

syeopite commented Oct 13, 2021 •

edited

Loading

syeopite commented Oct 16, 2021

unixfox commented Oct 22, 2021

syeopite commented Oct 23, 2021

syeopite Nov 2, 2021

SamantazFox Nov 5, 2021

SamantazFox Nov 5, 2021

SamantazFox Nov 5, 2021

SamantazFox Nov 5, 2021

SamantazFox Nov 8, 2021

syeopite commented Nov 12, 2021 •

edited

Loading

unixfox commented Nov 12, 2021

syeopite commented Nov 12, 2021

Add compile-time flag to remove code for QUIC #2364

Add compile-time flag to remove code for QUIC #2364

Conversation

syeopite commented Aug 30, 2021

syeopite commented Aug 31, 2021

unixfox commented Sep 1, 2021

syeopite commented Sep 28, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SamantazFox Sep 29, 2021 • edited Loading

Choose a reason for hiding this comment

TheFrenchGhosty commented Oct 3, 2021

syeopite commented Oct 8, 2021

syeopite commented Oct 13, 2021 • edited Loading

syeopite commented Oct 16, 2021

unixfox commented Oct 22, 2021

syeopite commented Oct 23, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

syeopite commented Nov 12, 2021 • edited Loading

unixfox commented Nov 12, 2021

syeopite commented Nov 12, 2021

syeopite commented Sep 28, 2021 •

edited

Loading

SamantazFox Sep 29, 2021 •

edited

Loading

syeopite commented Oct 13, 2021 •

edited

Loading

syeopite commented Nov 12, 2021 •

edited

Loading