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

Raise error if video not playable, also handle missing related videos #2892

Merged
merged 1 commit into from
Feb 13, 2022

Conversation

matthewmcgarvey
Copy link
Contributor

@matthewmcgarvey matthewmcgarvey commented Feb 11, 2022

Fixes #2877
Related to #602

By returning params if reason is added to it and raising an exception if it's present, then we show an understandable error to users.

7IpVsQVyok8 is a video that is age restricted.

Before
Screen Shot 2022-02-10 at 11 48 48 PM
After
Screen Shot 2022-02-10 at 11 46 23 PM

I noticed that while working with 7IpVsQVyok8 it would occasionally return as playable and then fail because it didn't return any related videos. I updated the code to not fail on that. It shows the video, but without related videos

Example
Screen Shot 2022-02-10 at 11 53 05 PM
Normal video just for reference
Screen Shot 2022-02-10 at 11 54 46 PM

@matthewmcgarvey matthewmcgarvey requested a review from a team as a code owner February 11, 2022 05:53
@matthewmcgarvey matthewmcgarvey requested review from SamantazFox and removed request for a team February 11, 2022 05:53
Copy link
Member

@SamantazFox SamantazFox left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Comment on lines +954 to +955
secondary_results = main_results
.dig?("secondaryResults", "secondaryResults", "results")
Copy link
Member

@SamantazFox SamantazFox Feb 11, 2022

Choose a reason for hiding this comment

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

Just not sure why you moved this block ^^ (yeah, that's nitpicking)

@unixfox
Copy link
Member

unixfox commented Feb 11, 2022

in testing on https://yewtu.be

@SamantazFox
Copy link
Member

@unixfox seems good?

@unixfox
Copy link
Member

unixfox commented Feb 13, 2022

Seems like yes

@SamantazFox SamantazFox merged commit 8af202e into iv-org:master Feb 13, 2022
@matthewmcgarvey matthewmcgarvey deleted the video-playability branch February 14, 2022 14:04
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.

[Bug] Missing JSON element "twoColumnWatchNextResults" (BrokenTubeException)
3 participants