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

plugins.chzzk: live detail api response validation error fix #6002

Conversation

Clap2rap
Copy link
Contributor

@Clap2rap Clap2rap commented May 23, 2024

Fixes: #6001

Any better suggestions are welcome.

Copy link
Member

@bastimeyer bastimeyer 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. Changes look fine.

However, should there be a need for extending the validation schemas even further, then I'd say that the error handling code should be moved from the Plugin subclass (_get_live() and _get_video()) to the API class. The error handling is currently done in the Plugin subclass, so that it can simply return None instead of having to raise a NoStreamsError in the API class. Irrelevant here though, just a quick thought...

Talking about error handling, are VODs also affected by a potential None return type?

@bastimeyer bastimeyer added the plugin issue A Plugin does not work correctly label May 23, 2024
@Clap2rap
Copy link
Contributor Author

Not affected, based on VODs I've tested. All the VODs have non-null content if they exist. Otherwise, the API returns a code field with the value 404 (or other error codes) and a message field indicating that the VOD does not exist (or other error messages).

And I agree with what you said. Raising NoStreamsError would be a more elegant way to handle it.

@bastimeyer bastimeyer merged commit 6458c4a into streamlink:master May 24, 2024
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plugin issue A Plugin does not work correctly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

plugins.chzzk: new live detail response found
2 participants