-
-
Notifications
You must be signed in to change notification settings - Fork 80
Koenig: Fix race condition in Unsplash search that causes incorrect results #1574
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
base: main
Are you sure you want to change the base?
Koenig: Fix race condition in Unsplash search that causes incorrect results #1574
Conversation
Search requests might get cancelled if a previous request is running. This fix ensures that all search requests fire, and the results are included only if the current search term matches the term for which the results were fetched.
|
Warning Rate limit exceeded@niranjan-uma-shankar has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 27 minutes and 27 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (6)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
The loading indicator is persisted to be displayed as long as results are not updated. Moreover, the in memory equivalent is updated so that the last searched term is saved and available over there too.
Search debounce is increased to 600ms to match with the value in the admin Unsplash component.
Fixes TryGhost/Ghost#24640
Problem
The Unsplash search component (used in the lexical editor and Design Settings) can sometimes fail to request results for the final search term. When typing a term like "Trumpet", the component may only send a request for a partial keyword (e.g. "Trump") and never send the request for the full term "Trumpet". This is caused by logic that prevents a new search from being initiated while a prior request is still in flight.
In the example below, the search for "Trumpet" shows results for "Trump".
lfinal.mp4
Root cause
New search is skipped while a prior request is running. The search logic contains a guard that prevents starting a new request if another is still in progress. This means the final, complete query may never be sent.
For example, when typing "Jupiter" with a brief pause after "Jupi", the debounced search (300 ms) fires for "Jupi". If the remaining "ter" is typed while the "Jupi" request is still in flight, the "Jupiter" request is never made, because the guard blocks it until the current request completes.
Debounce is short. The debounce is relatively short at 300 ms, which increases the probability that a previous request is still running when a new search is fired (The Ember Unsplash component uses 600 ms for reference.)
Fix
For example, in the "Jupiter" scenario above, if the active term is "Jupiter", ignore updating the UI with "Jupi" response and instead continue showing the loading indicator.