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

Thumbnail Error Responses are 200 OK and text/plain #333

Closed
Nerixyz opened this issue Jul 9, 2022 · 2 comments · Fixed by #334
Closed

Thumbnail Error Responses are 200 OK and text/plain #333

Nerixyz opened this issue Jul 9, 2022 · 2 comments · Fixed by #334
Assignees

Comments

@Nerixyz
Copy link
Contributor

Nerixyz commented Jul 9, 2022

Animated webp links aren't resolved, so requesting the thumbnail for such link will result in an error. However, the HTTP status code of the response is 200 OK (even though the response says 404). Furthermore, the content-type is text/plain but it's clear that JSON is returned.

Example link:
https://braize.pajlada.com/chatterino/thumbnail/https%3A%2F%2Fcdn.7tv.app%2Femote%2F60afbfbaa3648f409a6e5211%2F4x

@pajlada
Copy link
Member

pajlada commented Jul 9, 2022

I can confirm the issue that you've noted.

I'm working on fixing this, however, this required a pretty big overhaul as the values stored in the cache needs to include the status code and content type now.

As a result of this, Chatterino2 should be updated as well to handle non-200 codes better (i.e. handle onError in src/providers/LinkResolver.cpp) to output the message string if it exists in the json response, if the content type was application/json

@pajlada pajlada self-assigned this Jul 9, 2022
@pajlada
Copy link
Member

pajlada commented Jul 9, 2022

Thumbnails should return error codes (e.g. 500 or 404) if we can't produce a valid thumbnail

Normal tooltips we create should, in most cases, return 200 even if the underlying YouTube video did not exist.
The tooltip was created correctly. status informs the API user what sort of data to expect.

(just explaining for myself xd)

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 a pull request may close this issue.

2 participants