Skip to content

Fix not-found route lookup handling#607

Draft
guytepper wants to merge 1 commit intomainfrom
error-not-found-fix
Draft

Fix not-found route lookup handling#607
guytepper wants to merge 1 commit intomainfrom
error-not-found-fix

Conversation

@guytepper
Copy link
Copy Markdown
Member

Summary

  • Stop throwing when route search exhausts fallback dates with no results
  • Keep resultType set to not-found so the UI can show the empty-state message

Testing

  • Not run (not requested)

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request modifies the train routes store to return an empty array instead of throwing an error when no routes are found, treating this as an expected UI state. While this change improves error handling, the feedback identifies a potential UI regression where the 'not-found' state is set unconditionally, which could cause full-screen empty states during 'load more' operations even if previous results exist. Additionally, the store currently retains stale route data from previous searches when a new search returns no results.

Comment on lines +91 to +92
// "No routes" is an expected UI state, not an exceptional failure.
return []
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Returning an empty array instead of throwing is a good improvement for handling expected 'no results' states. However, there is a side effect regarding the UI logic in RouteListScreen:

  1. UI Regression: Previously, when this function threw an error, the screen's onError handler would only set resultType to "not-found" if the total routeData was empty. Now, because getRoutes updates the store's resultType unconditionally on line 90 and returns successfully, the NoTrainsFoundMessage (which is a full-screen empty state) will be displayed at the bottom of the list whenever a 'load more' request for a subsequent date fails to find trains, even if the list already contains results from previous dates.
  2. Stale Data: The store's routes state is not cleared on line 90. If a previous search was successful, the store will continue to hold those routes even though the current search returned no results.

Consider making the state updates conditional or moving them to the caller to ensure the UI behaves correctly in aggregated views.

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 this pull request may close these issues.

1 participant