Fix issues raised by code review

Remove explicit `self.` from #process of parsers

Remove explicit return tuple in get_issue_template

Fix formatting

Move inline issue template style to stylesheet

Use @id in ProblematicTimelineItem xml repr

Fix naming
This commit is contained in:
syeopite 2025-04-05 12:23:42 -07:00
parent 7b27585454
commit 6c063436d4
No known key found for this signature in database
GPG Key ID: A73C186DA3955A1A
7 changed files with 25 additions and 22 deletions

View File

@ -880,4 +880,9 @@ h1, h2, h3, h4, h5, p,
.error-card pre { .error-card pre {
height: 300px; height: 300px;
}
.error-issue-template {
padding: 20px;
background: rgba(0, 0, 0, 0.12345);
} }

View File

@ -31,7 +31,7 @@ def get_issue_template(env : HTTP::Server::Context, exception : Exception) : Tup
issue_template += github_details("Backtrace", exception.inspect_with_backtrace) issue_template += github_details("Backtrace", exception.inspect_with_backtrace)
return {issue_title, issue_template} return issue_title, issue_template
end end
def error_template_helper(env : HTTP::Server::Context, status_code : Int32, exception : Exception) def error_template_helper(env : HTTP::Server::Context, status_code : Int32, exception : Exception)
@ -78,7 +78,7 @@ def error_template_helper(env : HTTP::Server::Context, status_code : Int32, exce
<p>#{translate(locale, "crash_page_report_issue", url_new_issue)}</p> <p>#{translate(locale, "crash_page_report_issue", url_new_issue)}</p>
<!-- TODO: Add a "copy to clipboard" button --> <!-- TODO: Add a "copy to clipboard" button -->
<pre style="padding: 20px; background: rgba(0, 0, 0, 0.12345);">#{issue_template}</pre> <pre class="error-issue-template">#{issue_template}</pre>
</div> </div>
END_HTML END_HTML

View File

@ -320,7 +320,7 @@ struct ProblematicTimelineItem
def to_xml(env, locale, xml : XML::Builder) def to_xml(env, locale, xml : XML::Builder)
xml.element("entry") do xml.element("entry") do
xml.element("id") { xml.text "iv-err-#{Random.new.base64(8)}" } xml.element("id") { xml.text "iv-err-#{@id}" }
xml.element("title") { xml.text "Parse Error: This item has failed to parse" } xml.element("title") { xml.text "Parse Error: This item has failed to parse" }
xml.element("updated") { xml.text Time.utc.to_rfc3339 } xml.element("updated") { xml.text Time.utc.to_rfc3339 }

View File

@ -501,9 +501,7 @@ def extract_playlist_videos(initial_data : Hash(String, JSON::Any))
}) })
end end
rescue ex rescue ex
videos << ProblematicTimelineItem.new( videos << ProblematicTimelineItem.new(parse_exception: ex)
parse_exception: ex
)
end end
return videos return videos

View File

@ -13,14 +13,14 @@ module Invidious::Routes::Embed
raise NotFoundException.new(translate(locale, "error_video_not_in_playlist", url)) raise NotFoundException.new(translate(locale, "error_video_not_in_playlist", url))
end end
get_first_video = videos[0].as(PlaylistVideo) first_playlist_video = videos[0].as(PlaylistVideo)
rescue ex : NotFoundException rescue ex : NotFoundException
return error_template(404, ex) return error_template(404, ex)
rescue ex rescue ex
return error_template(500, ex) return error_template(500, ex)
end end
url = "/embed/#{get_first_video}?#{env.params.query}" url = "/embed/#{first_playlist_video}?#{env.params.query}"
if env.params.query.size > 0 if env.params.query.size > 0
url += "?#{env.params.query}" url += "?#{env.params.query}"
@ -75,14 +75,14 @@ module Invidious::Routes::Embed
raise NotFoundException.new(translate(locale, "error_video_not_in_playlist", url)) raise NotFoundException.new(translate(locale, "error_video_not_in_playlist", url))
end end
get_first_video = videos[0].as(PlaylistVideo) first_playlist_video = videos[0].as(PlaylistVideo)
rescue ex : NotFoundException rescue ex : NotFoundException
return error_template(404, ex) return error_template(404, ex)
rescue ex rescue ex
return error_template(500, ex) return error_template(500, ex)
end end
url = "/embed/#{get_first_video.id}" url = "/embed/#{first_playlist_video.id}"
elsif video_series elsif video_series
url = "/embed/#{video_series.shift}" url = "/embed/#{video_series.shift}"
env.params.query["playlist"] = video_series.join(",") env.params.query["playlist"] = video_series.join(",")

View File

@ -106,7 +106,7 @@
</div> </div>
<details> <details>
<summary class="pure-button pure-button-secondary"><%=translate(locale, "timeline_parse_error_show_technical_details")%></summary> <summary class="pure-button pure-button-secondary"><%=translate(locale, "timeline_parse_error_show_technical_details")%></summary>
<pre style="padding: 20px; background: rgba(0, 0, 0, 0.12345);"><%=get_issue_template(env, item.parse_exception)[1]%></pre> <pre class="error-issue-template"><%=get_issue_template(env, item.parse_exception)[1]%></pre>
</details> </details>
</div> </div>
<% else %> <% else %>

