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

handle youtube-dl metadata parsing errors + Json parsing errors now contain the parsed text #31

Merged
merged 1 commit into from
Dec 13, 2020

Conversation

Maspenguin
Copy link
Contributor

When youtube-dl is outdated it will fail to download some videos. Serenity handles this by silently ignoring the issue, leading to confusion.
This PR changes the youtube-dl metadata parsing to return an Error::Json when it fails to parse and also stores the parsed text in Error::Json.

With this PR when the serenity voice example is run with the old version of youtube-dl the following is output on some videos:
Err starting source: Json { error: Error("expected value", line: 1, column: 1), parsed_text: "ERROR: "token" parameter not in video info for unknown reason; please report this issue on https://yt-dl.org/bug . Make sure you are using the latest version; type youtube-dl -U to update. Be sure to call youtube-dl with the --verbose flag and include its complete output.\n" }
This indicates to the user how to fix the problem. i.e. update youtube-dl
Previously the example reported that it was playing the song even when the video failed to load due to outdated youtube-dl resulting in no audio output.

@arqunis arqunis added the enhancement New feature or request label Dec 13, 2020
Copy link
Member

@FelixMcFelix FelixMcFelix left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. LGTM, I think this is a common issue that people run into and this should reduce friction on that front!

@FelixMcFelix FelixMcFelix merged commit 8d6bd4f into serenity-rs:current Dec 13, 2020
FelixMcFelix pushed a commit to FelixMcFelix/songbird that referenced this pull request Dec 19, 2020
@FelixMcFelix FelixMcFelix mentioned this pull request Jan 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants