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

feat(ScheduleController): smaller schedules_for_stop response + logging #1790

Merged
merged 3 commits into from
Nov 7, 2023

Conversation

thecristen
Copy link
Collaborator

Summary of changes

Asana Ticket: Stop pages intermittently missing Commuter Rail schedules

I happened to be able to consistently reproduce the problem for a particular bus route on a particular stop on prod... but ultimately I never figured out a definitive cause for this. Splunk didn't reveal any errors. No network problems or console errors. I suspect that caching might exacerbate the problem and cause the backend to return [] instead of re-trying within the cache max_age window. But still, ideally it wouldn't erroneously return [] in the first place.

This PR does 3 things:

  1. Returns an error response if the Schedules.Repo.schedules_for_stop/2 call returns {:error, error} value. An error will be logged in Splunk and the frontend would handle it by showing the error view with the bus gif.
  2. Logs a message if the list of schedules returned happens to be empty. Just for informational/future debugging purposes should be issue persist and not be directly related to a Schedules.Repo error.
  3. Dramatically reduces the size of the JSON response for SiteWeb.ScheduleController.schedules_for_stop/2 in hopes of improving performance. It removes data that's not used on the frontend.

Comparison for Alewife Station (/api/stops/place-alfcl/schedules?last_stop_departures=false&future_departures=true), using Chrome DevTools with no caching and "Fast 3G" network throttling setting:

Before After Difference
Response size 272 kB 53.4 kB -80%
Time 2.49s 885ms -64%

General checks

  • Are the changes organized into self-contained commits with descriptive and well-formatted commit messages? This is a good practice that can facilitate easier reviews.
  • Testing. Do the changes include relevant passing updates to tests? This includes updating screenshots. Preferably tests are run locally to verify that there are no test failures created by these changes, before opening a PR.
  • Tech debt. Have you checked for tech debt you can address in the area you're working in? This can be a good time to address small issues, or create Asana tickets for larger issues.

and return error response to frontend
reduce response size by removing data that's unused by the frontend
@thecristen thecristen requested a review from a team as a code owner November 6, 2023 21:16
@thecristen thecristen requested a review from kotva006 November 6, 2023 21:16
@thecristen thecristen merged commit c2e04a9 into master Nov 7, 2023
25 of 26 checks passed
@thecristen thecristen deleted the cbj/schedules-for-stop-improvements branch November 7, 2023 17:34
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.

2 participants