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

lyrics: detect MusixMatch blocking #2634

Merged
merged 2 commits into from
Jul 18, 2017

Conversation

anarcat
Copy link
Contributor

@anarcat anarcat commented Jul 17, 2017

we just look for the bad string in the HTML. this has the downside
that we may consider songs that have those exact lyrics (you never
know, really) may trigger this warning as well and we would fail to
fetch those songs.

we also fail if lyrics contain another magic string that seems to come
up when you do fill in the CAPTCHA after being blocked.

for an example of error messages in songs, I suggest listening to the
excellent "Retry? Abort? Ignore?" from the band called "Martyr".

this is to fix #2632, obviously.

we just look for the bad string in the HTML. this has the downside
that we may consider songs that have those exact lyrics (you never
know, really) may trigger this warning as well and we would fail to
fetch those songs.

we also fail if lyrics contain another magic string that seems to come
up when you do fill in the CAPTCHA after being blocked.
@mention-bot
Copy link

@anarcat, thanks for your PR! By analyzing the history of the files in this pull request, we identified @jrobeson, @Kraymer and @sampsyo to be potential reviewers.

@anarcat
Copy link
Contributor Author

anarcat commented Jul 17, 2017

i'm not sure about this. as I mention in the description, some bands actually use error messages in their lyrics now (!) so this might create false positives. especially for the latter check introduced...

@anarcat
Copy link
Contributor Author

anarcat commented Jul 17, 2017

i also wonder if the warning should not be a little more violent: should we just completely abort?

@anarcat
Copy link
Contributor Author

anarcat commented Jul 17, 2017

btw - the CAPTCHA solving doesn't work unless you carry over that cookie it sends you when you solve it. :( it also looks like the block is not permanent. i suspect there may be some rate-limiting going on here... for example:

lyrics: we are blocked at MusixMatch: url https://www.musixmatch.com/lyrics/Keny-Arkana/Entre-Les-Lignes-%3A-Une-Goutte-De-Plus failed
lyrics: lyrics not found: Keny Arkana - Entre ciment et belle étoile - Entre les lignes : Une goutte de plus
lyrics: lyrics already present: Keny Arkana - Entre ciment et belle étoile - Prière
lyrics: lyrics already present: Louis Armstrong - Louis Armstrong Plays W.C. Handy - St. Louis Blues
lyrics: lyrics already present: Louis Armstrong - Louis Armstrong Plays W.C. Handy - Yellow Dog Blues
lyrics: fetched lyrics: Louis Armstrong - Louis Armstrong Plays W.C. Handy - Loveless Love

now maybe the lyrics for that second entry were fetched elsewhere... but it seems to me an indicator of rate-limiting...

@sampsyo
Copy link
Member

sampsyo commented Jul 18, 2017

Looks good. Could you please add a changelog entry? Then this should be good to go.

i also wonder if the warning should not be a little more violent: should we just completely abort?

I'm good with this just being a warning—a fail-stop error seems a little aggressive, since other backends could work just fine.

@anarcat
Copy link
Contributor Author

anarcat commented Jul 18, 2017

changelog: done.

@sampsyo
Copy link
Member

sampsyo commented Jul 18, 2017

Perfect! ✨

@sampsyo sampsyo merged commit c06eca7 into beetbox:master Jul 18, 2017
@sampsyo
Copy link
Member

sampsyo commented Jul 18, 2017

Also, I sent you an invitation to join the @beetbox org. Thanks for all your help recently, and welcome aboard! As a collaborator you can push directly, of course, but don’t hesitate to use a PR if you ever want a code review.

@anarcat anarcat deleted the musixmatch-block-detect branch July 18, 2017 21:36
@anarcat
Copy link
Contributor Author

anarcat commented Jul 18, 2017

oh wow, thanks! i'll certainly send PRs for a while still, thank you so much for your confidence again, it's greatly appreciated!

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.

lyrics: Detect when you get blocked by Musixmatch
3 participants