View File

@ -62,7 +62,7 @@ private module Parsers
extend self extend self
include BaseParser include BaseParser
def self.process(item : JSON::Any, author_fallback : AuthorFallback) def process(item : JSON::Any, author_fallback : AuthorFallback)
if item_contents = (item["videoRenderer"]? || item["gridVideoRenderer"]?) if item_contents = (item["videoRenderer"]? || item["gridVideoRenderer"]?)
return self.parse(item_contents, author_fallback) return self.parse(item_contents, author_fallback)
end end
@ -190,7 +190,7 @@ private module Parsers
extend self extend self
include BaseParser include BaseParser
def self.process(item : JSON::Any, author_fallback : AuthorFallback) def process(item : JSON::Any, author_fallback : AuthorFallback)
if item_contents = (item["channelRenderer"]? || item["gridChannelRenderer"]?) if item_contents = (item["channelRenderer"]? || item["gridChannelRenderer"]?)
return self.parse(item_contents, author_fallback) return self.parse(item_contents, author_fallback)
end end
@ -253,7 +253,7 @@ private module Parsers
extend self extend self
include BaseParser include BaseParser
def self.process(item : JSON::Any, author_fallback : AuthorFallback) def process(item : JSON::Any, author_fallback : AuthorFallback)
if item_contents = item["hashtagTileRenderer"]? if item_contents = item["hashtagTileRenderer"]?
return self.parse(item_contents) return self.parse(item_contents)
end end
@ -306,7 +306,7 @@ private module Parsers
extend self extend self
include BaseParser include BaseParser
def self.process(item : JSON::Any, author_fallback : AuthorFallback) def process(item : JSON::Any, author_fallback : AuthorFallback)
if item_contents = item["gridPlaylistRenderer"]? if item_contents = item["gridPlaylistRenderer"]?
return self.parse(item_contents, author_fallback) return self.parse(item_contents, author_fallback)
end end
@ -350,7 +350,7 @@ private module Parsers
extend self extend self
include BaseParser include BaseParser
def self.process(item : JSON::Any, author_fallback : AuthorFallback) def process(item : JSON::Any, author_fallback : AuthorFallback)
if item_contents = item["playlistRenderer"]? if item_contents = item["playlistRenderer"]?
return self.parse(item_contents, author_fallback) return self.parse(item_contents, author_fallback)
end end
@ -413,7 +413,7 @@ private module Parsers
extend self extend self
include BaseParser include BaseParser
def self.process(item : JSON::Any, author_fallback : AuthorFallback) def process(item : JSON::Any, author_fallback : AuthorFallback)
if item_contents = item["shelfRenderer"]? if item_contents = item["shelfRenderer"]?
return self.parse(item_contents, author_fallback) return self.parse(item_contents, author_fallback)
end end
@ -481,7 +481,7 @@ private module Parsers
extend self extend self
include BaseParser include BaseParser
def self.process(item : JSON::Any, author_fallback : AuthorFallback) def process(item : JSON::Any, author_fallback : AuthorFallback)
if item_contents = item.dig?("itemSectionRenderer", "contents", 0) if item_contents = item.dig?("itemSectionRenderer", "contents", 0)
return self.parse(item_contents, author_fallback) return self.parse(item_contents, author_fallback)
end end
@ -510,7 +510,7 @@ private module Parsers
extend self extend self
include BaseParser include BaseParser
def self.process(item : JSON::Any, author_fallback : AuthorFallback) def process(item : JSON::Any, author_fallback : AuthorFallback)
if item_contents = item.dig?("richItemRenderer", "content") if item_contents = item.dig?("richItemRenderer", "content")
return self.parse(item_contents, author_fallback) return self.parse(item_contents, author_fallback)
end end
@ -543,7 +543,7 @@ private module Parsers
extend self extend self
include BaseParser include BaseParser
def self.process(item : JSON::Any, author_fallback : AuthorFallback) def process(item : JSON::Any, author_fallback : AuthorFallback)
if item_contents = item["reelItemRenderer"]? if item_contents = item["reelItemRenderer"]?
return self.parse(item_contents, author_fallback) return self.parse(item_contents, author_fallback)
end end
@ -640,7 +640,7 @@ private module Parsers
extend self extend self
include BaseParser include BaseParser
def self.process(item : JSON::Any, author_fallback : AuthorFallback) def process(item : JSON::Any, author_fallback : AuthorFallback)
if item_contents = item["lockupViewModel"]? if item_contents = item["lockupViewModel"]?
return self.parse(item_contents, author_fallback) return self.parse(item_contents, author_fallback)
end end
@ -718,7 +718,7 @@ private module Parsers
extend self extend self
include BaseParser include BaseParser
def self.process(item : JSON::Any, author_fallback : AuthorFallback) def process(item : JSON::Any, author_fallback : AuthorFallback)
if item_contents = item["shortsLockupViewModel"]? if item_contents = item["shortsLockupViewModel"]?
return self.parse(item_contents, author_fallback) return self.parse(item_contents, author_fallback)
end end