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

Video Ripper Broken #549

Closed
rephormat opened this issue May 19, 2017 · 7 comments
Closed

Video Ripper Broken #549

rephormat opened this issue May 19, 2017 · 7 comments
Assignees
Labels

Comments

@rephormat
Copy link
Contributor

rephormat commented May 19, 2017

No longer able to download videos. Sample error log from xHamster is attached.

Affected versions: >= 1.4.14

ripme.txt

@cyian-1756
Copy link
Contributor

cyian-1756 commented May 19, 2017

Can you give an example link?

Edit: nevermind it's in the log https://xhamster.com/movies/7711362/mature_take_a_shower.html

@cyian-1756
Copy link
Contributor

cyian-1756 commented May 19, 2017

After a bit of looking it seems that one of the updates since 1.4.13 has broken the ripper, I'll diff the versions and see if I can fix it

@cyian-1756
Copy link
Contributor

It looks like it was some of the changes @metaprime made here that broke the video ripper.

Replacing that file with the version from 1.4.13 fixes the error

@rephormat
Copy link
Contributor Author

Confirmed @cyian-1756 findings. The video ripper is broken for all supported sites. Updating Issue.

@rephormat rephormat changed the title xHamster.com: Video Downloads Not Working Video Ripper Broken May 19, 2017
@metaprime
Copy link
Collaborator

@cyian-1756 which file did you replace to fix it?

@metaprime metaprime self-assigned this May 23, 2017
@metaprime metaprime added this to the On-deck for 1.4.x milestone May 23, 2017
@metaprime metaprime added the bug label May 23, 2017
@metaprime
Copy link
Collaborator

Oh it's this change (part of #185):

-            } catch (Exception e) {
+            } catch (InstantiationException e) {
+                // Incompatible rippers *will* throw exceptions during instantiation.
+            } catch (IllegalAccessException e) {
+                // Incompatible rippers *will* throw exceptions during instantiation.
+            } catch (IllegalArgumentException e) {
+                // Incompatible rippers *will* throw exceptions during instantiation.
+            } catch (InvocationTargetException e) {
                 // Incompatible rippers *will* throw exceptions during instantiation.
             }

I wasn't a fan of this to begin with (but no strong opinion so I merged it anyway). There's no value in enumerating each type of exception when the handling is the same. And look, it introduced a bug by not handling all cases.

This also points out the need for improving test coverage and fixing up the CI...

@metaprime
Copy link
Collaborator

Fixed in 1.4.18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants