Fix race condition in Unsplash search that causes mixed results#24583
Conversation
WalkthroughThe code modifies the Unsplash image search service to prevent stale search results from being appended when the user changes the search term during an asynchronous request. It introduces a mechanism to track the search term at the time of the request and ensures that only results matching the latest search term are processed. This involves updating the Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found. Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
🔇 Additional comments (4)
✨ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
The search debouncing was causing new search results to get appended to old search results, instead of overwriting the results. This fix captures the search term at request time and validates it matches the current search term before applying results, ensuring only relevant search results are displayed.
ref TryGhost#24583 - a stale search response was still able to overwrite the stored pagination links, so loading more results could fetch pages for the previous search term - applies the same stale search term guard to pagination extraction and adds unit tests covering both the discarded and accepted response paths
ce8db6c to
3d45c2d
Compare
9larsons
left a comment
There was a problem hiding this comment.
@niranjan-uma-shankar Thanks for this! I think there's still some gaps in the logic, though this ought to improve the gap here. Ultimately, we're moving to React and we'll need to rebuild all of this.
This PR fixes a race condition in the Unsplash search flow where results from an intermediate search term (e.g., "germ") may appear alongside the final term (e.g., "germany") when users pause briefly while typing.
Fixes: #24584
Root cause
The race condition occurs due to the combination of:
A debounce in
_search()delaying API calls.The
updateSearch()method triggering a UI reset too early, beforethis.photosis populated.No mechanism to discard results from outdated search terms.
I could consistently reproduce results on a ~50Mbps connection:
Changes made
The fix adds validation to ensure that search results are only applied if the returned term matches the current user input. Each
_makeRequest()call captures the term at the time it was made, and the response handler checks if it still matches the latest search term. If not, the results are discarded.Before
Screen.Recording.2025-08-02.at.3.47.40.PM.mp4
After
Screen.Recording.2025-08-02.at.3.50.05.PM.mp4
Testing Instructions