iv-org / invidious

Invidious is an alternative front-end to YouTube
https://invidious.io
GNU Affero General Public License v3.0
16.47k stars 1.85k forks source link

checking the status code returned by youtube #5052

Closed unixfox closed 3 weeks ago

unixfox commented 3 weeks ago

Should provide a better error message for the 429 rate limit: https://github.com/iv-org/invidious/issues/5032

SamantazFox commented 3 weeks ago

Such messages should be translated in the future, but that requires some rework of the translate functions first to support interpolation of more than one field.

iBicha commented 3 weeks ago

I feel like this change made things slightly worse. Invidious erroring using to show where things were failing (things that are not parsing, etc) and while this is the correct way to do it, now all we have is the status code without further context

unixfox commented 3 weeks ago

I think based on the URL alone it's going to be pretty obvious.

Usually even on errors, the yt api just returns a 200 status code.

iBicha commented 3 weeks ago

I think based on the URL alone it's going to be pretty obvious.

Usually even on errors, the yt api just returns a 200 status code.

I think it should at least contain the error from the body to make this more helpful than it was

unixfox commented 3 weeks ago

We would need an error template which summarize the error AND print the full error stack, but we don't have one.

For the moment, that's the best that we can do for clearly explaining to the user what's going on.

I still insist on the case that when you receive another status code other than 200 then something really wrong is going on. Many people will discover it and create new issues, the URL causing the issue will come up too at some point.

iBicha commented 3 weeks ago

I mean the YT response body can be just added to the body of InfoException, that is also fine

unixfox commented 3 weeks ago

But then these messages will never be able to be translated.

Hence why I said, we do not have a page template with a summary of the error AND the full stack error.

unixfox commented 3 weeks ago

And I forgot to say that we already parse the error given by youtube here: https://github.com/iv-org/invidious/blob/792d0d5f6df912039a58768e6ff503ae00abe7c0/src/invidious/yt_backend/youtube_api.cr#L654

Here https://github.com/iv-org/invidious/issues/5032 why Invidious is crashing. Is that it expect youtube to give a JSON body but receive an HTML one.

Now that I think again. Maybe we could only throw a simple error status code message when the status code is not 200 AND it's not a JSON body.

I think youtube probably return a JSON body with an error and a status code of 400

syeopite commented 3 weeks ago

I have a PR opened upstream in Kemal to allow for routes pertaining to specific raised errors https://github.com/kemalcr/kemal/pull/688

I haven't had time to work on it lately but once it is finished and merged we should be able to create customized error pages for different exceptions