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

throw error if the videoID returned is different #3278

Merged
merged 1 commit into from
Aug 23, 2022

Conversation

unixfox
Copy link
Member

@unixfox unixfox commented Aug 23, 2022

Closes #3257

@unixfox unixfox requested a review from a team as a code owner August 23, 2022 09:52
@unixfox unixfox requested review from SamantazFox and removed request for a team August 23, 2022 09:52
@unixfox unixfox force-pushed the throw-error-wrong-video branch from d3ed16b to f527ca2 Compare August 23, 2022 11:22
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.

[Don't mind my previous (now deleted) comment]

We should probably put something in info["error"] instead of rising an exception. I'm pretty sure that rising an exception for the wrong video ID will break sonner or later.

Also, if we raise something, We should raise an InfoException (no trace) or just an Exception. A BrokenTubeException is used to give the name of a JSON object that we can't find.

EDIT: We can also create a dedicated VideoNotAvailableException

@unixfox
Copy link
Member Author

unixfox commented Aug 23, 2022

We should probably put something in info["error"] instead of rising an exception. I'm pretty sure that rising an exception for the wrong video ID will break sonner or later.

Then the user wouldn't have direct easy access to reporting the issue. Returning an info[error] means only a single message is displayed.

Also, if we raise something, We should raise an InfoException (no trace) or just an Exception. A BrokenTubeException is used to give the name of a JSON object that we can't find.

EDIT: We can also create a dedicated VideoNotAvailableException

Ok for a VideoNotAvailableException.

@SamantazFox
Copy link
Member

Ok for a VideoNotAvailableException.

Go for it, then!

@unixfox unixfox force-pushed the throw-error-wrong-video branch from f527ca2 to f47b674 Compare August 23, 2022 19:21
@unixfox
Copy link
Member Author

unixfox commented Aug 23, 2022

Ok I got it, please review again.

@unixfox unixfox force-pushed the throw-error-wrong-video branch from f47b674 to 85cb114 Compare August 23, 2022 19:23
@unixfox unixfox force-pushed the throw-error-wrong-video branch from 85cb114 to 389e491 Compare August 23, 2022 19:35
@SamantazFox SamantazFox merged commit 16b23ef into master Aug 23, 2022
@SamantazFox SamantazFox deleted the throw-error-wrong-video branch August 23, 2022 20:00
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.

[Enhancement] Throw an error if the video ID result is not the same as the one requested
2 participants