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

Reevaluate thumbnail fallback behavior #510

Open
zackkrida opened this issue Aug 31, 2022 · 6 comments
Open

Reevaluate thumbnail fallback behavior #510

zackkrida opened this issue Aug 31, 2022 · 6 comments
Labels
🕹 aspect: interface Concerns end-users' experience with the software 🛠 goal: fix Bug fix 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: frontend Related to the Nuxt frontend 💬 talk: discussion Open for discussions and feedback

Comments

@zackkrida
Copy link
Member

Currently, the if the search results page isn't able to get a thumbnail from the API /thumbs endpoint for an image, it falls back to using the full resolution image.

In situations where an entire source is unavailable due to an outage, or other problems where thumbnails are unavailable and the full-size images are quite large, this can mean a lot of large images downloading for the user at once and is a very poor user experience.

What are the situations where we would want an image with a failing thumbnail to be included in search results? Typically, a failing thumbnail suggests an issue with the source. It can also simple mean that source images are too large and our thumbnail service isn't able to process them without timing out.

Not sure what the best solution is here, some alternatives I see are:

  • Remove a result from the search results if the thumbnail fails. The frontend could automatically retry when the 'load more' button is pressed.
  • Show a 'thumbnail unavailable' or other placeholder thumbnail. Seems like a very bad experience.
@zackkrida zackkrida added 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work 🛠 goal: fix Bug fix 🟨 priority: medium Not blocking but should be addressed soon 💬 talk: discussion Open for discussions and feedback 🕹 aspect: interface Concerns end-users' experience with the software and removed 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work labels Aug 31, 2022
@obulat obulat transferred this issue from WordPress/openverse-frontend Feb 22, 2023
@obulat obulat added the 🧱 stack: frontend Related to the Nuxt frontend label Feb 22, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Openverse Backlog Feb 23, 2023
@sarayourfriend
Copy link
Collaborator

What are the situations where we would want an image with a failing thumbnail to be included in search results? Typically, a failing thumbnail suggests an issue with the source. It can also simple mean that source images are too large and our thumbnail service isn't able to process them without timing out.

@zackkrida do you know if this happens in practice? Theoretically dead link filtering should prevent this. Of course, there's always the possibility of a mismatch between the HEAD request used by dead link filtering and the real request used by the thumbnail proxy.

We could add a Plausible event to see how often this happens. I'd be curious to see whether there are real cases where this is a problem, rather than just hypothetically. If it is, it might suggest an issue with dead link detection, rather than something the frontend needs to consider... but it might be both.

@zackkrida
Copy link
Member Author

@sarayourfriend good questions, thanks for revisiting this.

Typically, a failing thumbnail suggests an issue with the source.

I think this assumption is pretty inaccurate in hindsight. I at least have no evidence to support it.

The titular issue to "Reevaluate thumbnail fallback behavior" can be closed, IMO, as we haven't encountered this problem in a long time. If images are unusually large to the point that thumbnails can not be generated or requests time out, I think we should fix that in the provider scripts themselves and see if a more usable file is available for the api's url column.

As I recall this issue was written in response to some of the SMK images being multiple GB in size, and we fixed it with the DAG-based approach I shared above.

We could add a Plausible event to see how often this happens

If we chose to investigate this, do you think we could look at API logs and thumbnail proxy error codes and avoid the need for a plausible event?

@sarayourfriend
Copy link
Collaborator

sarayourfriend commented May 23, 2024

If we chose to investigate this, do you think we could look at API logs and thumbnail proxy error codes and avoid the need for a plausible event?

Only if we think API logs will accurately reflect user experience to a meaningful degree. There's always the chance of something going wrong after the API has responded and clients not being able to receive the response for one reason or another. That's only known to the client. I think it's a fine starting point to look at the API response logs to see how much (if any) mismatch there is, but it's important to understand the limitations of that approach, depending on which perspective we're trying to capture.

We have an issue elsewhere to incorporate thumbnail errors into the dead link cache, IIRC, which might imply there's at least some assumption of misalignment between how we filter dead links and which thumbnail requests happen. Someone can of course pass filter_dead=False and request as many potentially dead thumbnails as they like. That's probably the biggest way in which using frontend real-user data is valuable here, because we know it will reflect actual frontend search flows, rather than the potentially random behaviours of other API users. If we look at request logs, we can at least filter on the referrer being openverse.org, though 🙂

@sarayourfriend
Copy link
Collaborator

@zackkrida #4665 has me thinking that it would be good to prioritise this. We show that "image not available" thing, so in some ways this general "thumbnail fallback" question is resolved (I am still on holiday so haven't looked into the history of how that started happening)... but it's a shame we don't incorporate a failing thumbnail on the frontend into how we cache/check dead links during search.

One option for that is in relation to #3585, we could query API request logs in a daily DAG for thumbnails (and waveforms!) that failed over the 24 hour period for the DAG execution date and bust the cached dead link check in redis or go ahead and do a "deep" link verification check using a GET rather than HEAD request. Log querying would be relatively easy and require no new API code, but I can't remember if we already have something in Redis that indicates a thumbnail failed within a certain time period that could be queried instead of logs. The keys from #4249 have a pretty short TTL IIRC, but might be suitable depending. Again, I'm on holiday, so haven't taken the time to look at the code, I opened #4665 during personal usage of Openverse and realised we'd discussed some ways of incorporating the frontend failure. The problem of real-user vs API-level failures still stands when it comes to API logs vs Plausible events. However, in the interest of finding the most immediately usable solution, API-level failures are easy to implement and could theoretically improve search in a meaningful way, without being blocked on the as-of-yet unsolved question of programmatic interactions with Plausible.

@zackkrida
Copy link
Member Author

@sarayourfriend I think the log-querying approach is a great idea, and could lead to some really quick wins!

On the frontend, I will update this issue to "Reevaluate thumbnail fallback behavior". The "image not avaliable" placeholder is pretty atrocious, with raster text, and could easily be a more-accessible html or svg fallback that matches the styling of the site better.

OR, probably better, is to just exclude those results from display.

@sarayourfriend
Copy link
Collaborator

Sounds good 👍

I mentioned other options to the "image not available" placeholder in #4665, including removing the result, but it would require a bit of thought to avoid potentially large layout shifts (at least with the current flow of the UI, showing the "skeleton" placeholders before the results load). Replacing them with HTML could be good, might be worth putting that suggestion on #4665 in case it's easy to implement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🕹 aspect: interface Concerns end-users' experience with the software 🛠 goal: fix Bug fix 🟨 priority: medium Not blocking but should be addressed soon 🧱 stack: frontend Related to the Nuxt frontend 💬 talk: discussion Open for discussions and feedback
Projects
Status: 📋 Backlog
Development

No branches or pull requests

3 participants